From 4602e38251a6fc9628f323e3f98b346f9f21702f Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Sun, 30 Aug 2026 12:58:17 +0600 Subject: [PATCH 1/3] feat(web-api): resolve category/product by alias or uri Add public GET /category/get and /product/get lookup for headless SSR routing without numeric resource ids. Cache remains a follow-up. --- .../minishop3/config/routes/web.php | 10 + .../minishop3/lexicon/en/default.inc.php | 3 + .../minishop3/lexicon/ru/default.inc.php | 3 + .../Api/Web/CategoryController.php | 43 ++++ .../Controllers/Api/Web/ProductController.php | 43 ++++ .../src/Middleware/TokenMiddleware.php | 8 +- .../src/Services/Catalog/CatalogResolve.php | 195 ++++++++++++++++++ .../Category/CategoryCatalogService.php | 69 +++++++ .../Product/ProductCatalogService.php | 73 +++++++ .../tests/CategoryCatalogRoutesTest.php | 16 +- .../WebApi/HeadlessStorefrontErrorsTest.php | 6 + .../tests/ProductCatalogRoutesTest.php | 61 ++++++ .../tests/TokenMiddlewarePublicRoutesTest.php | 11 +- .../Services/Catalog/CatalogResolveTest.php | 140 +++++++++++++ 14 files changed, 674 insertions(+), 7 deletions(-) create mode 100644 core/components/minishop3/src/Services/Catalog/CatalogResolve.php create mode 100644 core/components/minishop3/tests/ProductCatalogRoutesTest.php create mode 100644 core/components/minishop3/tests/Unit/Services/Catalog/CatalogResolveTest.php diff --git a/core/components/minishop3/config/routes/web.php b/core/components/minishop3/config/routes/web.php index 1ab357bc6..7944b69e0 100644 --- a/core/components/minishop3/config/routes/web.php +++ b/core/components/minishop3/config/routes/web.php @@ -270,6 +270,11 @@ // Public catalog — no TokenMiddleware (headless storefront without customer session) $router->group('/product', function ($router) use ($modx) { + $router->get('/get', function ($params) use ($modx) { + $controller = new \MiniShop3\Controllers\Api\Web\ProductController($modx); + return $controller->resolve($params); + }); + $router->get('/get/{id}', function ($params) use ($modx) { $controller = new \MiniShop3\Controllers\Api\Web\ProductController($modx); return $controller->get($params); @@ -293,6 +298,11 @@ // Public category catalog — no TokenMiddleware (headless nav / PLP) $router->group('/category', function ($router) use ($modx) { + $router->get('/get', function ($params) use ($modx) { + $controller = new \MiniShop3\Controllers\Api\Web\CategoryController($modx); + return $controller->resolve($params); + }); + $router->get('/get/{id}', function ($params) use ($modx) { $controller = new \MiniShop3\Controllers\Api\Web\CategoryController($modx); return $controller->get($params); diff --git a/core/components/minishop3/lexicon/en/default.inc.php b/core/components/minishop3/lexicon/en/default.inc.php index dd1563220..ffd9249c7 100644 --- a/core/components/minishop3/lexicon/en/default.inc.php +++ b/core/components/minishop3/lexicon/en/default.inc.php @@ -232,6 +232,9 @@ $_lang['ms3_err_product_update_failed'] = 'Failed to update product'; $_lang['ms3_err_catalog_parents_invalid'] = 'Invalid parents filter'; $_lang['ms3_err_catalog_parents_limit'] = 'Too many parent category IDs'; +$_lang['ms3_err_catalog_lookup_required'] = 'Provide exactly one of alias or uri.'; +$_lang['ms3_err_catalog_lookup_conflict'] = 'Provide alias or uri, not both.'; +$_lang['ms3_err_catalog_lookup_invalid'] = 'Invalid catalog lookup parameter.'; $_lang['ms3_err_catalog_price_invalid'] = 'Invalid price filter'; $_lang['ms3_err_catalog_price_range'] = 'price_max must be greater than or equal to price_min'; $_lang['ms3_err_catalog_stock_invalid'] = 'Invalid stock_min filter'; diff --git a/core/components/minishop3/lexicon/ru/default.inc.php b/core/components/minishop3/lexicon/ru/default.inc.php index 8a16a5903..4bca98fbd 100644 --- a/core/components/minishop3/lexicon/ru/default.inc.php +++ b/core/components/minishop3/lexicon/ru/default.inc.php @@ -232,6 +232,9 @@ $_lang['ms3_err_product_update_failed'] = 'Не удалось обновить товар'; $_lang['ms3_err_catalog_parents_invalid'] = 'Некорректный фильтр parents'; $_lang['ms3_err_catalog_parents_limit'] = 'Слишком много ID категорий в parents'; +$_lang['ms3_err_catalog_lookup_required'] = 'Укажите ровно один параметр: alias или uri.'; +$_lang['ms3_err_catalog_lookup_conflict'] = 'Укажите alias или uri, но не оба сразу.'; +$_lang['ms3_err_catalog_lookup_invalid'] = 'Некорректный параметр поиска в каталоге.'; $_lang['ms3_err_catalog_price_invalid'] = 'Некорректный фильтр цены'; $_lang['ms3_err_catalog_price_range'] = 'price_max должен быть не меньше price_min'; $_lang['ms3_err_catalog_stock_invalid'] = 'Некорректный фильтр stock_min'; diff --git a/core/components/minishop3/src/Controllers/Api/Web/CategoryController.php b/core/components/minishop3/src/Controllers/Api/Web/CategoryController.php index 01edf1cd8..8b6836f2b 100644 --- a/core/components/minishop3/src/Controllers/Api/Web/CategoryController.php +++ b/core/components/minishop3/src/Controllers/Api/Web/CategoryController.php @@ -6,6 +6,7 @@ use MiniShop3\Router\HttpStatus; use MiniShop3\Router\Response; +use MiniShop3\Services\Catalog\CatalogResolve; use MiniShop3\Services\Category\CategoryCatalogService; use MODX\Revolution\modX; @@ -52,6 +53,48 @@ public function get(array $params = []): Response return Response::success($category); } + /** + * GET /api/v1/category/get?alias=…|uri=…&context=… + * + * @param array $params + */ + public function resolve(array $params = []): Response + { + $parsed = CatalogResolve::parseLookup( + $params, + (string) ($this->modx->context->key ?? 'web'), + ); + + if (!$parsed['ok']) { + $lexiconKey = match ($parsed['error']) { + 'required' => 'ms3_err_catalog_lookup_required', + 'conflict' => 'ms3_err_catalog_lookup_conflict', + 'invalid' => 'ms3_err_catalog_lookup_invalid', + }; + + return Response::error( + $this->modx->lexicon($lexiconKey), + HttpStatus::BAD_REQUEST + ); + } + + $category = $this->catalog()->resolveByLookup( + $params, + $parsed['field'], + $parsed['value'], + $parsed['context'], + ); + + if ($category === null) { + return Response::error( + $this->modx->lexicon('ms3_err_category_nf'), + HttpStatus::NOT_FOUND + ); + } + + return Response::success($category); + } + /** * GET /api/v1/category/list * diff --git a/core/components/minishop3/src/Controllers/Api/Web/ProductController.php b/core/components/minishop3/src/Controllers/Api/Web/ProductController.php index a9c308312..cb7bff910 100644 --- a/core/components/minishop3/src/Controllers/Api/Web/ProductController.php +++ b/core/components/minishop3/src/Controllers/Api/Web/ProductController.php @@ -6,6 +6,7 @@ use MiniShop3\Router\HttpStatus; use MiniShop3\Router\Response; +use MiniShop3\Services\Catalog\CatalogResolve; use MiniShop3\Services\Product\ProductCatalogFilterException; use MiniShop3\Services\Product\ProductCatalogService; use MiniShop3\Services\Product\ProductFacetService; @@ -56,6 +57,48 @@ public function get(array $params = []): Response return Response::success($product); } + /** + * GET /api/v1/product/get?alias=…|uri=…&context=… + * + * @param array $params + */ + public function resolve(array $params = []): Response + { + $parsed = CatalogResolve::parseLookup( + $params, + (string) ($this->modx->context->key ?? 'web'), + ); + + if (!$parsed['ok']) { + $lexiconKey = match ($parsed['error']) { + 'required' => 'ms3_err_catalog_lookup_required', + 'conflict' => 'ms3_err_catalog_lookup_conflict', + 'invalid' => 'ms3_err_catalog_lookup_invalid', + }; + + return Response::error( + $this->modx->lexicon($lexiconKey), + HttpStatus::BAD_REQUEST + ); + } + + $product = $this->catalog()->resolveByLookup( + $params, + $parsed['field'], + $parsed['value'], + $parsed['context'], + ); + + if ($product === null) { + return Response::error( + $this->modx->lexicon('ms3_err_product_nf'), + HttpStatus::NOT_FOUND + ); + } + + return Response::success($product); + } + /** * GET /api/v1/product/list * diff --git a/core/components/minishop3/src/Middleware/TokenMiddleware.php b/core/components/minishop3/src/Middleware/TokenMiddleware.php index 6550d8a07..638ca5c6a 100644 --- a/core/components/minishop3/src/Middleware/TokenMiddleware.php +++ b/core/components/minishop3/src/Middleware/TokenMiddleware.php @@ -31,12 +31,12 @@ class TokenMiddleware implements MiddlewareInterface { private modX $modx; - /** @var list Prefixes matched with str_starts_with; missing token skips auto-mint */ + /** @var list Prefixes matched exactly or as path segment; missing token skips auto-mint */ private array $publicRoutes = [ - '/api/v1/product/get/', + '/api/v1/product/get', '/api/v1/product/list', '/api/v1/product/filters', - '/api/v1/category/get/', + '/api/v1/category/get', '/api/v1/category/list', '/api/v1/category/tree', '/api/v1/delivery/get/', @@ -209,7 +209,7 @@ private function isPublicRoute(string $uri): bool $route = $this->normalizePublicPath((string) $route); foreach ($this->publicRoutes as $publicRoute) { - if (str_starts_with($route, $publicRoute)) { + if ($route === $publicRoute || str_starts_with($route, $publicRoute . '/')) { return true; } } diff --git a/core/components/minishop3/src/Services/Catalog/CatalogResolve.php b/core/components/minishop3/src/Services/Catalog/CatalogResolve.php new file mode 100644 index 000000000..43bd4af4a --- /dev/null +++ b/core/components/minishop3/src/Services/Catalog/CatalogResolve.php @@ -0,0 +1,195 @@ + $criteria + */ + public static function findUniqueId(modX $modx, string $class, array $criteria): ?int + { + $query = $modx->newQuery($class, $criteria); + $query->select($modx->getSelectColumns($class, $class, '', ['id'])); + $query->limit(2); + + if (!$query->prepare() || !$query->stmt->execute()) { + return null; + } + + $ids = []; + while ($row = $query->stmt->fetch(\PDO::FETCH_ASSOC)) { + $id = (int) ($row['id'] ?? 0); + if ($id > 0) { + $ids[] = $id; + } + } + + return count($ids) === 1 ? $ids[0] : null; + } + + /** + * @param array $params + * @return array{ok: true, field: 'alias'|'uri', value: string, context: string}|array{ok: false, error: 'required'|'conflict'|'invalid'} + */ + public static function parseLookup(array $params, string $contextFallback = 'web'): array + { + foreach (['alias', 'uri', 'context'] as $key) { + if (array_key_exists($key, $params) && !self::isScalarLookupParam($params[$key])) { + return ['ok' => false, 'error' => 'invalid']; + } + } + + $hasAlias = array_key_exists('alias', $params); + $hasUri = array_key_exists('uri', $params); + + $aliasRaw = $hasAlias ? trim((string) $params['alias']) : ''; + $uriRaw = $hasUri ? trim((string) $params['uri']) : ''; + + $aliasProvided = $hasAlias && $aliasRaw !== ''; + $uriProvided = $hasUri && $uriRaw !== ''; + + if ($aliasProvided && $uriProvided) { + return ['ok' => false, 'error' => 'conflict']; + } + + if (!$aliasProvided && !$uriProvided) { + return ['ok' => false, 'error' => 'required']; + } + + $context = self::resolveLookupContext($params, $contextFallback); + if ($context === null) { + return ['ok' => false, 'error' => 'invalid']; + } + + if ($aliasProvided) { + if (!self::isValidAlias($aliasRaw)) { + return ['ok' => false, 'error' => 'invalid']; + } + + return [ + 'ok' => true, + 'field' => 'alias', + 'value' => $aliasRaw, + 'context' => $context, + ]; + } + + $uri = self::normalizeUri($uriRaw); + if ($uri === null) { + return ['ok' => false, 'error' => 'invalid']; + } + + return [ + 'ok' => true, + 'field' => 'uri', + 'value' => $uri, + 'context' => $context, + ]; + } + + /** + * Trim, strip leading slash, reject unsafe path segments. + */ + public static function normalizeUri(string $uri): ?string + { + $uri = trim($uri); + if ($uri === '') { + return null; + } + + $uri = ltrim($uri, '/'); + if ($uri === '' || self::containsUriRejectPattern($uri)) { + return null; + } + + return $uri; + } + + /** + * Exact uri first, then slash variant (with or without trailing slash). + * + * @return list + */ + public static function uriLookupVariants(string $normalizedUri): array + { + $variants = [$normalizedUri]; + + if (str_ends_with($normalizedUri, '/')) { + $trimmed = rtrim($normalizedUri, '/'); + if ($trimmed !== '') { + $variants[] = $trimmed; + } + } else { + $variants[] = $normalizedUri . '/'; + } + + return array_values(array_unique($variants)); + } + + /** + * @param array $params + */ + private static function resolveLookupContext(array $params, string $fallback): ?string + { + if (array_key_exists('context', $params)) { + $raw = trim((string) $params['context']); + if ($raw !== '') { + return self::sanitizeContext($raw); + } + } + + $resolved = CatalogQuery::resolveContext($params, $fallback); + + return self::sanitizeContext($resolved); + } + + private static function isScalarLookupParam(mixed $value): bool + { + return is_string($value) || is_int($value); + } + + private static function sanitizeContext(string $key): ?string + { + $key = trim($key); + if ($key === '' || strlen($key) > self::MAX_CONTEXT_LENGTH) { + return null; + } + + if (!preg_match('/^[a-zA-Z0-9_-]+$/', $key)) { + return null; + } + + if (str_starts_with(strtolower($key), 'mgr')) { + return null; + } + + return $key; + } + + private static function isValidAlias(string $alias): bool + { + return $alias !== '' && !self::containsUriRejectPattern($alias); + } + + private static function containsUriRejectPattern(string $value): bool + { + return str_contains($value, '..') + || str_contains($value, '://') + || str_contains($value, "\0") + || str_contains($value, '//'); + } +} diff --git a/core/components/minishop3/src/Services/Category/CategoryCatalogService.php b/core/components/minishop3/src/Services/Category/CategoryCatalogService.php index e5292cf1e..f5c54de3a 100644 --- a/core/components/minishop3/src/Services/Category/CategoryCatalogService.php +++ b/core/components/minishop3/src/Services/Category/CategoryCatalogService.php @@ -6,6 +6,7 @@ use MiniShop3\Model\msCategory; use MiniShop3\Services\Catalog\CatalogQuery; +use MiniShop3\Services\Catalog\CatalogResolve; use MODX\Revolution\modX; use xPDO\Om\xPDOQuery; @@ -161,6 +162,33 @@ public function getById(int $categoryId, array $params = []): ?array return $payload; } + /** + * Resolve category by alias OR uri (+ context). Same payload as getById(). + * + * @param array $params + */ + public function resolveByLookup( + array $params, + string $field, + string $value, + string $context, + ): ?array { + $paramsWithContext = array_merge($params, ['context' => $context]); + $includeHidden = CatalogQuery::toBool($params['include_hidden'] ?? false); + + $categoryId = match ($field) { + 'alias' => $this->findIdByAlias($value, $paramsWithContext, $includeHidden), + 'uri' => $this->findIdByUri($value, $paramsWithContext, $includeHidden), + default => null, + }; + + if ($categoryId === null) { + return null; + } + + return $this->getById($categoryId, $paramsWithContext); + } + /** * @param array $params * @return array{items: list>, total: int, limit: int, offset: int} @@ -258,6 +286,47 @@ private function findVisibleCategory(int $categoryId, array $params, bool $inclu return $category ?: null; } + /** + * @param array $params + */ + private function findIdByAlias(string $alias, array $params, bool $includeHidden): ?int + { + return CatalogResolve::findUniqueId( + $this->modx, + msCategory::class, + $this->lookupCriteria(['alias' => $alias], $params, $includeHidden), + ); + } + + /** + * @param array $params + */ + private function findIdByUri(string $uri, array $params, bool $includeHidden): ?int + { + foreach (CatalogResolve::uriLookupVariants($uri) as $variant) { + $id = CatalogResolve::findUniqueId( + $this->modx, + msCategory::class, + $this->lookupCriteria(['uri' => $variant], $params, $includeHidden), + ); + if ($id !== null) { + return $id; + } + } + + return null; + } + + /** + * @param array $extra + * @param array $params + * @return array + */ + private function lookupCriteria(array $extra, array $params, bool $includeHidden): array + { + return $this->publicCriteria(array_merge($extra, $this->visibilityCriteria($params, $includeHidden))); + } + /** * @param array $params */ diff --git a/core/components/minishop3/src/Services/Product/ProductCatalogService.php b/core/components/minishop3/src/Services/Product/ProductCatalogService.php index 6c8876d18..498f11fd0 100644 --- a/core/components/minishop3/src/Services/Product/ProductCatalogService.php +++ b/core/components/minishop3/src/Services/Product/ProductCatalogService.php @@ -8,6 +8,7 @@ use MiniShop3\Model\msProductData; use MiniShop3\Model\msCategoryMember; use MiniShop3\Services\Catalog\CatalogQuery; +use MiniShop3\Services\Catalog\CatalogResolve; use MiniShop3\Services\Category\CategoryProductMenuindexService; use MiniShop3\Services\Category\CategoryProductScopeService; use MiniShop3\Services\Option\OptionService; @@ -263,6 +264,31 @@ private function loadImagesForProducts(array $products): array return $this->gallery()->loadForProducts($meta, ProductGalleryPublicSerializer::MAX_IMAGES_LIST); } + /** + * Resolve product by alias OR uri (+ context). Same payload as getById(). + * + * @param array $params + */ + public function resolveByLookup( + array $params, + string $field, + string $value, + string $context, + ): ?array { + $paramsWithContext = array_merge($params, ['context' => $context]); + $productId = match ($field) { + 'alias' => $this->findIdByAlias($value, $paramsWithContext), + 'uri' => $this->findIdByUri($value, $paramsWithContext), + default => null, + }; + + if ($productId === null) { + return null; + } + + return $this->getById($productId, $paramsWithContext); + } + /** * Paginated product list. * @@ -352,6 +378,53 @@ private function resolveContext(array $params): string ); } + /** + * @param array $params + */ + private function findIdByAlias(string $alias, array $params): ?int + { + return CatalogResolve::findUniqueId( + $this->modx, + msProduct::class, + $this->lookupCriteria(['alias' => $alias], $params), + ); + } + + /** + * @param array $params + */ + private function findIdByUri(string $uri, array $params): ?int + { + foreach (CatalogResolve::uriLookupVariants($uri) as $variant) { + $id = CatalogResolve::findUniqueId( + $this->modx, + msProduct::class, + $this->lookupCriteria(['uri' => $variant], $params), + ); + if ($id !== null) { + return $id; + } + } + + return null; + } + + /** + * @param array $extra + * @param array $params + * @return array + */ + private function lookupCriteria(array $extra, array $params): array + { + $criteria = $extra; + $context = $this->resolveContext($params); + if ($context !== '') { + $criteria['context_key'] = $context; + } + + return $this->publicCriteria($criteria); + } + /** * @param array $params */ diff --git a/core/components/minishop3/tests/CategoryCatalogRoutesTest.php b/core/components/minishop3/tests/CategoryCatalogRoutesTest.php index 2ebe19ae1..1c102a4b1 100644 --- a/core/components/minishop3/tests/CategoryCatalogRoutesTest.php +++ b/core/components/minishop3/tests/CategoryCatalogRoutesTest.php @@ -24,10 +24,12 @@ foreach ( [ "group('/category'", + "get('/get',", "get('/get/{id}'", "get('/list'", "get('/tree'", 'CategoryController', + '->resolve($params)', ] as $needle ) { if (!str_contains($webRoutes, $needle)) { @@ -35,7 +37,7 @@ } } -foreach (['function get(', 'function getList(', 'function getTree('] as $method) { +foreach (['function get(', 'function resolve(', 'function getList(', 'function getTree('] as $method) { if (!str_contains($controllerSrc, $method)) { $fail("CategoryController missing {$method}"); } @@ -49,6 +51,18 @@ $fail('CategoryCatalogService must scope published msCategory'); } +if (!str_contains($serviceSrc, 'resolveByLookup')) { + $fail('CategoryCatalogService must implement resolveByLookup'); +} + +if (!str_contains($serviceSrc, 'CatalogResolve::findUniqueId')) { + $fail('CategoryCatalogService must resolve alias/uri via CatalogResolve::findUniqueId'); +} + +if (!str_contains($controllerSrc, 'CatalogResolve::parseLookup')) { + $fail('CategoryController must parse catalog lookup via CatalogResolve'); +} + if (!str_contains($serviceSrc, 'hidemenu')) { $fail('CategoryCatalogService must handle hidemenu / include_hidden'); } diff --git a/core/components/minishop3/tests/Integration/WebApi/HeadlessStorefrontErrorsTest.php b/core/components/minishop3/tests/Integration/WebApi/HeadlessStorefrontErrorsTest.php index 23af5c4c8..e4eea6cbc 100644 --- a/core/components/minishop3/tests/Integration/WebApi/HeadlessStorefrontErrorsTest.php +++ b/core/components/minishop3/tests/Integration/WebApi/HeadlessStorefrontErrorsTest.php @@ -21,6 +21,12 @@ public function testProductGetUnknownIdReturns404Envelope(): void $this->assertApiError($res, HttpStatus::NOT_FOUND, 'ms3_err_product_nf'); } + public function testProductGetWithoutLookupParamsReturns400(): void + { + $res = $this->dispatch('GET', '/api/v1/product/get'); + $this->assertApiError($res, HttpStatus::BAD_REQUEST, 'ms3_err_catalog_lookup_required'); + } + public function testInvalidBearerReturns401(): void { $res = $this->dispatch( diff --git a/core/components/minishop3/tests/ProductCatalogRoutesTest.php b/core/components/minishop3/tests/ProductCatalogRoutesTest.php new file mode 100644 index 000000000..51b6c6e38 --- /dev/null +++ b/core/components/minishop3/tests/ProductCatalogRoutesTest.php @@ -0,0 +1,61 @@ +resolve($params)', + ] as $needle +) { + if (!str_contains($webRoutes, $needle)) { + $fail("web.php missing: {$needle}"); + } +} + +foreach (['function get(', 'function resolve('] as $method) { + if (!str_contains($controllerSrc, $method)) { + $fail("ProductController missing {$method}"); + } +} + +if (!str_contains($controllerSrc, 'CatalogResolve::parseLookup')) { + $fail('ProductController must parse catalog lookup via CatalogResolve'); +} + +if (!str_contains($controllerSrc, 'ms3_err_catalog_lookup_required')) { + $fail('ProductController must map catalog lookup parse errors'); +} + +if (!str_contains($serviceSrc, 'resolveByLookup')) { + $fail('ProductCatalogService must implement resolveByLookup'); +} + +if (!str_contains($serviceSrc, "'hidemenu' => 0")) { + $fail('ProductCatalogService publicCriteria must enforce hidemenu=0'); +} + +fwrite(STDOUT, "OK ProductCatalogRoutesTest\n"); +exit(0); diff --git a/core/components/minishop3/tests/TokenMiddlewarePublicRoutesTest.php b/core/components/minishop3/tests/TokenMiddlewarePublicRoutesTest.php index fe39388f9..8be2d3231 100644 --- a/core/components/minishop3/tests/TokenMiddlewarePublicRoutesTest.php +++ b/core/components/minishop3/tests/TokenMiddlewarePublicRoutesTest.php @@ -45,10 +45,10 @@ foreach ( [ - '/api/v1/product/get/', + '/api/v1/product/get', '/api/v1/product/list', '/api/v1/product/filters', - '/api/v1/category/get/', + '/api/v1/category/get', '/api/v1/category/list', '/api/v1/category/tree', '/api/v1/delivery/get/', @@ -105,6 +105,13 @@ $fail('TokenMiddleware mint failure must use internal_error (not token_required)'); } +if ( + !str_contains($middlewareSrc, '$route === $publicRoute') + || !str_contains($middlewareSrc, "str_starts_with(\$route, \$publicRoute . '/')") +) { + $fail('isPublicRoute must match exact path or segment prefix, not blind str_starts_with'); +} + if ( !str_contains($middlewareSrc, "'ms3_err_token_expired'") && !str_contains($middlewareSrc, '"ms3_err_token_expired"') diff --git a/core/components/minishop3/tests/Unit/Services/Catalog/CatalogResolveTest.php b/core/components/minishop3/tests/Unit/Services/Catalog/CatalogResolveTest.php new file mode 100644 index 000000000..2641b404a --- /dev/null +++ b/core/components/minishop3/tests/Unit/Services/Catalog/CatalogResolveTest.php @@ -0,0 +1,140 @@ + false, 'error' => 'required'], $result); + } + + public function testRequiredWhenBothEmptyAfterTrim(): void + { + $result = CatalogResolve::parseLookup(['alias' => ' ', 'uri' => '']); + + self::assertSame(['ok' => false, 'error' => 'required'], $result); + } + + public function testConflictWhenBothProvided(): void + { + $result = CatalogResolve::parseLookup(['alias' => 'tea', 'uri' => 'catalog/tea']); + + self::assertSame(['ok' => false, 'error' => 'conflict'], $result); + } + + #[DataProvider('invalidLookupParams')] + public function testInvalidLookup(array $params): void + { + $result = CatalogResolve::parseLookup($params); + + self::assertSame(['ok' => false, 'error' => 'invalid'], $result); + } + + /** + * @return iterable}> + */ + public static function invalidLookupParams(): iterable + { + yield 'uri path traversal' => [['uri' => '../secret']]; + yield 'uri scheme' => [['uri' => 'http://evil']]; + yield 'uri double slash' => [['uri' => 'a//b']]; + yield 'uri nul' => [['uri' => "bad\0uri"]]; + yield 'alias path traversal' => [['alias' => '..']]; + yield 'mgr context' => [['alias' => 'tea', 'context' => 'mgr']]; + yield 'mgr prefix context' => [['alias' => 'tea', 'context' => 'mgrCustom']]; + yield 'invalid context chars' => [['alias' => 'tea', 'context' => 'en us']]; + yield 'alias array param' => [['alias' => ['x']]]; + yield 'uri array param' => [['uri' => ['catalog/tea']]]; + yield 'context array param' => [['alias' => 'tea', 'context' => ['web']]]; + yield 'alias object param' => [['alias' => (object) ['x' => 1]]]; + } + + public function testAliasLookupWithContextFallback(): void + { + $result = CatalogResolve::parseLookup(['alias' => 'tea'], 'shop'); + + self::assertSame( + [ + 'ok' => true, + 'field' => 'alias', + 'value' => 'tea', + 'context' => 'shop', + ], + $result + ); + } + + public function testUriLookupNormalizesLeadingSlash(): void + { + $result = CatalogResolve::parseLookup(['uri' => '/catalog/tea/']); + + self::assertTrue($result['ok']); + self::assertSame('uri', $result['field']); + self::assertSame('catalog/tea/', $result['value']); + self::assertSame('web', $result['context']); + } + + public function testExplicitContextIsSanitized(): void + { + $result = CatalogResolve::parseLookup(['alias' => 'tea', 'context' => ' en ']); + + self::assertTrue($result['ok']); + self::assertSame('en', $result['context']); + } + + public function testEmptyContextFallsBack(): void + { + $result = CatalogResolve::parseLookup(['alias' => 'tea', 'context' => ' '], 'web'); + + self::assertTrue($result['ok']); + self::assertSame('web', $result['context']); + } + + public function testMgrFallbackContextIsRejected(): void + { + $result = CatalogResolve::parseLookup(['alias' => 'tea'], 'mgr'); + + self::assertSame(['ok' => false, 'error' => 'invalid'], $result); + } + + public function testIntAliasIsAccepted(): void + { + $result = CatalogResolve::parseLookup(['alias' => 42], 'web'); + + self::assertTrue($result['ok']); + self::assertSame('42', $result['value']); + } + + public function testNormalizeUriStripsLeadingSlash(): void + { + self::assertSame('catalog/tea', CatalogResolve::normalizeUri('/catalog/tea')); + } + + public function testNormalizeUriRejectsUnsafePatterns(): void + { + self::assertNull(CatalogResolve::normalizeUri('../x')); + self::assertNull(CatalogResolve::normalizeUri('https://x')); + self::assertNull(CatalogResolve::normalizeUri('a//b')); + } + + public function testUriLookupVariantsIncludeSlashForms(): void + { + self::assertSame( + ['catalog/tea', 'catalog/tea/'], + CatalogResolve::uriLookupVariants('catalog/tea') + ); + self::assertSame( + ['catalog/tea/', 'catalog/tea'], + CatalogResolve::uriLookupVariants('catalog/tea/') + ); + } +} From bf9840d678fb36f85c4383d5dd12b0bb58aa7029 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Sun, 30 Aug 2026 13:02:47 +0600 Subject: [PATCH 2/3] fix(web-api): select id without FQCN alias in catalog resolve getSelectColumns with the FQCN as table alias produced invalid SQL for namespaced msProduct/msCategory, so alias/uri lookup always returned 404. --- .../minishop3/src/Services/Catalog/CatalogResolve.php | 3 ++- .../minishop3/tests/ProductCatalogRoutesTest.php | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/core/components/minishop3/src/Services/Catalog/CatalogResolve.php b/core/components/minishop3/src/Services/Catalog/CatalogResolve.php index 43bd4af4a..3c2048596 100644 --- a/core/components/minishop3/src/Services/Catalog/CatalogResolve.php +++ b/core/components/minishop3/src/Services/Catalog/CatalogResolve.php @@ -23,7 +23,8 @@ final class CatalogResolve public static function findUniqueId(modX $modx, string $class, array $criteria): ?int { $query = $modx->newQuery($class, $criteria); - $query->select($modx->getSelectColumns($class, $class, '', ['id'])); + // Table alias for namespaced models is the short class name; FQCN breaks SELECT. + $query->select('id'); $query->limit(2); if (!$query->prepare() || !$query->stmt->execute()) { diff --git a/core/components/minishop3/tests/ProductCatalogRoutesTest.php b/core/components/minishop3/tests/ProductCatalogRoutesTest.php index 51b6c6e38..d83eaaad6 100644 --- a/core/components/minishop3/tests/ProductCatalogRoutesTest.php +++ b/core/components/minishop3/tests/ProductCatalogRoutesTest.php @@ -57,5 +57,16 @@ $fail('ProductCatalogService publicCriteria must enforce hidemenu=0'); } +$resolveSrc = file_get_contents(__DIR__ . '/../src/Services/Catalog/CatalogResolve.php'); +if ($resolveSrc === false) { + $fail('unable to read CatalogResolve.php'); +} +if (!str_contains($resolveSrc, "->select('id')")) { + $fail('CatalogResolve::findUniqueId must select id without FQCN SQL alias'); +} +if (preg_match('/getSelectColumns\s*\(\s*\$class\s*,\s*\$class\b/', $resolveSrc)) { + $fail('CatalogResolve must not pass FQCN as getSelectColumns table alias'); +} + fwrite(STDOUT, "OK ProductCatalogRoutesTest\n"); exit(0); From 61927c79ae169088289af3d7eede223433d535a0 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Mon, 7 Sep 2026 22:09:36 +0600 Subject: [PATCH 3/3] fix(web-api): drop trailing slash on delivery/payment public routes Align publicRoutes with segment matching so delivery/get/{id} and payment/get/{id} stay public after the exact-or-prefix rewrite. --- .../src/Middleware/TokenMiddleware.php | 4 +-- .../tests/TokenMiddlewarePublicRoutesTest.php | 34 +++++++++++++++++-- .../TokenMiddlewarePublicPatternTest.php | 4 +++ 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/core/components/minishop3/src/Middleware/TokenMiddleware.php b/core/components/minishop3/src/Middleware/TokenMiddleware.php index 638ca5c6a..8f1049d74 100644 --- a/core/components/minishop3/src/Middleware/TokenMiddleware.php +++ b/core/components/minishop3/src/Middleware/TokenMiddleware.php @@ -39,9 +39,9 @@ class TokenMiddleware implements MiddlewareInterface '/api/v1/category/get', '/api/v1/category/list', '/api/v1/category/tree', - '/api/v1/delivery/get/', + '/api/v1/delivery/get', '/api/v1/delivery/list', - '/api/v1/payment/get/', + '/api/v1/payment/get', '/api/v1/payment/list', '/api/v1/customer/token/get', '/api/v1/customer/logout', diff --git a/core/components/minishop3/tests/TokenMiddlewarePublicRoutesTest.php b/core/components/minishop3/tests/TokenMiddlewarePublicRoutesTest.php index 8be2d3231..629e44fb0 100644 --- a/core/components/minishop3/tests/TokenMiddlewarePublicRoutesTest.php +++ b/core/components/minishop3/tests/TokenMiddlewarePublicRoutesTest.php @@ -9,6 +9,12 @@ declare(strict_types=1); +require __DIR__ . '/stubs/ModxStub.php'; +require __DIR__ . '/../vendor/autoload.php'; + +use MiniShop3\Middleware\TokenMiddleware; +use MODX\Revolution\modX; + $fail = static function (string $message): never { fwrite(STDERR, "FAIL: {$message}\n"); exit(1); @@ -51,9 +57,9 @@ '/api/v1/category/get', '/api/v1/category/list', '/api/v1/category/tree', - '/api/v1/delivery/get/', + '/api/v1/delivery/get', '/api/v1/delivery/list', - '/api/v1/payment/get/', + '/api/v1/payment/get', '/api/v1/payment/list', '/api/v1/customer/token/get', '/api/v1/health', @@ -112,6 +118,30 @@ $fail('isPublicRoute must match exact path or segment prefix, not blind str_starts_with'); } +if ( + in_array('/api/v1/delivery/get/', $publicRoutes, true) + || in_array('/api/v1/payment/get/', $publicRoutes, true) +) { + $fail('delivery/get and payment/get must not keep a trailing slash (breaks segment match)'); +} + +// Runtime: trailing-slash entries would leave delivery/get/{id} closed (#579 review) +$middleware = new TokenMiddleware(new modX()); +$isPublic = new ReflectionMethod(TokenMiddleware::class, 'isPublicRoute'); +$isPublic->setAccessible(true); + +$_REQUEST['route'] = '/api/v1/delivery/get/5'; +if ($isPublic->invoke($middleware, '/') !== true) { + $fail('delivery/get/{id} must stay public after exact/segment match'); +} + +$_REQUEST['route'] = '/api/v1/payment/get/3'; +if ($isPublic->invoke($middleware, '/') !== true) { + $fail('payment/get/{id} must stay public after exact/segment match'); +} + +unset($_REQUEST['route']); + if ( !str_contains($middlewareSrc, "'ms3_err_token_expired'") && !str_contains($middlewareSrc, '"ms3_err_token_expired"') diff --git a/core/components/minishop3/tests/Unit/Middleware/TokenMiddlewarePublicPatternTest.php b/core/components/minishop3/tests/Unit/Middleware/TokenMiddlewarePublicPatternTest.php index 618608d35..612175a36 100644 --- a/core/components/minishop3/tests/Unit/Middleware/TokenMiddlewarePublicPatternTest.php +++ b/core/components/minishop3/tests/Unit/Middleware/TokenMiddlewarePublicPatternTest.php @@ -50,6 +50,10 @@ public static function routes(): iterable yield 'images query' => ['/api/v1/product/42/images?include_thumbs=1', true]; yield 'filters prefix' => ['/api/v1/product/filters', true]; yield 'list prefix' => ['/api/v1/product/list', true]; + yield 'product get by id' => ['/api/v1/product/get/5', true]; + yield 'delivery get by id' => ['/api/v1/delivery/get/5', true]; + yield 'payment get by id' => ['/api/v1/payment/get/3', true]; + yield 'getting-started not public' => ['/api/v1/product/getting-started', false]; yield 'unknown product sibling' => ['/api/v1/product/42/reviews', false]; yield 'product root' => ['/api/v1/product/42', false]; yield 'nested after images' => ['/api/v1/product/42/images/raw', false];