Skip to content

[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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: 7.1
Choose a base branch
from
Open

[HttpKernel] Fatal Error when using #[MapUploadedFile] with non-array/non-variadic argument #57824

wants to merge 3 commits into from
+74 −4

Conversation

melya
Copy link
Contributor

@melya melya commented Jul 25, 2024 •
edited
Loading

Q A
Branch? 7.1
Bug fix? yes
New feature? no
Deprecations? no
License MIT

Having a simple controller with MapUploadedFile as non-array.
When the file is omitted in request, symfony throws ValueError because resolver returns an empty array and it doesn't match with UploadedFile $file.

In my opinion, the HttpException should be thrown in this case.
For array argument the behaviour stays the same.

#[AsController]
#[Route(path: "/upload", methods: ["post"])]
final class Upload
{
    public function __invoke(
        #[MapUploadedFile()]
        UploadedFile $file,
    ): Response {
        ...
    }
}

Example with omitted file - ValueError occurs here

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

Error example

App\Controller\Upload::__invoke(): 
Argument #2 ($file) must be of type Symfony\Component\HttpFoundation\File\UploadedFile, array given, 
called in /app/vendor/symfony/http-kernel/HttpKernel.php on line 183 (500 Internal Server Error)

@carsonbot carsonbot added this to the 7.1 milestone Jul 25, 2024
@melya
Copy link
Contributor Author

melya commented Jul 25, 2024

#[MapUploadedFile] was introduced with #49978.

@renedelima Thanks for this feature 💪 .
Could you please check my fix to ensure I haven't missed any initial ideas?

@melya melya changed the title [HttpKernel] Fix resolving of MapUploadedFile for non-array/non-variadic arguments [HttpKernel] Fix resolving of MapUploadedFile for non-array/non-variadic arguments Jul 29, 2024
@melya melya changed the title [HttpKernel] Fix resolving of MapUploadedFile for non-array/non-variadic arguments [HttpKernel] Fix resolving of #[MapUploadedFile] for non-array/non-variadic arguments Jul 29, 2024
@melya melya changed the title [HttpKernel] Fix resolving of #[MapUploadedFile] for non-array/non-variadic arguments [HttpKernel] Fatal Error when using #[MapUploadedFile] as non-array/non-variadic argument Jul 29, 2024
@melya melya changed the title [HttpKernel] Fatal Error when using #[MapUploadedFile] as non-array/non-variadic argument [HttpKernel] Fatal Error when using #[MapUploadedFile] with non-array/non-variadic argument Jul 29, 2024
@melya melya mentioned this pull request Jul 29, 2024
Closed
nicolas-grekas
nicolas-grekas reviewed Aug 16, 2024
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

LGTM, I just have minor comments.

@melya
Copy link
Contributor Author

melya commented Aug 29, 2024

Seems like failed tests are unrelated to my changes

@melya
Copy link
Contributor Author

melya commented Sep 13, 2024

@nicolas-grekas @stetodd friendly reminder ;)

@melya melya force-pushed the fix-map-uploaded-file-in-request-payload-resolver2 branch from faa10b6 to 93d4190 Compare September 24, 2024 14:22
@melya
Copy link
Contributor Author

melya commented Nov 4, 2024

any updates?

@nicolas-grekas nicolas-grekas modified the milestones: 7.1, 7.2 Feb 4, 2025
@xabbuh xabbuh modified the milestones: 7.2, 7.3 Aug 14, 2025
@fabpot
Copy link
Member

fabpot commented Aug 23, 2025

It looks like we've already worked on the same problem in #61381. Can you have a look and rebase this PR accordingly (maybe this PR is not needed anymore)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OskarStark OskarStark OskarStark left review comments

@fabpot fabpot fabpot approved these changes

@stetodd stetodd Awaiting requested review from stetodd

@nicolas-grekas nicolas-grekas Awaiting requested review from nicolas-grekas

Assignees
No one assigned
Labels
Bug HttpKernel Status: Reviewed
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.
7 participants
@melya @fabpot @nicolas-grekas @OskarStark @stetodd @xabbuh @carsonbot