Skip to content
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

7.47.1 VS 7.61.1 / curl_formadd deals with '\' in param differently #3361

Closed
daboul opened this issue Dec 11, 2018 · 7 comments
Closed

7.47.1 VS 7.61.1 / curl_formadd deals with '\' in param differently #3361

daboul opened this issue Dec 11, 2018 · 7 comments
Labels
HTTP not-a-bug This is not a bug in curl

Comments

@daboul
Copy link

daboul commented Dec 11, 2018

Hi.

In 7.47.1, we were passing as curl_formadd parameter CURLFORM_COPYNAME (comment also apply to CURLFORM_FILENAME) a string with a backslash ''. So a simple, single byte per character, null terminated string like "a\b" is being picked up nicely by the server, no issue.

But in 7.61.1, the back-slash ends up being doubled and the server get a string "a\b" instead in the content-disposition.

I can see in https://curl.haxx.se/libcurl/c/curl_formadd.html that the 7.56 introduced a difference in the management of parameters, but I don't understand that particular behaviour change. Doubling the backslashes seems like a bug to me. Am I missing anything? If it is done on purpose, for a reason I'm missing, maybe updating the documentation would be helpful?

Thank you,
David.

@bagder bagder added the HTTP label Dec 11, 2018
@bagder
Copy link
Member

bagder commented Dec 11, 2018

Doubling the backslashes seems like a bug to me

Agreed. @monnerat can you recall the reason behind this? The code that does this seems to be here

curl/lib/mime.c

Line 1747 in 3a9cb0d

name = escape_string(part->name);

I could reproduce the problem by adjusting tests/libcurl/lib554.c like this:

diff --git a/tests/libtest/lib554.c b/tests/libtest/lib554.c
index cc21d245b..79ab72136 100644
--- a/tests/libtest/lib554.c
+++ b/tests/libtest/lib554.c
@@ -80,11 +80,11 @@ static int once(char *URL, bool oldstyle)
 
   /* Fill in the file upload field */
   if(oldstyle) {
     formrc = curl_formadd(&formpost,
                           &lastptr,
-                          CURLFORM_COPYNAME, "sendfile",
+                          CURLFORM_COPYNAME, "send\\file",
                           CURLFORM_STREAM, &pooh,
                           CURLFORM_CONTENTSLENGTH, (long)pooh.sizeleft,
                           CURLFORM_FILENAME, "postit2.c",
                           CURLFORM_END);
   }

@bagder
Copy link
Member

bagder commented Dec 11, 2018

Given some more thoughts: I suppose this treatment is done to "escape" the letter correctly then generating the header so that a MIME compliant receiver would un-escape it correctly in the other end.

@daboul
Copy link
Author

daboul commented Dec 11, 2018

My server is a basic ASP.NET Core one and when I end up trying to extract the Request.Form.File[0].Name it is failing to "de-escape" that doubled backslash.

Visual Studio screenshot

I don't find a clear specifications of what should be the content-disposition encoding (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition), but if libcurl way of encoding the backslash is correct, I guess I should notify the ASP.NET Core. Does anyone know for sure how such a character should be encoded in that case?

Thank you,
David.

@monnerat
Copy link
Contributor

can you recall the reason behind this?

RFC 7578 (multipart/form-data) introduces the name parameter without specific additional BNF, thus it falls to be a generic Content-Disposition parameter.

RFC 2183 (content disposition) defines filename-parm := "filename" "=" value and refers to RFC 2045 for the value and parameter syntaxes.

RFC 2045 (MIME) defines parameter := attribute "=" value and value := token / quoted-string. It widely refers to RFC 822 for everything it does not define itself explicitly.

RFC 822 (Mail) defines quoted-string as:

     qtext       =  <any CHAR excepting <">,     ; => may be folded
                     "\" & CR, and including
                     linear-white-space>
     quoted-pair =  "\" CHAR                     ; may quote any char
     quoted-string = <"> *(qtext/quoted-pair) <">; Regular qtext or
                                                 ;   quoted chars.

Thus " and \ must be escaped with \ in name= and filename= values of the Content-Disposition header.

IMO, the escaping done by curl is correct and a receiving server should conform to this too.

@daboul
Copy link
Author

daboul commented Dec 11, 2018

@monnerat Thanks a lot, I was struggling to find this information. Then I guess I should notify the ASP.NET Core team. In the meantime, as a work around, I will be using another form text parameter to pass my information un-escaped.

@monnerat
Copy link
Contributor

@bagder Can we admit this is not a bug and add a note in the curl_formadd documentation?

@bagder
Copy link
Member

bagder commented Dec 11, 2018

Yes!

@bagder bagder added the not-a-bug This is not a bug in curl label Dec 11, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Mar 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
HTTP not-a-bug This is not a bug in curl
Development

No branches or pull requests

3 participants