From 97833e95ab751d4635a885df0b3982de5620c784 Mon Sep 17 00:00:00 2001 From: Jelle van Oosterbosch Date: Sun, 6 Sep 2026 22:19:19 +0200 Subject: [PATCH 1/3] Validate a request against its own Nelmio area, not always the default NelmioAreaRoutesChecker::isApiRoute() accepts a route belonging to any registered area, since CollectNelmioApiDocRoutesPass collects routes and path_patterns for all of them. But config/validators.php wired RequestValidator to nelmio_api_doc.generator.default, so every request was validated against the default area's document regardless of which area it belonged to. In a multi-area application a request to a non-default area passed route detection, was validated against a document with no such path, raised NoPath, and surfaced as a 400 openapi_request_validation on a valid request. Single-area applications are unaffected, since the only area is `default`. NelmioAreaRoutesChecker now exposes areaFor(); isApiRoute() is kept as `null !== areaFor()`. The compiler pass collects the per-area generators alongside the routes, using Nelmio's nelmio_api_doc.generator. naming. RequestValidator resolves the area and memoises one validator per area. Both new constructor arguments are optional and default to null; without them the validator behaves as before. RequestValidator is final and @internal, and the @api ValidatorInterface is unchanged. Adds a CHANGELOG entry under Unreleased and documents the per-area behaviour in docs/validation.md. --- CHANGELOG.md | 7 ++ config/validators.php | 3 + docs/validation.md | 2 + .../CollectNelmioApiDocRoutesPass.php | 12 ++ src/Routing/NelmioAreaRoutesChecker.php | 29 +++-- src/Validator/RequestValidator.php | 33 ++++-- tests/Unit/Validator/RequestValidatorTest.php | 108 ++++++++++++++++++ 7 files changed, 178 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 197745d..899c3e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ may contain breaking changes; read the **Upgrading** notes before bumping a mino ## [Unreleased] +### Fixed + +- Requests are validated against the OpenAPI document of **their own** Nelmio area. Previously every + request was validated against the `default` area's document, while route detection accepted a route + belonging to any area — so in a multi-area application a valid request to a non-default area was + rejected with a `400` `openapi_request_validation`. Single-area applications are unaffected. + ## [0.12.4] - 2026-08-30 Documentation only — no code changes since `0.12.3`. diff --git a/config/validators.php b/config/validators.php index c7083e6..e3e925c 100644 --- a/config/validators.php +++ b/config/validators.php @@ -3,6 +3,7 @@ declare(strict_types=1); use Nyholm\Psr7\Factory\Psr17Factory; +use Stixx\OpenApiCommandBundle\Routing\NelmioAreaRoutesChecker; use Stixx\OpenApiCommandBundle\Validator\RequestValidator; use Stixx\OpenApiCommandBundle\Validator\RequestValidatorChain; use Stixx\OpenApiCommandBundle\Validator\ValidatorInterface as StixxValidatorInterface; @@ -37,5 +38,7 @@ ->set(RequestValidator::class) ->arg('$apiDocGenerator', service('nelmio_api_doc.generator.default')) ->arg('$psrHttpFactory', service('stixx_openapi_command.psr_http_factory')) + ->arg('$generatorsLocator', service('stixx_openapi_command.nelmio.generators_locator')) + ->arg('$areaRoutesChecker', service(NelmioAreaRoutesChecker::class)) ->tag(StixxValidatorInterface::TAG_NAME); }; diff --git a/docs/validation.md b/docs/validation.md index 3b9b26e..0e29beb 100644 --- a/docs/validation.md +++ b/docs/validation.md @@ -161,6 +161,8 @@ services: All tagged validators are executed in a chain during the `kernel.request` event, but only for routes that are managed by this bundle (detected via `NelmioAreaRoutesChecker`). If any validator throws an exception, the request cycle is interrupted. +When several Nelmio areas are configured, each has its own OpenAPI document. `NelmioAreaRoutesChecker` resolves the area a request belongs to, and the request is validated against that area's document. Areas are checked in registration order and the first match wins. + --- ## Customizing Error Responses diff --git a/src/DependencyInjection/Compiler/CollectNelmioApiDocRoutesPass.php b/src/DependencyInjection/Compiler/CollectNelmioApiDocRoutesPass.php index 963d1e9..bfae883 100644 --- a/src/DependencyInjection/Compiler/CollectNelmioApiDocRoutesPass.php +++ b/src/DependencyInjection/Compiler/CollectNelmioApiDocRoutesPass.php @@ -36,6 +36,8 @@ public function process(ContainerBuilder $container): void $routesMap = []; $pathPatterns = []; + $generatorsMap = []; + foreach ($areas as $area) { $serviceId = sprintf('nelmio_api_doc.routes.%s', $area); @@ -45,6 +47,11 @@ public function process(ContainerBuilder $container): void $routesMap[$area] = new Reference($serviceId); $pathPatterns[$area] = $this->extractPathPatterns($container, $serviceId); + + $generatorId = sprintf('nelmio_api_doc.generator.%s', $area); + if ($container->has($generatorId)) { + $generatorsMap[$area] = new Reference($generatorId); + } } $container->register('stixx_openapi_command.nelmio.routes_locator', ServiceLocator::class) @@ -52,6 +59,11 @@ public function process(ContainerBuilder $container): void ->setPublic(false) ->setArguments([$routesMap]); + $container->register('stixx_openapi_command.nelmio.generators_locator', ServiceLocator::class) + ->addTag('container.service_locator') + ->setPublic(false) + ->setArguments([$generatorsMap]); + $container->setParameter('stixx_openapi_command.nelmio.path_patterns', $pathPatterns); } diff --git a/src/Routing/NelmioAreaRoutesChecker.php b/src/Routing/NelmioAreaRoutesChecker.php index cf3cbf5..3aa6aa1 100644 --- a/src/Routing/NelmioAreaRoutesChecker.php +++ b/src/Routing/NelmioAreaRoutesChecker.php @@ -33,10 +33,21 @@ public function __construct( } public function isApiRoute(Request $request): bool + { + return null !== $this->areaFor($request); + } + + /** + * The Nelmio area a request belongs to, or null if none. Areas are checked in registration order. + */ + public function areaFor(Request $request): ?string { $routeName = $request->attributes->get('_route', ''); - if (is_string($routeName) && $routeName !== '' && $this->matchesByRouteName($routeName)) { - return true; + if (is_string($routeName) && $routeName !== '') { + $area = $this->matchesByRouteName($routeName); + if (null !== $area) { + return $area; + } } // Symfony does not set _route when the path doesn't match any route (404) or when no method @@ -45,7 +56,7 @@ public function isApiRoute(Request $request): bool return $this->matchesByPath($request->getPathInfo()); } - private function matchesByRouteName(string $routeName): bool + private function matchesByRouteName(string $routeName): ?string { foreach (array_keys($this->routesLocator->getProvidedServices()) as $area) { $routeCollection = $this->routesLocator->get($area); @@ -54,23 +65,23 @@ private function matchesByRouteName(string $routeName): bool } if (null !== $routeCollection->get($routeName)) { - return true; + return $area; } } - return false; + return null; } - private function matchesByPath(string $path): bool + private function matchesByPath(string $path): ?string { - foreach ($this->pathPatterns as $patterns) { + foreach ($this->pathPatterns as $area => $patterns) { foreach ($patterns as $pattern) { if (preg_match('{'.$pattern.'}', $path) === 1) { - return true; + return $area; } } } - return false; + return null; } } diff --git a/src/Validator/RequestValidator.php b/src/Validator/RequestValidator.php index efabd5f..d87a7ce 100644 --- a/src/Validator/RequestValidator.php +++ b/src/Validator/RequestValidator.php @@ -16,7 +16,9 @@ use League\OpenAPIValidation\PSR7\RequestValidator as OpenApiRequestValidator; use League\OpenAPIValidation\PSR7\ValidatorBuilder; use Nelmio\ApiDocBundle\ApiDocGenerator; +use Stixx\OpenApiCommandBundle\Routing\NelmioAreaRoutesChecker; use Symfony\Bridge\PsrHttpMessage\HttpMessageFactoryInterface; +use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\HttpFoundation\Request; /** @@ -24,28 +26,45 @@ */ final class RequestValidator implements ValidatorInterface { - private ?OpenApiRequestValidator $cachedValidator = null; + /** @var array */ + private array $cachedValidators = []; + /** + * @param ServiceLocator|null $generatorsLocator + */ public function __construct( private readonly ApiDocGenerator $apiDocGenerator, private readonly HttpMessageFactoryInterface $psrHttpFactory, + private readonly ?ServiceLocator $generatorsLocator = null, + private readonly ?NelmioAreaRoutesChecker $areaRoutesChecker = null, ) { } public function validate(Request $request): void { $psrRequest = $this->psrHttpFactory->createRequest($request); - $this->getValidator()->validate($psrRequest); + $this->getValidator($this->areaFor($request))->validate($psrRequest); } - private function getValidator(): OpenApiRequestValidator + private function areaFor(Request $request): string { - if ($this->cachedValidator !== null) { - return $this->cachedValidator; + return $this->areaRoutesChecker?->areaFor($request) ?? 'default'; + } + + private function getValidator(string $area): OpenApiRequestValidator + { + if (isset($this->cachedValidators[$area])) { + return $this->cachedValidators[$area]; } - $apiDoc = $this->apiDocGenerator->generate(); + $generator = $this->generatorsLocator?->has($area) === true + ? $this->generatorsLocator->get($area) + : $this->apiDocGenerator; + + $apiDoc = $generator->generate(); - return $this->cachedValidator = new ValidatorBuilder()->fromJson($apiDoc->toJson())->getRequestValidator(); + return $this->cachedValidators[$area] = new ValidatorBuilder() + ->fromJson($apiDoc->toJson()) + ->getRequestValidator(); } } diff --git a/tests/Unit/Validator/RequestValidatorTest.php b/tests/Unit/Validator/RequestValidatorTest.php index fccbf73..325f3b6 100644 --- a/tests/Unit/Validator/RequestValidatorTest.php +++ b/tests/Unit/Validator/RequestValidatorTest.php @@ -26,9 +26,13 @@ use OpenApi\Annotations\Schema; use OpenApi\Context; use PHPUnit\Framework\TestCase; +use Stixx\OpenApiCommandBundle\Routing\NelmioAreaRoutesChecker; use Stixx\OpenApiCommandBundle\Validator\RequestValidator; use Symfony\Bridge\PsrHttpMessage\Factory\PsrHttpFactory; +use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Routing\Route; +use Symfony\Component\Routing\RouteCollection; final class RequestValidatorTest extends TestCase { @@ -109,6 +113,56 @@ public function describe(OpenApi $api): void self::assertSame(1, $describer->describeCalls); } + public function testValidatesAgainstTheRequestsOwnAreaNotTheDefaultArea(): void + { + // Arrange + $defaultGenerator = new ApiDocGenerator([$this->createDescriberForPath('/default')], []); + $internalGenerator = new ApiDocGenerator([$this->createDescriberForPath('/internal')], []); + + $request = Request::create('/internal', 'POST'); + $request->attributes->set('_route', 'internal_route'); + + $validator = new RequestValidator( + $defaultGenerator, + $this->createPsrHttpFactory(), + $this->createGeneratorsLocator($defaultGenerator, $internalGenerator), + $this->createAreaChecker(['default' => '/default', 'internal' => '/internal']), + ); + + // Act + $validator->validate($request); + + // Assert: /internal exists only in the internal area's document; the default document would raise NoPath. + $this->expectNotToPerformAssertions(); + } + + public function testKeepsASeparateValidatorPerArea(): void + { + // Arrange + $defaultGenerator = new ApiDocGenerator([$this->createDescriberForPath('/default')], []); + $internalGenerator = new ApiDocGenerator([$this->createDescriberForPath('/internal')], []); + + $validator = new RequestValidator( + $defaultGenerator, + $this->createPsrHttpFactory(), + $this->createGeneratorsLocator($defaultGenerator, $internalGenerator), + $this->createAreaChecker(['default' => '/default', 'internal' => '/internal']), + ); + + $internal = Request::create('/internal', 'POST'); + $internal->attributes->set('_route', 'internal_route'); + + $default = Request::create('/default', 'POST'); + $default->attributes->set('_route', 'default_route'); + + // Act: the internal area first, so its validator is memoised before the default area is used. + $validator->validate($internal); + $validator->validate($default); + + // Assert: a shared memo would validate /default against the internal document and raise NoPath. + $this->expectNotToPerformAssertions(); + } + private function createPsrHttpFactory(): PsrHttpFactory { $psr17 = new Psr17Factory(); @@ -149,4 +203,58 @@ public function describe(OpenApi $api): void } }; } + + private function createDescriberForPath(string $path): DescriberInterface + { + return new class ($path) implements DescriberInterface { + public function __construct(private readonly string $path) + { + } + + public function describe(OpenApi $api): void + { + $api->info = new Info(['title' => 'Test', 'version' => '1.0.0', '_context' => new Context(['version' => '3.0.0'], null)]); + $api->paths = [ + new PathItem([ + 'path' => $this->path, + 'post' => new Post([ + 'responses' => [new Response(['response' => 200, 'description' => 'ok', '_context' => new Context(['version' => '3.0.0'], null)])], + '_context' => new Context(['version' => '3.0.0'], null), + ]), + '_context' => new Context(['version' => '3.0.0'], null), + ]), + ]; + } + }; + } + + /** @return ServiceLocator */ + private function createGeneratorsLocator(ApiDocGenerator $default, ApiDocGenerator $internal): ServiceLocator + { + /** @var ServiceLocator $locator */ + $locator = new ServiceLocator([ + 'default' => static fn (): ApiDocGenerator => $default, + 'internal' => static fn (): ApiDocGenerator => $internal, + ]); + + return $locator; + } + + /** @param array $areaToPath */ + private function createAreaChecker(array $areaToPath): NelmioAreaRoutesChecker + { + $routes = []; + $patterns = []; + foreach ($areaToPath as $area => $path) { + $collection = new RouteCollection(); + $collection->add($area.'_route', new Route($path)); + $routes[$area] = static fn (): RouteCollection => $collection; + $patterns[$area] = ['^'.preg_quote($path, '{}')]; + } + + /** @var ServiceLocator $locator */ + $locator = new ServiceLocator($routes); + + return new NelmioAreaRoutesChecker($locator, $patterns); + } } From e44c4de069a798842271164c7774e5755901391e Mon Sep 17 00:00:00 2001 From: Jelle van Oosterbosch Date: Sun, 6 Sep 2026 22:29:31 +0200 Subject: [PATCH 2/3] Assert the generators_locator wiring, and handle numeric area names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the new wiring was pinned by nothing: CollectNelmioApiDocRoutesPass built the generator service id from a string literal that no test asserted. Replacing `nelmio_api_doc.generator.%s` with a typo left the whole suite green — 226 tests passing with the fix silently disabled, every area falling back to the default document, which is the bug this PR exists to remove. The two new RequestValidator tests could not catch it because they hand-build the locator themselves. CollectNelmioApiDocRoutesPassTest now asserts the generators_locator map the same way it already asserts the routes_locator one. The same typo now fails. Also casts area names to string. Symfony's YAML parser makes a numeric area key an int, and areaFor() declares ?string, so `areas: { default: ..., 2024: ... }` hit a TypeError on the 404 path. The ServiceLocator::get() call had the same problem before this PR; both are covered by the cast. --- src/Routing/NelmioAreaRoutesChecker.php | 6 +++--- .../Compiler/CollectNelmioApiDocRoutesPassTest.php | 13 +++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/Routing/NelmioAreaRoutesChecker.php b/src/Routing/NelmioAreaRoutesChecker.php index 3aa6aa1..4867369 100644 --- a/src/Routing/NelmioAreaRoutesChecker.php +++ b/src/Routing/NelmioAreaRoutesChecker.php @@ -59,13 +59,13 @@ public function areaFor(Request $request): ?string private function matchesByRouteName(string $routeName): ?string { foreach (array_keys($this->routesLocator->getProvidedServices()) as $area) { - $routeCollection = $this->routesLocator->get($area); + $routeCollection = $this->routesLocator->get((string) $area); if (!$routeCollection instanceof RouteCollection) { continue; } if (null !== $routeCollection->get($routeName)) { - return $area; + return (string) $area; } } @@ -77,7 +77,7 @@ private function matchesByPath(string $path): ?string foreach ($this->pathPatterns as $area => $patterns) { foreach ($patterns as $pattern) { if (preg_match('{'.$pattern.'}', $path) === 1) { - return $area; + return (string) $area; } } } diff --git a/tests/Unit/DependencyInjection/Compiler/CollectNelmioApiDocRoutesPassTest.php b/tests/Unit/DependencyInjection/Compiler/CollectNelmioApiDocRoutesPassTest.php index 9582b8d..41eb9f0 100644 --- a/tests/Unit/DependencyInjection/Compiler/CollectNelmioApiDocRoutesPassTest.php +++ b/tests/Unit/DependencyInjection/Compiler/CollectNelmioApiDocRoutesPassTest.php @@ -39,6 +39,8 @@ public function testProcessWithAreas(): void $container->setDefinition('nelmio_api_doc.routes.default', new Definition(stdClass::class)); // 'nelmio_api_doc.routes.internal' is missing on purpose — it should be skipped silently. + $container->setDefinition('nelmio_api_doc.generator.default', new Definition(stdClass::class)); + $pass = new CollectNelmioApiDocRoutesPass(); // Act @@ -56,6 +58,17 @@ public function testProcessWithAreas(): void ]; self::assertEquals([$expectedMap], $definition->getArguments()); + self::assertTrue($container->hasDefinition('stixx_openapi_command.nelmio.generators_locator')); + $generatorsDefinition = $container->getDefinition('stixx_openapi_command.nelmio.generators_locator'); + + self::assertSame(ServiceLocator::class, $generatorsDefinition->getClass()); + self::assertTrue($generatorsDefinition->hasTag('container.service_locator')); + + $expectedGeneratorsMap = [ + 'default' => new Reference('nelmio_api_doc.generator.default'), + ]; + self::assertEquals([$expectedGeneratorsMap], $generatorsDefinition->getArguments()); + self::assertTrue($container->hasParameter('stixx_openapi_command.nelmio.path_patterns')); self::assertSame( ['default' => []], From a8cb4c1135e1f3e3f0d566d9c6994f80a17b1f7b Mon Sep 17 00:00:00 2001 From: Jelle van Oosterbosch Date: Sun, 6 Sep 2026 22:36:58 +0200 Subject: [PATCH 3/3] Release the changelog entry as 0.13.1 An entry under [Unreleased] does not ship inside the tag: it only ever appears on the default branch, so an installed 0.13.1 would document nothing. 0.12.3 and earlier hit this. Name the version up front instead of renaming it in a follow-up release commit. Also corrects the Unreleased compare link, which still pointed at 0.12.4 despite 0.13.0 having been tagged. --- CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 899c3e5..f5c83f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,12 +8,16 @@ may contain breaking changes; read the **Upgrading** notes before bumping a mino ## [Unreleased] +## [0.13.1] - 2026-09-06 + ### Fixed - Requests are validated against the OpenAPI document of **their own** Nelmio area. Previously every request was validated against the `default` area's document, while route detection accepted a route belonging to any area — so in a multi-area application a valid request to a non-default area was rejected with a `400` `openapi_request_validation`. Single-area applications are unaffected. +- An area whose name is numeric (`2024`) no longer raises a `TypeError` during route lookup. PHP stores + such a key as an `int`, which `ServiceLocator::get(string $id)` rejects under `strict_types`. ## [0.12.4] - 2026-08-30 @@ -215,7 +219,8 @@ Installing it with Composer is not enough. See the README's installation section details outside debug mode, and a `CommandValueResolver` that supports list endpoints and the combination of parameters with a request body. -[Unreleased]: https://github.com/stixx/openapi-command-bundle/compare/0.12.4...HEAD +[Unreleased]: https://github.com/stixx/openapi-command-bundle/compare/0.13.1...HEAD +[0.13.1]: https://github.com/stixx/openapi-command-bundle/compare/0.13.0...0.13.1 [0.12.4]: https://github.com/stixx/openapi-command-bundle/compare/0.12.3...0.12.4 [0.12.3]: https://github.com/stixx/openapi-command-bundle/compare/0.12.2...0.12.3 [0.12.2]: https://github.com/stixx/openapi-command-bundle/compare/0.12.1...0.12.2