Skip to content

Commit 6cff3b1

Browse files
nicolas-grekasnicolas-grekas
committed
[HttpKernel] Fix handling of MapRequest* attributes
1 parent 6c28196 commit 6cff3b1

File tree

6 files changed

+127
-33
lines changed

6 files changed

+127
-33
lines changed

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ public function load(array $configs, ContainerBuilder $container)
369369
.(class_exists(Serializer::class) ? 'enabled. Try setting "framework.serializer" to true.' : 'installed. Try running "composer require symfony/serializer-pack".')
370370
);
371371

372-
$container->getDefinition('argument_resolver.request_payload')
372+
$container->getDefinition('request_payload_value_resolver')
373373
->replaceArgument(0, new Reference('.argument_resolver.request_payload.no_serializer', ContainerInterface::RUNTIME_EXCEPTION_ON_INVALID_REFERENCE));
374374

375375
$container->removeDefinition('console.command.serializer_debug');

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/web.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,24 @@
6363
])
6464
->tag('controller.argument_value_resolver', ['priority' => 100, 'name' => DateTimeValueResolver::class])
6565

66-
->set('argument_resolver.request_payload', RequestPayloadValueResolver::class)
66+
->set('request_payload_value_resolver', RequestPayloadValueResolver::class)
6767
->args([
6868
service('serializer'),
6969
service('validator')->nullOnInvalid(),
7070
service('translator')->nullOnInvalid(),
7171
])
72+
->lazy()
73+
74+
->set('argument_resolver.request_payload', RequestPayloadValueResolver::class)
75+
->factory('current')
76+
->args([[service('request_payload_value_resolver')]])
7277
->tag('controller.targeted_value_resolver', ['name' => RequestPayloadValueResolver::class])
7378

79+
->set('request_payload_listener', RequestPayloadValueResolver::class)
80+
->factory('current')
81+
->args([[service('request_payload_value_resolver')]])
82+
->tag('kernel.event_subscriber')
83+
7484
->set('argument_resolver.request_attribute', RequestAttributeValueResolver::class)
7585
->tag('controller.argument_value_resolver', ['priority' => 100, 'name' => RequestAttributeValueResolver::class])
7686

‎src/Symfony/Component/HttpKernel/Attribute/MapQueryString.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\HttpKernel\Attribute;
1313

1414
use Symfony\Component\HttpKernel\Controller\ArgumentResolver\RequestPayloadValueResolver;
15+
use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata;
1516
use Symfony\Component\Validator\Constraints\GroupSequence;
1617

