8000 HTTPClient multipart/form-data request issues · Issue #35443 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

HTTPClient multipart/form-data request issues #35443

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
n3o77 opened this issue Jan 22, 2020 · 18 comments
Closed

HTTPClient multipart/form-data request issues #35443

n3o77 opened this issue Jan 22, 2020 · 18 comments
Labels
Bug HttpClient Status: Needs Review

Comments

@n3o77
Copy link
Contributor
n3o77 commented Jan 22, 2020

Symfony version(s) affected: 4.4.3

Description
I'm communicating with an API and ran in some problems when I tried to upload files with the HTTPClient.

How to reproduce
Unfortunately it's not a public API so i can't provide the exact examples but i'm basically doing what's described in the docs:

$fields = [
    'file' => new DataPart($csv, 'test.csv', 'text/csv'),
];

$formData = new FormDataPart($fields);

$httpClient->request('POST', '/', ['body' => $formData->bodyToString()]);

Possible Solution
The problem is because in the data part the content-type has an additional token name=file (see below). At least if i remove it from the request everything works fine. I was looking in the rfc7233 and rfc7578 spec but couldn't really find anything about this, but not sure if i'm just missing it or looking in the wrong spec.

--_=_symfony_1579722514_dac69a39ca05c454f2a6375249ce7614_=_
Content-Type: text/csv; name=file
Content-Transfer-Encoding: 8bit
Content-Disposition: form-data; name=file; filename=test.csv

.... data ...

--_=_symfony_1579722514_dac69a39ca05c454f2a6375249ce7614_=_--

The second issue I ran into was that the boundary in the request headers is enclosed in quotes, which also causes propblems. Again, couldn't really find something about that but it's not in the request when i'm just using curl in the terminal.

Doesn't work:

Content-Type: multipart/form-data; boundary="_=_symfony_1579723754_2b148207ef318a96c271a8e5bd56b206_=_"

Works:

Content-Type: multipart/form-data; boundary=_=_symfony_1579723754_2b148207ef318a96c271a8e5bd56b206_=_

Are these problems in the symfony mime client or in the API i'm working with?
@martinbertinat
Copy link

Hi @n3o77 did you find a solution? I'm dealing with the same problem

@n3o77
Copy link
Contributor Author
n3o77 commented Jan 28, 2020

@martinbertinat my quick and dirty solution was just a preg_replace on the result of $formData->bodyToString().

I actually think it's a bug in the mime component which needs to be fixed. But i'm not sure if that's only an issue with HTTP requests as this component is also used for emails and maybe other stuff as well. I'm not very familiar with a lot of these specs so i hoped that someone who worked on the mime component could chime in if this was intended or not. I'm happy to create a PR to fix it.

@nicolas-grekas
Copy link
Member

Up for a PR anyone, with tests?

n3o77 added a commit to n3o77/mime that referenced this issue Feb 2, 2020
@n3o77
don’t include name token in content-type for TextPart
58a812e 
fixes symfony/symfony#35443
This was referenced Feb 2, 2020
Closed
Closed
@n3o77
Copy link
Contributor Author
n3o77 commented Feb 3, 2020

Regarding the boundary (defined here), because of the = it will be put between quotes but only in the http header and not in the multipart/form-data headers which is probably why it doesn't work.

I'm wondering if that format generally makes sense as it also leaks the client used to make the request. For reference how curl, firefox, chrome and safari handle the boundary:

Curl: ------------------------06c924ca23bff054< 8000 /code>
Firefox: -----------------------------67514980420866487271897199179
Chrome / Safari: ------WebKitFormBoundaryrGJIVv8vJ6wOwAST

Because of this i would suggest doing something similar like ---------------'.time().bin2hex(random_bytes(16)).'

And maybe even have a public setter to overwrite the value. Any feedback would be nice, if others agree i'm happy to create a PR.

@emnsen
Copy link
emnsen commented Feb 11, 2020

Is there any improvement regarding this problem?

@nicolas-grekas
Copy link
Member

@emnsen up for a PR? @n3o77 up for another try? anyone else?

This was referenced Mar 11, 2020
Closed
Merged
@nicolas-grekas
Copy link
Member
nicolas-grekas commented Mar 12, 2020 •
edited
Loading

