Skip to content

Commit a97336b

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

File tree

6 files changed

+111
-39
lines changed

6 files changed

+111
-39
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,8 @@ public function load(array $configs, ContainerBuilder $container)
370370
);
371371

372372
$container->getDefinition('argument_resolver.request_payload')
373-
->replaceArgument(0, new Reference('.argument_resolver.request_payload.no_serializer', ContainerInterface::RUNTIME_EXCEPTION_ON_INVALID_REFERENCE));
373+
->replaceArgument(0, new Reference('.argument_resolver.request_payload.no_serializer', ContainerInterface::RUNTIME_EXCEPTION_ON_INVALID_REFERENCE))
374+
->clearTag('kernel.event_listener');
374375

375376
$container->removeDefinition('console.command.serializer_debug');
376377
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
service('translator')->nullOnInvalid(),
7171
])
7272
->tag('controller.targeted_value_resolver', ['name' => RequestPayloadValueResolver::class])
73+
->tag('kernel.event_listener', ['event' => 'kernel.controller_arguments', 'method' => 'onKernelControllerArguments'])
7374

7475
->set('argument_resolver.request_attribute', RequestAttributeValueResolver::class)
7576
->tag('controller.argument_value_resolver', ['priority' => 100, 'name' => RequestAttributeValueResolver::class])

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
#[\Attribute(\Attribute::TARGET_PARAMETER)]
2323
class MapQueryString extends ValueResolver
2424
{
25+
public string $name;
26+
public string $type;
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
@@ -22,6 +22,9 @@
2222
#[\Attribute(\Attribute::TARGET_PARAMETER)]
2323
class MapRequestPayload extends ValueResolver
2424
{
25+
public string $name;
26+
public string $type;
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: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Symfony\Component\HttpKernel\Attribute\MapRequestPayload;
1818
use Symfony\Component\HttpKernel\Controller\ValueResolverInterface;
1919
use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata;
20+
use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent;
2021
use Symfony\Component\HttpKernel\Exception\HttpException;
2122
use Symfony\Component\Serializer\Exception\NotEncodableValueException;
2223
use Symfony\Component\Serializer\Exception\PartialDenormalizationException;
@@ -59,24 +60,44 @@ public function __construct(
5960

6061
public function resolve(Request $request, ArgumentMetadata $argument): iterable
6162
{
62-
$payloadMappers = [
63-
MapQueryString::class => ['mapQueryString', Response::HTTP_NOT_FOUND],
64-
MapRequestPayload::class => ['mapRequestPayload', Response::HTTP_UNPROCESSABLE_ENTITY],
65-
];
63+
$attribute = $argument->getAttributesOfType(MapQueryString::class, ArgumentMetadata::IS_INSTANCEOF)[0]
64+
?? $argument->getAttributesOfType(MapRequestPayload::class, ArgumentMetadata::IS_INSTANCEOF)[0]
65+
?? null;
6666

67-
foreach ($payloadMappers as $mappingAttribute => [$payloadMapper, $validationFailedCode]) {
68-
if (!$attributes = $argument->getAttributesOfType($mappingAttribute, ArgumentMetadata::IS_INSTANCEOF)) {
69-
continue;
70-
}
67+
if (!$attribute) {
68+
return [];
69+
}
70+
71+
if (!$type = $argument->getType()) {
72+
throw new \LogicException(sprintf('Could not resolve the "$%s" controller argument: argument should be typed.', $argument->getName()));
73+
}
74+
75+
$attribute->name = $argument->getName();
76+
$attribute->type = $type;
7177

72-
if (!$type = $argument->getType()) {
73-
throw new \LogicException(sprintf('Could not resolve the "$%s" controller argument: argument should be typed.', $argument->getName()));
78+
return [$attribute];
79+
}
80+
81+
public function onKernelControllerArguments(ControllerArgumentsEvent $event): void
82+
{
83+
$arguments = $event->getArguments();
84+
85+
foreach ($arguments as $i => $argument) {
86+
if ($argument instanceof MapQueryString) {
87+
$payloadMapper = 'mapQueryString';
88+
$validationFailedCode = Response::HTTP_NOT_FOUND;
89+
} elseif ($argument instanceof MapRequestPayload) {
90+
$payloadMapper = 'mapRequestPayload';
91+
$validationFailedCode = Response::HTTP_UNPROCESSABLE_ENTITY;
92+
} else {
93+
continue;
7494
}
95+
$request = $event->getRequest();
7596

7697
if ($this->validator) {
7798
$violations = new ConstraintViolationList();
7899
try {
79-
$payload = $this->$payloadMapper($request, $type, $attributes[0]);
100+
$payload = $this->$payloadMapper($request, $argument);
80101
} catch (PartialDenormalizationException $e) {
81102
$trans = $this->translator ? $this->translator->trans(...) : fn ($m, $p) => strtr($m, $p);
82103
foreach ($e->getErrors() as $error) {
@@ -92,38 +113,36 @@ public function resolve(Request $request, ArgumentMetadata $argument): iterable
92113
}
93114

94115
if (null !== $payload) {
95-
$violations->addAll($this->validator->validate($payload, null, $attributes[0]->validationGroups ?? null));
116+
$violations->addAll($this->validator->validate($payload, null, $argument->validationGroups ?? null));
96117
}
97118

98119
if (\count($violations)) {
99120
throw new HttpException($validationFailedCode, implode("\n", array_map(static fn ($e) => $e->getMessage(), iterator_to_array($violations))), new ValidationFailedException($payload, $violations));
100121
}
101122
} else {
102123
try {
103-
$payload = $this->$payloadMapper($request, $type, $attributes[0]);
124+
$payload = $this->$payloadMapper($request, $argument);
104125
} catch (PartialDenormalizationException $e) {
105126
throw new HttpException($validationFailedCode, implode("\n", array_map(static fn ($e) => $e->getMessage(), $e->getErrors())), $e);
106127
}
107128
}
108129

109-
if (null !== $payload || $argument->isNullable()) {
110-
return [$payload];
111-
}
130+
$arguments[$i] = $payload;
112131
}
113132

114-
return [];
133+
$event->setArguments($arguments);
115134
}
116135

117-
private function mapQueryString(Request $request, string $type, MapQueryString $attribute): ?object
136+
private function mapQueryString(Request $request, MapQueryString $attribute): ?object
118137
{
119138
if (!$data = $request->query->all()) {
120139
return null;
121140
}
122141

123-
return $this->serializer->denormalize($data, $type, null, self::CONTEXT_DENORMALIZE + $attribute->serializationContext);
142+
return $this->serializer->denormalize($data, $attribute->type, null, self::CONTEXT_DENORMALIZE + $attribute->serializationContext);
124143
}
125144

126-
private function mapRequestPayload(Request $request, string $type, MapRequestPayload $attribute): ?object
145+
private function mapRequestPayload(Request $request, MapRequestPayload $attribute): ?object
127146
{
128147
if (null === $format = $request->getContentTypeFormat()) {
129148
throw new HttpException(Response::HTTP_UNSUPPORTED_MEDIA_TYPE, 'Unsupported format.');
@@ -134,7 +153,7 @@ private function mapRequestPayload(Request $request, string $type, MapRequestPay
134153
}
135154

136155
if ($data = $request->request->all()) {
137-
return $this->serializer->denormalize($data, $type, null, self::CONTEXT_DENORMALIZE + $attribute->serializationContext);
156+
return $this->serializer->denormalize($data, $attribute->type, null, self::CONTEXT_DENORMALIZE + $attribute->serializationContext);
138157
}
139158

140159
if ('' === $data = $request->getContent()) {
@@ -146,7 +165,7 @@ private function mapRequestPayload(Request $request, string $type, MapRequestPay
146165
}
147166

148167
try {
149-
return $this->serializer->deserialize($data, $type, $format, self::CONTEXT_DESERIALIZE + $attribute->serializationContext);
168+
return $this->serializer->deserialize($data, $attribute->type, $format, self::CONTEXT_DESERIALIZE + $attribute->serializationContext);
150169
} catch (UnsupportedFormatException $e) {
151170
throw new HttpException(Response::HTTP_UNSUPPORTED_MEDIA_TYPE, sprintf('Unsupported format: "%s".', $format), $e);
152171
} catch (NotEncodableValueException $e) {

‎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)