-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpKernel] Fatal Error when using #[MapUploadedFile]
with non-array/non-variadic argument
#57824
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 7.1
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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($validationFailedCode); | ||
} | ||
\array_splice($arguments, $i, \count($payload), $payload); | ||
} else { | ||
$arguments[$i] = $payload; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about the situation when the argument is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting 🤔 |
||
} | ||
} | ||
|
||
$event->setArguments($arguments); | ||
|
@@ -233,6 +240,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); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check if it is variadic as well as if the type is an array here, similar to the check in the
mapUploadedFile
method?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically
$arguments
here are original arguments of an action.In example below
$arguments = [$request, $payload, $files];
And the magic here is to expand variadic
$files
into several items of$arguments
.I hope PHP won't change mechanics of how variadic works and it will be always only 1 possible variadic argument in function signature 🤞
Also, there is already an ability to use
#[MapUploadedFile]
as bellow. And it works and covered byUploadedFileValueResolverTest::testMultipleFilesArray
test case.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested variant
with request like
Shortly: current implementation is not designed to work like this, because data for
#[MapRequestPayload]
is fetched like this :
https://github.com/melya/symfony/blob/fix-map-uploaded-file-in-request-payload-resolver2/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php?plain=1#L212-L232