Skip to content

Commit 949a2e2

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

File tree

6 files changed

+124
-33
lines changed

6 files changed

+124
-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: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,23 @@
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+
73+
->set('argument_resolver.request_payload', RequestPayloadValueResolver::class)
74+
->factory('current')
75+
->args([[service('request_payload_value_resolver')]])
7276
->tag('controller.targeted_value_resolver', ['name' => RequestPayloadValueResolver::class])
7377

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

‎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: 46 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;
@@ -32,7 +35,7 @@
3235
/**
3336
* @author Konstantin Myakshin <molodchick@gmail.com>
3437
*/
35-
final class RequestPayloadValueResolver implements ValueResolverInterface
38+
final class RequestPayloadValueResolver implements ValueResolverInterface, EventSubscriberInterface
3639
{
3740
/**
3841
* @see \Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::DISABLE_TYPE_ENFORCEMENT
@@ -59,24 +62,43 @@ public function __construct(
5962

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

67-
foreach ($payloadMappers as $mappingAttribute => [$payloadMapper, $validationFailedCode]) {
68-
if (!$attributes = $argument->getAttributesOfType($mappingAttribute, ArgumentMetadata::IS_INSTANCEOF)) {
75+
return [$attribute];
76+
}
77+
78+
public function onKernelControllerArguments(ControllerArgumentsEvent $event): void
79+
{
80+
$arguments = $event->getArguments();
81+
82+
foreach ($arguments as $i => $argument) {
83+
if ($argument instanceof MapQueryString) {
84+
$payloadMapper = 'mapQueryString';
85+
$validationFailedCode = Response::HTTP_NOT_FOUND;
86+
} elseif ($argument instanceof MapRequestPayload) {
87+
$payloadMapper = 'mapRequestPayload';
88+
$validationFailedCode = Response::HTTP_UNPROCESSABLE_ENTITY;
89+
} else {
6990
continue;
7091
}
92+
$request = $event->getRequest();
7193

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

7698
if ($this->validator) {
7799
$violations = new ConstraintViolationList();
78100
try {
79-
$payload = $this->$payloadMapper($request, $type, $attributes[0]);
101+
$payload = $this->$payloadMapper($request, $type, $argument);
80102
} catch (PartialDenormalizationException $e) {
81103
$trans = $this->translator ? $this->translator->trans(...) : fn ($m, $p) => strtr($m, $p);
82104
foreach ($e->getErrors() as $error) {
@@ -92,26 +114,35 @@ public function resolve(Request $request, ArgumentMetadata $argument): iterable
92114
}
93115

94116
if (null !== $payload) {
95-
$violations->addAll($this->validator->validate($payload, null, $attributes[0]->validationGroups ?? null));
117+
$violations->addAll($this->validator->validate($payload, null, $argument->validationGroups ?? null));
96118
}
97119

98120
if (\count($violations)) {
99121
throw new HttpException($validationFailedCode, implode("\n", array_map(static fn ($e) => $e->getMessage(), iterator_to_array($violations))), new ValidationFailedException($payload, $violations));
100122
}
101123
} else {
102124
try {
103-
$payload = $this->$payloadMapper($request, $type, $attributes[0]);
125+
$payload = $this->$payloadMapper($request, $argument);
104126
} catch (PartialDenormalizationException $e) {
105127
throw new HttpException($validationFailedCode, implode("\n", array_map(static fn ($e) => $e->getMessage(), $e->getErrors())), $e);
106128
}
107129
}
108130

109-
if (null !== $payload || $argument->isNullable()) {
110-
return [$payload];
131+
if (null === $payload && !$argument->metadata->isNullable()) {
132+
throw new HttpException($validationFailedCode);
111133
}
134+
135+
$arguments[$i] = $payload;
112136
}
113137

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

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