Bug 16729 – [cURL] Allow multiple files for upload

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Luca Bruno]

Hello,
currently the setupPOST method in the ResourceHandleManager of cURL seems to handle multiple files but doesn't. Actually it uses two different methods for posting, one POSTFIELDS and the other HTTPPOST which are mutually exclusive.
Current issues:
- No problem without file uploading
- If files need upload, only one file is uploaded
- The other data when the file is uploaded is not POSTed due to the above conflict
- Uploading this file doesn't respect the field names of the FORM, instead forces to use "sendfile" and "filename".

[Luca Bruno]

Created attachment 18259 [details]
proof

You can try it here: http://encodable.com/uploaddemo/

No regressions found with the patch. The two parts are just copied from the main.c of curl.
It's all C since i don't develop C++, hopefully someone can help.
Also i didn't change the job internals, by removing m_filename and changing m_postBytes to char*, would be done later.

[Luca Bruno]

Forgot to say, file2string is copied directly from cURL, and i don't know why it strips \n and \r, is that right for our purposes?

[Mark Rowe (bdash)]

Comment on attachment 18259 [details]
proof

Clearing review flag as this patch is not in a reviewable state.

[Luca Bruno]

Created attachment 18263 [details]
use read callback

Use read callback for sending data, and setupPOST to set the size of the POST.

[Luca Bruno]

Created attachment 18264 [details]
clean up

Remove the loop from readCallback, cURL does it for us. This cleaned up things avoiding possible mistakes with cURL.
Assert type size in setupPOST as suggested by bdash.

[Luca Bruno]

Created attachment 18265 [details]
little mistake

Read the previous comment, thanks.

[Luca Bruno]

Created attachment 18270 [details]
introduce chunked transfer

Use chunked data transfer when size of files can't be obtained as suggested by bdash.

Chunked transfer must be automatically handled by Curl, as it does, but don't know if it does the right way. I've done some tests and doesn't seem to work well. But this could be another issue and might not be related to our manager but instead to cURL or the web server.

[Mark Rowe (bdash)]

Comment on attachment 18270 [details]
introduce chunked transfer

Unsetting review flag as this patch has known issues that prevent it from being reviewable.

[Luca Bruno]

Created attachment 18277 [details]
file checks

Check for file opening and reading, and in case cancel the job.

[Luca Bruno]

Created attachment 18279 [details]
take care of the post size

curl_off_t doesn't seem to be 64-bit wide on most platforms, so we shouldn't use an assertion. The patch checks for an overflow and in case use chunked encoding for large files as suggested by Mark Rowe.

[Luca Bruno]

Created attachment 18312 [details]
do not stream simple POSTs

Mark Rowe noticed me CF does simple POSTs without streaming the data. This is just what's done originally.

[Luca Bruno]

Created attachment 18313 [details]
return if empty POST

[Alp Toker]

Comment on attachment 18313 [details]
return if empty POST

r=me

There are whitespace issues (space before *) and relocated some comments. Please check these changes for next time.

Also found some tabs in the changelog entry.

Nice work otherwise!

[Alp Toker]

Landed in r29423.