Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[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

Open
wants to merge 3 commits into
base: 7.1
Choose a base branch
Loading
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,14 @@ public function onKernelControllerArguments(ControllerArgumentsEvent $event): vo
};
}

$arguments[$i] = $payload;
if ($argument->metadata->isVariadic()) {
Copy link

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?

Copy link
Contributor Author

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];

public function action(
 Request $request,
 #[MapRequestPayload] Payload $payload,
 #[MapUploadedFile] UploadedFile ...$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 by UploadedFileValueResolverTest::testMultipleFilesArray test case.

public function action(
 Request $request,
 #[MapRequestPayload] Payload $payload,
 /** @var UploadedFile[] */
 #[MapUploadedFile] array $files]
) {
   ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested variant

final class TestPayloadRequest
{
    public UploadedFile $file;
}

final class IndexController extends AbstractController
{
    #[Route(path: "/test-upload", methods: "POST")]
    public function testUpload(
        #[MapRequestPayload] TestPayloadRequest $file,
        #[MapUploadedFile] UploadedFile ...$files,
    ): Response {
        return new Response();
    }
}

with request like

POST /test-upload HTTP/1.1
Cookie: XDEBUG_SESSION=PHPSTORM;
Content-Type: multipart/form-data; charset=utf-8; boundary=__X_PAW_BOUNDARY__

--__X_PAW_BOUNDARY__
Content-Disposition: form-data; name="files[]"; filename="test.yaml"
Content-Type: application/x-yaml

info: test
--__X_PAW_BOUNDARY__
Content-Disposition: form-data; name="file"; filename="test.yaml"
Content-Type: application/x-yaml

info: test
--__X_PAW_BOUNDARY__
Content-Disposition: form-data; name="files[]"; filename="test.yaml"
Content-Type: application/x-yaml

info: test

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

if (!\is_array($payload) || !\array_is_list($payload)) {
throw HttpException::fromStatusCode($validationFailedCode);
}
\array_splice($arguments, $i, \count($payload), $payload);
} else {
$arguments[$i] = $payload;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the situation when the argument is UploadedFile and the $payload is an array of files? I think this would still result in a 500?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting 🤔
Is it possible to have UploadedFile as part of #[MapRequestPayload] $payload? (#[MapQuery] we can exclude, I guess)
Could you please share an example of what you mean, so I can elaborate more on this ?

}
}

$event->setArguments($arguments);
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public function testEmpty(RequestPayloadValueResolver $resolver, Request $reques
$attribute = new MapUploadedFile();
$argument = new ArgumentMetadata(
'qux',
UploadedFile::class,
'array',
false,
false,
null,
Expand All @@ -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
*/
Expand Down Expand Up @@ -269,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());
Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.