1718
/**
@@ -22,6 +23,8 @@
2223
#[\Attribute(\Attribute::TARGET_PARAMETER)]
2324
class MapQueryString extends ValueResolver
2425
{
26+
public ArgumentMetadata $metadata;
27+
2528
public function __construct(
2629
public readonly array $serializationContext = [],
2730
public readonly string|GroupSequence|array|null $validationGroups = null,

‎src/Symfony/Component/HttpKernel/Attribute/MapRequestPayload.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\HttpKernel\Attribute;
1313

1414
use Symfony\Component\HttpKernel\Controller\ArgumentResolver\RequestPayloadValueResolver;
15+
use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata;
1516
use Symfony\Component\Validator\Constraints\GroupSequence;
1617

1718
/**
@@ -22,6 +23,8 @@
2223
#[\Attribute(\Attribute::TARGET_PARAMETER)]
2324
class MapRequestPayload extends ValueResolver
2425
{
26+
public ArgumentMetadata $metadata;
27+
2528
public function __construct(
2629
public readonly array|string|null $acceptFormat = null,
2730
public readonly array $serializationContext = [],

‎src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,16 @@
1111

1212
namespace Symfony\Component\HttpKernel\Controller\ArgumentResolver;
1313

14+
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
1415
use Symfony\Component\HttpFoundation\Request;
1516
use Symfony\Component\HttpFoundation\Response;
1617
use Symfony\Component\HttpKernel\Attribute\MapQueryString;
1718
use Symfony\Component\HttpKernel\Attribute\MapRequestPayload;
1819
use Symfony\Component\HttpKernel\Controller\ValueResolverInterface;
1920
use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata;
21+
use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent;
2022
use Symfony\Component\HttpKernel\Exception\HttpException;
23+
use Symfony\Component\HttpKernel\KernelEvents;
2124
use Symfony\Component\Serializer\Exception\NotEncodableValueException;
2225
use Symfony\Component\Serializer\Exception\PartialDenormalizationException;
2326
use Symfony\Component\Serializer\Exception\UnsupportedFormatException;
@@ -31,8 +34,10 @@
3134

3235
/**
3336
* @author Konstantin Myakshin <molodchick@gmail.com>
37+
*
38+
* @final
3439
*/
35-
final class RequestPayloadValueResolver implements ValueResolverInterface
40+
class RequestPayloadValueResolver implements ValueResolverInterface, EventSubscriberInterface
3641
{
3742
/**
3843
* @see \Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::DISABLE_TYPE_ENFORCEMENT
@@ -59,24 +64,43 @@ public function __construct(
5964

6065
public function resolve(Request $request, ArgumentMetadata $argument): iterable
6166
{
62-
$payloadMappers = [
63-
MapQueryString::class => ['mapQueryString', Response::HTTP_NOT_FOUND],
64-
MapRequestPayload::class => ['mapRequestPayload', Response::HTTP_UNPROCESSABLE_ENTITY],
65-
];
67+
$attribute = $argument->getAttributesOfType(MapQueryString::class, ArgumentMetadata::IS_INSTANCEOF)[0]
68+
?? $argument->getAttributesOfType(MapRequestPayload::class, ArgumentMetadata::IS_INSTANCEOF)[0]
69+
?? null;
70+
71+
if (!$attribute) {
72+
return [];
73+
}
74+
75+
$attribute->metadata = $argument;
76+
77+
return [$attribute];
78+
}
6679

67-
foreach ($payloadMappers as $mappingAttribute => [$payloadMapper, $validationFailedCode]) {
68-
if (!$attributes = $argument->getAttributesOfType($mappingAttribute, ArgumentMetadata::IS_INSTANCEOF)) {
80+
public function onKernelControllerArguments(ControllerArgumentsEvent $event): void
81+
{
82+
$arguments = $event->getArguments();
83+
84+
foreach ($arguments as $i => $argument) {
85+
if ($argument instanceof MapQueryString) {
86+
$payloadMapper = 'mapQueryString';
87+
$validationFailedCode = Response::HTTP_NOT_FOUND;
88+
} elseif ($argument instanceof MapRequestPayload) {
89+
$payloadMapper = 'mapRequestPayload';
90+
$validationFailedCode = Response::HTTP_UNPROCESSABLE_ENTITY;
91+
} else {
6992
continue;
7093
}
94+
$request = $event->getRequest();
7195

72-
if (!$type = $argument->getType()) {
73-
throw new \LogicException(sprintf('Could not resolve the "$%s" controller argument: argument should be typed.', $argument->getName()));
96+
if (!$type = $argument->metadata->getType()) {
97+
throw new \LogicException(sprintf('Could not resolve the "$%s" controller argument: argument should be typed.', $argument->metadata->getName()));
7498
}
7599

76100
if ($this->validator) {
77101
$violations = new ConstraintViolationList();
78102
try {
79-
$payload = $this->$payloadMapper($request, $type, $attributes[0]);
103+
$payload = $this->$payloadMapper($request, $type, $argument);
80104
} catch (PartialDenormalizationException $e) {
81105
$trans = $this->translator ? $this->translator->trans(...) : fn ($m, $p) => strtr($m, $p);
82106
foreach ($e->getErrors() as $error) {
@@ -92,26 +116,35 @@ public function resolve(Request $request, ArgumentMetadata $argument): iterable
92116
}
93117

94118
if (null !== $payload) {
95-
$violations->addAll($this->validator->validate($payload, null, $attributes[0]->validationGroups ?? null));
119+
$violations->addAll($this->validator->validate($payload, null, $argument->validationGroups ?? null));
96120
}
97121

98122
if (\count($violations)) {
99123
throw new HttpException($validationFailedCode, implode("\n", array_map(static fn ($e) => $e->getMessage(), iterator_to_array($violations))), new ValidationFailedException($payload, $violations));
100124
}
101125
} else {
102126
try {
103-
$payload = $this->$payloadMapper($request, $type, $attributes[0]);
127+
$payload = $this->$payloadMapper($request, $argument);
104128
} catch (PartialDenormalizationException $e) {
105129
throw new HttpException($validationFailedCode, implode("\n", array_map(static fn ($e) => $e->getMessage(), $e->getErrors())), $e);
106130
}
107131
}
108132

109-
if (null !== $payload || $argument->isNullable()) {
110-
return [$payload];
133+
if (null === $payload && !$argument->metadata->isNullable()) {
134+
throw new HttpException($validationFailedCode);
111135
}
136+
137+
$arguments[$i] = $payload;
112138
}
113139

114-
return [];
140+
$event->setArguments($arguments);
141+
}
142+
143+
public static function getSubscribedEvents(): array
144+
{
145+
return [
146+
KernelEvents::CONTROLLER_ARGUMENTS => 'onKernelControllerArguments',
147+
];
115148
}
116149

117150
private function mapQueryString(Request $request, string $type, MapQueryString $attribute): ?object

‎src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/RequestPayloadValueResolverTest.php

Lines changed: 61 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@
1818
use Symfony\Component\HttpKernel\Attribute\ValueResolver;
1919
use Symfony\Component\HttpKernel\Controller\ArgumentResolver\RequestPayloadValueResolver;
2020
use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata;
21+
use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent;
2122
use Symfony\Component\HttpKernel\Exception\HttpException;
23+
use Symfony\Component\HttpKernel\HttpKernelInterface;
24+
use Symfony\Component\HttpKernel\KernelInterface;
2225
use Symfony\Component\Serializer\Encoder\JsonEncoder;
2326
use Symfony\Component\Serializer\Encoder\XmlEncoder;
2427
use Symfony\Component\Serializer\Exception\PartialDenormalizationException;
@@ -63,8 +66,12 @@ public function testWithoutValidatorAndCouldNotDenormalize()
6366
]);
6467
$request = Request::create('/', 'POST', server: ['CONTENT_TYPE' => 'application/json'], content: $content);
6568

69+
$kernel = $this->createMock(HttpKernelInterface::class);
70+
$arguments = $resolver->resolve($request, $argument);
71+
$event = new ControllerArgumentsEvent($kernel, function () {}, $arguments, $request, HttpKernelInterface::MAIN_REQUEST);
72+
6673
try {
67-
$resolver->resolve($request, $argument);
74+
$resolver->onKernelControllerArguments($event);
6875
$this->fail(sprintf('Expected "%s" to be thrown.', HttpException::class));
6976
} catch (HttpException $e) {
7077
$this->assertInstanceOf(PartialDenormalizationException::class, $e->getPrevious());
@@ -90,8 +97,12 @@ public function testValidationNotPassed()
9097
]);
9198
$request = Request::create('/', 'POST', server: ['CONTENT_TYPE' => 'application/json'], content: $content);
9299

100+
$kernel = $this->createMock(HttpKernelInterface::class);
101+
$arguments = $resolver->resolve($request, $argument);
102+
$event = new ControllerArgumentsEvent($kernel, function () {}, $arguments, $request, HttpKernelInterface::MAIN_REQUEST);
103+
93104
try {
94-
$resolver->resolve($request, $argument);
105+
$resolver->onKernelControllerArguments($event);
95106
$this->fail(sprintf('Expected "%s" to be thrown.', HttpException::class));
96107
} catch (HttpException $e) {
97108
$validationFailedException = $e->getPrevious();
@@ -112,9 +123,12 @@ public function testUnsupportedMedia()
112123
]);
113124
$request = Request::create('/', 'POST', server: ['CONTENT_TYPE' => 'foo/bar'], content: 'foo-bar');
114125

115-
try {
116-
$resolver->resolve($request, $argument);
126+
$kernel = $this->createMock(HttpKernelInterface::class);
127+
$arguments = $resolver->resolve($request, $argument);
128+
$event = new ControllerArgumentsEvent($kernel, function () {}, $arguments, $request, HttpKernelInterface::MAIN_REQUEST);
117129

130+
try {
131+
$resolver->onKernelControllerArguments($event);
118132
$this->fail(sprintf('Expected "%s" to be thrown.', HttpException::class));
119133
} catch (HttpException $e) {
120134
$this->assertSame(415, $e->getStatusCode());
@@ -139,7 +153,13 @@ public function testRequestContentValidationPassed()
139153
]);
140154
$request = Request::create('/', 'POST', server: ['CONTENT_TYPE' => 'application/json'], content: $content);
141155

142-
$this->assertEquals($payload, $resolver->resolve($request, $argument)[0]);
156+
$kernel = $this->createMock(HttpKernelInterface::class);
157+
$arguments = $resolver->resolve($request, $argument);
158+
$event = new ControllerArgumentsEvent($kernel, function () {}, $arguments, $request, HttpKernelInterface::MAIN_REQUEST);
159+
160+
$resolver->onKernelControllerArguments($event);
161+
162+
$this->assertEquals([$payload], $event->getArguments());
143163
}
144164

145165
public function testQueryStringValidationPassed()
@@ -161,7 +181,13 @@ public function testQueryStringValidationPassed()
161181
]);
162182
$request = Request::create('/', 'GET', $query);
163183

164-
$this->assertEquals($payload, $resolver->resolve($request, $argument)[0]);
184+
$kernel = $this->createMock(HttpKernelInterface::class);
185+
$arguments = $resolver->resolve($request, $argument);
186+
$event = new ControllerArgumentsEvent($kernel, function () {}, $arguments, $request, HttpKernelInterface::MAIN_REQUEST);
187+
188+
$resolver->onKernelControllerArguments($event);
189+
190+
$this->assertEquals([$payload], $event->getArguments());
165191
}
166192

167193
public function testRequestInputValidationPassed()
@@ -183,7 +209,13 @@ public function testRequestInputValidationPassed()
183209
]);
184210
$request = Request::create('/', 'POST', $input);
185211

186-
$this->assertEquals($payload, $resolver->resolve($request, $argument)[0]);
212+
$kernel = $this->createMock(HttpKernelInterface::class);
213+
$arguments = $resolver->resolve($request, $argument);
214+
$event = new ControllerArgumentsEvent($kernel, function () {}, $arguments, $request, HttpKernelInterface::MAIN_REQUEST);
215+
216+
$resolver->onKernelControllerArguments($event);
217+
218+
$this->assertEquals([$payload], $event->getArguments());
187219
}
188220

189221
/**
@@ -202,10 +234,13 @@ public function testAcceptFormatPassed(mixed $acceptFormat, string $contentType,
202234
MapRequestPayload::class => new MapRequestPayload(acceptFormat: $acceptFormat),
203235
]);
204236

205-
$resolved = $resolver->resolve($request, $argument);
237+
$kernel = $this->createMock(HttpKernelInterface::class);
238+
$arguments = $resolver->resolve($request, $argument);
239+
$event = new ControllerArgumentsEvent($kernel, function () {}, $arguments, $request, HttpKernelInterface::MAIN_REQUEST);
206240

207-
$this->assertCount(1, $resolved);
208-
$this->assertEquals(new RequestPayload(50), $resolved[0]);
241+
$resolver->onKernelControllerArguments($event);
242+
243+
$this->assertEquals([new RequestPayload(50)], $event->getArguments());
209244
}
210245

211246
public static function provideMatchedFormatContext(): iterable
@@ -262,9 +297,12 @@ public function testAcceptFormatNotPassed(mixed $acceptFormat, string $contentTy
262297
MapRequestPayload::class => new MapRequestPayload(acceptFormat: $acceptFormat),
263298
]);
264299

265-
try {
266-
$resolver->resolve($request, $argument);
300+
$kernel = $this->createMock(HttpKernelInterface::class);
301+
$arguments = $resolver->resolve($request, $argument);
302+
$event = new ControllerArgumentsEvent($kernel, function () {}, $arguments, $request, HttpKernelInterface::MAIN_REQUEST);
267303

304+
try {
305+
$resolver->onKernelControllerArguments($event);
268306
$this->fail(sprintf('Expected "%s" to be thrown.', HttpException::class));
269307
} catch (HttpException $e) {
270308
$this->assertSame(415, $e->getStatusCode());
@@ -330,10 +368,13 @@ public function testValidationGroupsPassed(string $method, ValueResolver $attrib
330368
$attribute::class => $attribute,
331369
]);
332370

333-
$resolved = $resolver->resolve($request, $argument);
371+
$kernel = $this->createMock(HttpKernelInterface::class);
372+
$arguments = $resolver->resolve($request, $argument);
373+
$event = new ControllerArgumentsEvent($kernel, function () {}, $arguments, $request, HttpKernelInterface::MAIN_REQUEST);
374+
375+
$resolver->onKernelControllerArguments($event);
334376

335-
$this->assertCount(1, $resolved);
336-
$this->assertEquals($payload, $resolved[0]);
377+
$this->assertEquals([$payload], $event->getArguments());
337378
}
338379

339380
/**
@@ -352,8 +393,12 @@ public function testValidationGroupsNotPassed(string $method, ValueResolver $att
352393
]);
353394
$request = Request::create('/', $method, $input);
354395

396+
$kernel = $this->createMock(HttpKernelInterface::class);
397+
$arguments = $resolver->resolve($request, $argument);
398+
$event = new ControllerArgumentsEvent($kernel, function () {}, $arguments, $request, HttpKernelInterface::MAIN_REQUEST);
399+
355400
try {
356-
$resolver->resolve($request, $argument);
401+
$resolver->onKernelControllerArguments($event);
357402
$this->fail(sprintf('Expected "%s" to be thrown.', HttpException::class));
358403
} catch (HttpException $e) {
359404
$validationFailedException = $e->getPrevious();

0 commit comments

Comments
 (0)