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

Comments

@daboul
Copy link

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Collaborator

commented Dec 11, 2018

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Collaborator

commented Dec 11, 2018

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

@bagder

This comment has been minimized.

Copy link
Member

commented Dec 11, 2018

Yes!

@bagder bagder added the not-a-bug label Dec 11, 2018

@monnerat monnerat closed this in 37093a2 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.
Projects
None yet
3 participants
You can’t perform that action at this time.