@n3o77 which server do you use, that parses the MIME parts and fails in this situation?

@n3o77
Copy link
Contributor Author
n3o77 commented Mar 12, 2020

@nicolas-grekas unfortunately not a server available to the public. I also don't have any specs about the software used etc..

@fabpot fabpot closed this as completed in f166fe5 Mar 15, 2020
@wiser
Copy link
wiser commented Mar 17, 2020

Hi, I've the same issue on several products that expose HTTP APIs : Dalim Twist (written in Java) Marklogic (written in C I guess...?).
None of them accept HTTP multipart/form-data file upload using HttpClient and FormDataPart libs.
All is ok with curl and/or PostMan so I need to play the 7 differences game to find out the explanation.

@nicolas-grekas
Copy link
Member
nicolas-grekas commented Mar 17, 2020 •
edited
Loading

@wiser then can you please confirm that the linked patch fixes the issue for you too?

@wiser
Copy link
wiser commented Mar 17, 2020 •
edited
Loading

@nicolas-grekas Unfortunately not :(
According to my tests, the problem is not relies to Symfony libs but to the 2 HTTP APIs that do not manage multipart/form-data requests when datapart names are serialized as "token" (without quotes). If I surround each part name by quotes it seems that request is parsed correctly.
I will check this strange behaviour and confirm it before trying to discuss with products editors.

@nicolas-grekas
Copy link
Member

OK thanks. Then please open a separate issue as this one is closed, and your issue looks different.

@n3o77
Copy link
Contributor Author
n3o77 commented Mar 17, 2020

@nicolas-grekas the PR does not solve this issue at all. The inital issue described from me is not related to the boundary header but to the name property in the Content-Type form data part.

@nicolas-grekas
Copy link
Member

You described there was an issue with the boundary, and it's fixed now, isn't it?
But we missed the 1st part, which suggests we should always add double quotes around names, is that correct? Up for a PR?

@n3o77
Copy link
Contributor Author
n3o77 commented Mar 17, 2020

@nicolas-grekas The issue with the name in the content-type is that it's not in the http spec. No browser does this. But it's required in e-mails as otherwise the attached files won't be displayed with a proper name in all clients. So i guess the only solution is to not use the symfony/mime component for this case and use something else. I'm happy to work on PR but would love some feedback first how it should roughly be implemented so i'm not wasting too much time. Should this then be an own component, should it be in the http client or another solution?

@wiser
Copy link
wiser commented Mar 17, 2020 •
edited
Loading

@n3o77 @nicolas-grekas As I also have issues with this lib so I check on rfc2388, rfc7233 and rfc7578 if I can find any helping informations.
I agree with @n3o77 about the extra "name" param for content-type in http context, I don’t see any reference of that.
But to solve my issue I also need to put double quotes around name values (as @nicolas-grekas
suggests). The rfc7578 give some examples and name values are always surrounded by double-quotes.
Did you have any opinion about this ?
Can I suggest to always add double quotes around name params ?

@wiser wiser mentioned this issue May 11, 2020
Closed
@nicolas-grekas nicolas-grekas mentioned this issue Jul 6, 2020
Closed
@fabpot
Copy link
Member
fabpot commented Jul 15, 2020

Closing as a duplicate of #37500

@fabpot fabpot closed this as completed Jul 15, 2020
This was referenced Jul 15, 2020
Closed
Merged
@fabpot
Copy link
Member
fabpot commented Jul 15, 2020

@n3o77 #37581 should fix your issue, can you confirm?

fabpot added a commit that referenced this issue Jul 15, 2020
@fabpot
bug #37581 [Mime] Fix compat with HTTP requests (fabpot)
f617380 
This PR was merged into the 4.4 branch.

Discussion
----------

[Mime] Fix compat with HTTP requests

| Q             | A
| ------------- | ---
| Branch?       | 4.4 <!-- see below -->
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #37500, Fix #36738, Fix #35443
| License       | MIT
| Doc PR        | n/a

Commits
-------

52e7d7c [Mime] Fix compat with HTTP requests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug HttpClient Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

don’t include name token in content-type for TextPart n3o77/mime
[Mime] Fix header boundary
8 participants
@fabpot @nicolas-grekas @n3o77 @wiser @martinbertinat @xabbuh @emnsen @carsonbot
0