From 5861123adbf1a679266369bbe431bcf105118cc6 Mon Sep 17 00:00:00 2001 From: Vladimir Melnik Date: Wed, 24 Jul 2024 16:27:18 +0200 Subject: [PATCH 1/3] [HttpKernel] Fix resolving of MapUploadedFile for non-array/non-variadic arguments --- .../RequestPayloadValueResolver.php | 7 ++- .../UploadedFileValueResolverTest.php | 60 ++++++++++++++++++- 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php index c35d5e7e29381..2c8e0ff81d7d9 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php @@ -233,6 +233,11 @@ private function mapRequestPayload(Request $request, ArgumentMetadata $argument, private function mapUploadedFile(Request $request, ArgumentMetadata $argument, MapUploadedFile $attribute): UploadedFile|array|null { - return $request->files->get($attribute->name ?? $argument->getName(), []); + $default = null; + if ($argument->isVariadic() || "array" === $argument->getType()) { + $default = []; + } + + return $request->files->get($attribute->name ?? $argument->getName(), $default); } } diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/UploadedFileValueResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/UploadedFileValueResolverTest.php index 5eb0d32483ed5..113577974508e 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/UploadedFileValueResolverTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/UploadedFileValueResolverTest.php @@ -68,7 +68,7 @@ public function testEmpty(RequestPayloadValueResolver $resolver, Request $reques $attribute = new MapUploadedFile(); $argument = new ArgumentMetadata( 'qux', - UploadedFile::class, + "array", false, false, null, @@ -82,12 +82,70 @@ static function () {}, $request, HttpKernelInterface::MAIN_REQUEST ); + $resolver->onKernelControllerArguments($event); $data = $event->getArguments()[0]; $this->assertEmpty($data); } + /** + * @dataProvider provideContext + */ + public function testNotNullableFile(RequestPayloadValueResolver $resolver, Request $request) + { + $attribute = new MapUploadedFile(); + $argument = new ArgumentMetadata( + 'qux', + UploadedFile::class, + false, + false, + null, + false, + [$attribute::class => $attribute] + ); + $event = new ControllerArgumentsEvent( + $this->createMock(HttpKernelInterface::class), + static function () {}, + $resolver->resolve($request, $argument), + $request, + HttpKernelInterface::MAIN_REQUEST + ); + + $this->expectException(HttpException::class); + + $resolver->onKernelControllerArguments($event); + } + + /** + * @dataProvider provideContext + */ + public function testNullableFile(RequestPayloadValueResolver $resolver, Request $request) + { + $attribute = new MapUploadedFile(); + $argument = new ArgumentMetadata( + 'qux', + UploadedFile::class, + false, + true, + null, + true, + [$attribute::class => $attribute] + ); + $event = new ControllerArgumentsEvent( + $this->createMock(HttpKernelInterface::class), + static function () {}, + $resolver->resolve($request, $argument), + $request, + HttpKernelInterface::MAIN_REQUEST + ); + + $resolver->onKernelControllerArguments($event); + $data = $event->getArguments()[0]; + + $this->assertNull($data); + } + /** * @dataProvider provideContext */ From 2a99d2d25b0bce1e7334d8b30f331a564a7be56b Mon Sep 17 00:00:00 2001 From: Vladimir Melnik Date: Mon, 29 Jul 2024 17:13:34 +0200 Subject: [PATCH 2/3] fixed usage of variadic uploaded-file --- .../ArgumentResolver/RequestPayloadValueResolver.php | 9 ++++++++- .../ArgumentResolver/UploadedFileValueResolverTest.php | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php index 2c8e0ff81d7d9..ec5f17e09b335 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php @@ -170,7 +170,14 @@ public function onKernelControllerArguments(ControllerArgumentsEvent $event): vo }; } - $arguments[$i] = $payload; + if ($argument->metadata->isVariadic()) { + if (!(is_array($payload) && array_is_list($payload))) { + throw HttpException::fromStatusCode(422); + } + array_splice($arguments, $i, count($payload), $payload); + } else { + $arguments[$i] = $payload; + } } $event->setArguments($arguments); diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/UploadedFileValueResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/UploadedFileValueResolverTest.php index 113577974508e..b6b59b1fcf889 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/UploadedFileValueResolverTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/UploadedFileValueResolverTest.php @@ -327,7 +327,7 @@ static function () {}, $resolver->onKernelControllerArguments($event); /** @var UploadedFile[] $data */ - $data = $event->getArguments()[0]; + $data = $event->getArguments(); $this->assertCount(2, $data); $this->assertSame('file-small.txt', $data[0]->getFilename()); From 93d41904910eaca3c10775b4140cbdcd030e98d6 Mon Sep 17 00:00:00 2001 From: Vladimir Melnik Date: Tue, 20 Aug 2024 21:04:42 +0200 Subject: [PATCH 3/3] review fixes --- .../ArgumentResolver/RequestPayloadValueResolver.php | 8 ++++---- .../ArgumentResolver/UploadedFileValueResolverTest.php | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php index ec5f17e09b335..59a1030ca807d 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php @@ -171,10 +171,10 @@ public function onKernelControllerArguments(ControllerArgumentsEvent $event): vo } if ($argument->metadata->isVariadic()) { - if (!(is_array($payload) && array_is_list($payload))) { - throw HttpException::fromStatusCode(422); + if (!\is_array($payload) || !\array_is_list($payload)) { + throw HttpException::fromStatusCode($validationFailedCode); } - array_splice($arguments, $i, count($payload), $payload); + \array_splice($arguments, $i, \count($payload), $payload); } else { $arguments[$i] = $payload; } @@ -241,7 +241,7 @@ private function mapRequestPayload(Request $request, ArgumentMetadata $argument, private function mapUploadedFile(Request $request, ArgumentMetadata $argument, MapUploadedFile $attribute): UploadedFile|array|null { $default = null; - if ($argument->isVariadic() || "array" === $argument->getType()) { + if ($argument->isVariadic() || 'array' === $argument->getType()) { $default = []; } diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/UploadedFileValueResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/UploadedFileValueResolverTest.php index b6b59b1fcf889..8817ee64ae6ae 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/UploadedFileValueResolverTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/UploadedFileValueResolverTest.php @@ -68,7 +68,7 @@ public function testEmpty(RequestPayloadValueResolver $resolver, Request $reques $attribute = new MapUploadedFile(); $argument = new ArgumentMetadata( 'qux', - "array", + 'array', false, false, null,