Menu

#1171 [PATCH] support upload file with ',' or ';' in the filename

closed-fixed
http (206)
5
2013-06-21
2012-12-11
Anonymous
No

Here 's a patch, support http form uploading file with ',' or ';' in its filename, by support parse quotes around the file and filename part.
Which does not change the behavior of curl of any existed external usage, just support parse file and filename which is quoted, if the caller does quoting the file or the filename part..

Discussion

  • Daniel Stenberg

    Daniel Stenberg - 2012-12-13

    Thanks a lot! I just miss a few little details in this patch:

    1 - how do you specify a file name with a double-quote in it?

    2 - the documentation update that explains how to use this feature

    3 - a test case or two that verify that the code actually works as intended

     
  • Daniel Stenberg

    Daniel Stenberg - 2012-12-21

    please?

     
  • Daniel Stenberg

    Daniel Stenberg - 2013-01-08
    • status: open --> pending
     
  • Daniel Stenberg

    Daniel Stenberg - 2013-01-08

    Due to its incompleteness, we will not apply this patch but it will be closed in a near future unless someone completes it or provides an alternative approach.

     
  • Ulion

    Ulion - 2013-01-17

    updated patch commited as PR on github: https://github.com/bagder/curl/pull/55

     
  • Daniel Stenberg

    Daniel Stenberg - 2013-01-18

    Thanks, but I really would like an answer to my question (1). How do you include a file name that has a double quote in its name? I find it strange to make a fix for ; and , but exclude ". I think would rather prefer an escape mechanism (like backslash which is rather common in shell/command lines) that would work for all of them!

     
  • Ulion

    Ulion - 2013-01-19

    Does curl support '"' in filename currently since it does not affected by the ',' or ';' parsing?
    If libcurl does not support '"', it would be another problem than current resolved one.
    If libcurl support '"', I will try to make it can be parsed and send to libcurl.

     
  • Ulion

    Ulion - 2013-01-19

    After a quick look, libcurl does not do any escape for filename or show_filename (the faked one), it just surrounded it with double quotes in the form header.
    if I change test39 param to -F "file=@log/test39.txt;type=moo/foobar;filename=faker\"file"
    it will result:
    Content-Disposition: form-data; name="file"; filename="faker"file"
    the server will only see "faker", the double-quote in filename is not correctly supported by libcurl.

    If we need to make it support double-quote in filename, it will be another patch and may affect existed code using libcurl.
    This patch only change the way curl command line program parse the filename, do nothing with libcurl, I don't want to mix two problems into one.

     
  • Daniel Stenberg

    Daniel Stenberg - 2013-01-19

    On Sat, 19 Jan 2013, Ulion wrote:

    the double-quote in filename is not correctly supported by libcurl.

    Yes, you're right that's a separate bug. But is the presence of that bug
    really a good reason to not do the right thing in the curl application? If
    (when?) we fix the problem in libcurl we shouldn't have to change curl again,
    should we?

    I'm just seeing (and liking) this opportunity to fix the file name thing in
    curl for formposting and while we're going at it I think "fixing it all the
    way" seems like a good idea!

    --

    / daniel.haxx.se

     
  • Ulion

    Ulion - 2013-01-20

    well, if you agree to fix the double-quote escaping in libcurl, the PR is updated to do that.

     
  • Daniel Stenberg

    Daniel Stenberg - 2013-01-20

    I agree that there's a (separate) bug in libcurl if you pass in a file name with an embedded double quote. I want that fixed but won't promise I will do it anytime soon...

     
  • Daniel Stenberg

    Daniel Stenberg - 2013-01-20
    • status: pending --> open-confirmed
     
  • Ulion

    Ulion - 2013-01-21

    Indeed I also fixed the libcurl bug you mentioned in separated commit, you can check it in the pull request: https://github.com/bagder/curl/pull/55

     
  • Daniel Stenberg

    Daniel Stenberg - 2013-01-21

    Thanks!

    I take it you don't want "proper" credits for your contribution as there's no real name anywhere here?

    I've edited the 20+ warnings I got when building this (C90 violations and code style nits mostly), but I've not yet made test 1133 run fine since the double quote in the file name makes memanalyze.pl fail for me in that test and it then wrongly identify an fclose without an fopen... I'll work on fixing that before I feel confident to push this patch.

     
  • Ulion

    Ulion - 2013-01-22

    I didn't notice warnings, thanks, C is really harder than other language to finish same job, I'm glad that we finally made it.

     
  • Daniel Stenberg

    Daniel Stenberg - 2013-01-22
    • status: open-confirmed --> closed-fixed
     
  • Daniel Stenberg

    Daniel Stenberg - 2013-01-22

    Thanks, your work has been merged into the master branch just now in commit 2698520aef593c