Skip to content

[HttpFoundation] Request::getContentType doesn't support multipart/form-data #34240

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
Closed
keichinger opened this issue Nov 5, 2019 · 9 comments · Fixed by #43017
Closed

[HttpFoundation] Request::getContentType doesn't support multipart/form-data #34240

keichinger opened this issue Nov 5, 2019 · 9 comments · Fixed by #43017
Labels
HttpFoundation

Comments

@keichinger
Copy link
Contributor

I've asked @nicolas-grekas on Twitter and it felt like he wasn't sure, so I investigated a little bit more and found the following sentence in the specs:

According to https://www.w3.org/TR/html401/interact/forms.html#h-17.13.4.2:

The content type "multipart/form-data" should be used for submitting forms that contain files, non-ASCII data, and binary data.

In my opinion, Request::getContentType(), or much rather Request::initializeFormats() should map multipart/form-data to form, as it's per-spec related to submitting forms with mixed data. Right now the behaviour is to return null, which is confusing and cumbersome when you have a controller action that is called via a regular POST and via fetch + new FormData() as body from your frontend.

I'd be very happy to file a PR to add it to the mapping.

Opinions?
@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@carsonbot
Copy link

Hello? This issue is about to be closed if nobody replies.

@keichinger
Copy link
Contributor Author

I'm still looking forward to get this issue resolved :) Happy to submit a PR.

@carsonbot carsonbot removed the Stalled label Jan 4, 2021
@glen-rhodes
Copy link

Would love to see this go forwards and would be glad to help if needed.

@evhenkaravan
Copy link

evhenkaravan commented Mar 11, 2021 •
edited
Loading

I think it's an obvious thing - to see multipart/form-data inside \Symfony\Component\HttpFoundation\Request::initializeFormats. It can be something like 'formData' key

@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@keichinger
Copy link
Contributor Author

I'd still like to get this issue resolved ☺️ I'm still open for contributing a pull request, implementing the necessary logic or by discussing the issue at hand.

@carsonbot carsonbot removed the Stalled label Sep 12, 2021
@wouterj
Copy link
Member

wouterj commented Sep 12, 2021

@keichinger if I understand correctly, you propose to add this mapping to the existing content type mappings in Request? If that is the case, I would recommend you to submit a PR doing that. PRs are more often reviewed by the correct people compared to the open issues.

Note that this is not my area of expertise, so I cannot say anything about the proposal itself. Hopefully this helps you getting a final answer. 🙂

@keichinger
Copy link
Contributor Author

@wouterj good idea :) Will do so as soon as possible ☺️

keichinger pushed a commit to keichinger/symfony that referenced this issue Sep 14, 2021
Map multipart/form-data as form Content-Type
a92393f 
As per RFC 2045  and RFC 2388, the `multipart/form-data` Content-Type should be used when submitting a mixed form containing files, binary data and non-ASCII data.

This commit helps with infrastructure that is directly checking against the `Request::getContentType()` method in scenarios, where a mixed form has been submitted.

Resolves symfony#34240
keichinger pushed a commit to keichinger/symfony that referenced this issue Sep 14, 2021
Map multipart/form-data as form Content-Type
b091d6e 
As per RFC 2045  and RFC 2388, the `multipart/form-data` Content-Type should be used when submitting a mixed form containing files, binary data and non-ASCII data.

This commit helps with infrastructure that is directly checking against the `Request::getContentType()` method in scenarios, where a mixed form has been submitted.

Resolves symfony#34240
keichinger pushed a commit to keichinger/symfony that referenced this issue Sep 14, 2021
Map multipart/form-data as form Content-Type
db4d779 
As per RFC 2045  and RFC 2388, the `multipart/form-data` Content-Type should be used when submitting a mixed form containing files, binary data and non-ASCII data.

This commit helps with infrastructure that is directly checking against the `Request::getContentType()` method in scenarios, where a mixed form has been submitted.

Resolves symfony#34240
@keichinger keichinger mentioned this issue Sep 14, 2021
Merged
@fabpot fabpot closed this as completed in e597655 Sep 19, 2021
fabpot added a commit that referenced this issue Sep 19, 2021
@fabpot
feature #43017 [HttpFoundation] Map multipart/form-data as form C…
b641fdd 
…ontent-Type (keichinger)

This PR was merged into the 5.4 branch.

Discussion
----------

[HttpFoundation] Map `multipart/form-data` as `form` Content-Type

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  |yes <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? |no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | #34240
| License       | MIT
| Doc PR        | –

As per RFC 2045 and RFC 2388 (see https://www.w3.org/TR/html401/interact/forms.html#h-17.13.4.2), the `multipart/form-data` Content-Type should be used when submitting a mixed forms containing files, binary data and non-ASCII data.
Currently, the same logic that would run in regular, non-mixed form scenarios can't run for mixed forms, as soon as it relies on the result of `Request::getContentType()` or anything else that might rely on `Request::$formats` without going low-level and avoiding the implemented abstraction. This PR fixes that.

Resolves #34240

Commits
-------

e597655 Map `multipart/form-data` as `form` Content-Type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
HttpFoundation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[HttpFoundation] Map multipart/form-data as form Content-Type keichinger/symfony
6 participants
@keichinger @wouterj @xabbuh @carsonbot @glen-rhodes @evhenkaravan