From 0dd1803c5341fc9b14c828ee92dff4b1437c5595 Mon Sep 17 00:00:00 2001 From: Jelle van Oosterbosch Date: Fri, 22 May 2026 19:44:36 +0200 Subject: [PATCH 1/2] Coerce string query/route scalars against constructor scalar types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HTTP query strings deliver every value as a string, so any list command typed `int $page` / `int $limit` / `float`/ `bool` returned 400 Bad Request when the client sent `?page=1&limit=25` — Symfony's ObjectNormalizer refuses to coerce `"1"` to `int 1` and the bundle surfaces a NotNormalizableValueException as a problem+json response. CommandValueResolver now reflects on the target class's constructor parameters and applies `filter_var(..., FILTER_NULL_ON_FAILURE)` to string payload values whose corresponding parameter is typed `int`/`float`/`bool` (ReflectionNamedType only — unions and intersections fall through untouched, preserving the existing `int|string` workaround pattern). On invalid input (`?page=foo`), the value is left as a string so the denormalizer surfaces its precise "expected int, got string" error rather than silently casting to 0 — keeping the schema-driven strict-validation promise of the bundle. JSON body values pass through untouched via the `is_string($value)` guard — JSON has real types and doesn't need coercion. Coverage: eight data-provider scenarios (int/float/bool happy paths, invalid-int leaves string, string-typed param untouched, union-typed skip, missing key, route-attribute coercion) plus one test for the no-constructor edge case (stdClass). Behavior fix only — no BC break, since GET pagination DTOs simply did not work before this commit. --- .../ArgumentResolver/CommandValueResolver.php | 59 +++++++++ .../CommandValueResolverTest.php | 125 ++++++++++++++++++ 2 files changed, 184 insertions(+) diff --git a/src/Controller/ArgumentResolver/CommandValueResolver.php b/src/Controller/ArgumentResolver/CommandValueResolver.php index 39051d0..9fa799c 100644 --- a/src/Controller/ArgumentResolver/CommandValueResolver.php +++ b/src/Controller/ArgumentResolver/CommandValueResolver.php @@ -14,6 +14,8 @@ namespace Stixx\OpenApiCommandBundle\Controller\ArgumentResolver; use JsonException; +use ReflectionClass; +use ReflectionNamedType; use Stixx\OpenApiCommandBundle\Attribute\CommandObject; use Symfony\Component\HttpFoundation\HeaderUtils; use Symfony\Component\HttpFoundation\Request; @@ -55,6 +57,8 @@ public function resolve(Request $request, ArgumentMetadata $argument): iterable $payload = array_replace($payload, $params); } + $payload = $this->coerceScalarsAgainstConstructor($payload, $type); + yield $this->denormalizeToType($payload, $type); } @@ -149,6 +153,61 @@ private function decodeJsonBodyToArray(Request $request): array return $decoded; } + /** + * Coerces string scalars in `$payload` to the declared `int`/`float`/`bool` types of the + * target class's constructor parameters. HTTP query strings deliver every value as a + * string; without this, Symfony's denormalizer rejects `"1"` for a typed `int $page` + * with `NotNormalizableValueException`. Invalid input (e.g. `?page=foo` against an int + * parameter) is left as a string so the validator surfaces its normal 400 error rather + * than silently coercing to `0`. + * + * @param array $payload + * + * @return array + */ + private function coerceScalarsAgainstConstructor(array $payload, string $type): array + { + if (!class_exists($type)) { + return $payload; + } + + $reflection = new ReflectionClass($type); + $constructor = $reflection->getConstructor(); + if ($constructor === null) { + return $payload; + } + + foreach ($constructor->getParameters() as $parameter) { + $name = $parameter->getName(); + if (!array_key_exists($name, $payload)) { + continue; + } + + $value = $payload[$name]; + if (!is_string($value)) { + continue; + } + + $parameterType = $parameter->getType(); + if (!$parameterType instanceof ReflectionNamedType) { + continue; + } + + $coerced = match ($parameterType->getName()) { + 'int' => filter_var($value, FILTER_VALIDATE_INT, FILTER_NULL_ON_FAILURE), + 'float' => filter_var($value, FILTER_VALIDATE_FLOAT, FILTER_NULL_ON_FAILURE), + 'bool' => filter_var($value, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE), + default => null, + }; + + if ($coerced !== null) { + $payload[$name] = $coerced; + } + } + + return $payload; + } + /** * @param array $data */ diff --git a/tests/Unit/Controller/ArgumentResolver/CommandValueResolverTest.php b/tests/Unit/Controller/ArgumentResolver/CommandValueResolverTest.php index 72d2391..2fbe20b 100644 --- a/tests/Unit/Controller/ArgumentResolver/CommandValueResolverTest.php +++ b/tests/Unit/Controller/ArgumentResolver/CommandValueResolverTest.php @@ -211,6 +211,113 @@ public function testResolveThrowsOnDenormalizationFailure(): void iterator_to_array($this->resolver->resolve($request, $argument)); } + /** + * @param array $expectedPayload + */ + #[DataProvider('provideQueryParameterCoercionCases')] + public function testResolveCoercesQueryScalarsAgainstConstructorTypes(Request $request, array $expectedPayload): void + { + // Arrange — the resolver must coerce string query params (`"1"`, `"true"`, `"1.5"`) + // to the declared scalar constructor parameter types before handing off to the + // denormalizer. Without coercion, Symfony's ObjectNormalizer rejects strings for + // typed scalar properties, producing a 400 on every paginated list endpoint. + $argument = new ArgumentMetadata( + 'command', + CommandValueResolverTestPaginatedCommand::class, + false, + false, + null, + false, + [new CommandObject()] + ); + + $this->serializer->expects($this->once()) + ->method('denormalize') + ->with($expectedPayload, CommandValueResolverTestPaginatedCommand::class) + ->willReturn(new stdClass()); + + // Act + iterator_to_array($this->resolver->resolve($request, $argument)); + + // Assert — expectation verified by mock `with()` matcher. + } + + /** + * @return iterable}> + */ + public static function provideQueryParameterCoercionCases(): iterable + { + yield 'string "1" coerces to int 1 for typed int parameter' => [ + new Request(query: ['page' => '1']), + ['page' => 1], + ]; + + yield 'string "1.5" coerces to float 1.5 for typed float parameter' => [ + new Request(query: ['ratio' => '1.5']), + ['ratio' => 1.5], + ]; + + yield 'string "true" coerces to bool true for typed bool parameter' => [ + new Request(query: ['archived' => 'true']), + ['archived' => true], + ]; + + yield 'string "false" coerces to bool false for typed bool parameter' => [ + new Request(query: ['archived' => 'false']), + ['archived' => false], + ]; + + yield 'invalid int string stays as string so validator surfaces its error' => [ + new Request(query: ['page' => 'not-a-number']), + ['page' => 'not-a-number'], + ]; + + yield 'string "1" stays as string for typed string parameter' => [ + new Request(query: ['status' => '1']), + ['status' => '1'], + ]; + + yield 'union-typed parameter is skipped (int|string stays string)' => [ + new Request(query: ['limit' => '25']), + ['limit' => '25'], + ]; + + yield 'missing constructor parameter key is left alone' => [ + new Request(query: ['unknownParameter' => '1']), + ['unknownParameter' => '1'], + ]; + + yield 'route attribute string coerces to int the same as a query string' => [ + new Request(attributes: ['page' => '7']), + ['page' => 7], + ]; + } + + public function testResolveSkipsCoercionWhenTargetClassHasNoConstructor(): void + { + // Arrange + $request = new Request(query: ['page' => '1']); + $argument = new ArgumentMetadata( + 'command', + stdClass::class, + false, + false, + null, + false, + [new CommandObject()] + ); + + $this->serializer->expects($this->once()) + ->method('denormalize') + ->with(['page' => '1'], stdClass::class) + ->willReturn(new stdClass()); + + // Act + iterator_to_array($this->resolver->resolve($request, $argument)); + + // Assert — expectation verified by mock. + } + /** * @param array $query * @param array $attributes @@ -223,3 +330,21 @@ private static function createJsonRequest(string $content, array $query = [], ar return $request; } } + +/** + * Test-only command class exercising every parameter type variation the coercion + * logic needs to recognise. Kept in the same file to avoid scattering test fixtures. + * + * @internal + */ +final readonly class CommandValueResolverTestPaginatedCommand +{ + public function __construct( + public int $page = 1, + public float $ratio = 0.0, + public bool $archived = false, + public string $status = '', + public int|string $limit = 20, + ) { + } +} From e2ecd807ad067fdf0af7d14a29ef5f0c67757516 Mon Sep 17 00:00:00 2001 From: Jelle van Oosterbosch Date: Fri, 22 May 2026 20:23:40 +0200 Subject: [PATCH 2/2] Trim coerce docblock and cover 0/0.0/negative edge cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docblock on coerceScalarsAgainstConstructor was a six-line paragraph re-stating what the method name and implementation already convey. Reduced to two sentences capturing the non-obvious why (HTTP query parameters always arrive as strings; invalid input deliberately stays as a string). Added five test cases to provideQueryParameterCoercionCases to lock down the strict !== null guard against future regressions to a truthy check: ?page=0 → int 0, ?ratio=0.0 → float 0.0, ?page=-1 → int -1, plus route-attribute equivalents for 0 and -3. These would all silently fall through if the coerced-value check were ever changed to a truthy test (PHP int 0 is falsy). --- .../ArgumentResolver/CommandValueResolver.php | 8 ++---- .../CommandValueResolverTest.php | 25 +++++++++++++++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/Controller/ArgumentResolver/CommandValueResolver.php b/src/Controller/ArgumentResolver/CommandValueResolver.php index 9fa799c..ef932f6 100644 --- a/src/Controller/ArgumentResolver/CommandValueResolver.php +++ b/src/Controller/ArgumentResolver/CommandValueResolver.php @@ -154,12 +154,8 @@ private function decodeJsonBodyToArray(Request $request): array } /** - * Coerces string scalars in `$payload` to the declared `int`/`float`/`bool` types of the - * target class's constructor parameters. HTTP query strings deliver every value as a - * string; without this, Symfony's denormalizer rejects `"1"` for a typed `int $page` - * with `NotNormalizableValueException`. Invalid input (e.g. `?page=foo` against an int - * parameter) is left as a string so the validator surfaces its normal 400 error rather - * than silently coercing to `0`. + * Coerces query/route string scalars to the declared `int`/`float`/`bool` constructor types. + * Invalid input stays as a string so the denormalizer surfaces its normal error. * * @param array $payload * diff --git a/tests/Unit/Controller/ArgumentResolver/CommandValueResolverTest.php b/tests/Unit/Controller/ArgumentResolver/CommandValueResolverTest.php index 2fbe20b..971f713 100644 --- a/tests/Unit/Controller/ArgumentResolver/CommandValueResolverTest.php +++ b/tests/Unit/Controller/ArgumentResolver/CommandValueResolverTest.php @@ -291,6 +291,31 @@ public static function provideQueryParameterCoercionCases(): iterable new Request(attributes: ['page' => '7']), ['page' => 7], ]; + + yield 'zero coerces to int 0 (verifies the strict !== null guard rather than a truthy check)' => [ + new Request(query: ['page' => '0']), + ['page' => 0], + ]; + + yield 'zero-point-zero coerces to float 0.0 (verifies the strict !== null guard)' => [ + new Request(query: ['ratio' => '0.0']), + ['ratio' => 0.0], + ]; + + yield 'negative integer string coerces to a negative int' => [ + new Request(query: ['page' => '-1']), + ['page' => -1], + ]; + + yield 'zero from a route attribute coerces to int 0 the same as from a query string' => [ + new Request(attributes: ['page' => '0']), + ['page' => 0], + ]; + + yield 'negative integer from a route attribute coerces to a negative int' => [ + new Request(attributes: ['page' => '-3']), + ['page' => -3], + ]; } public function testResolveSkipsCoercionWhenTargetClassHasNoConstructor(): void