Bug 16729 - [cURL] Allow multiple files for upload
Summary: [cURL] Allow multiple files for upload
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Trivial
Assignee: Nobody
URL: http://encodable.com/uploaddemo/
Keywords: Curl
Depends on:
Blocks:
 
Reported: 2008-01-04 02:38 PST by Luca Bruno
Modified: 2008-01-11 17:36 PST (History)
2 users (show)

See Also:


Attachments
proof (3.90 KB, patch)
2008-01-04 02:44 PST, Luca Bruno
no flags Details | Formatted Diff | Diff
use read callback (7.77 KB, patch)
2008-01-04 07:17 PST, Luca Bruno
no flags Details | Formatted Diff | Diff
clean up (7.23 KB, patch)
2008-01-04 08:07 PST, Luca Bruno
no flags Details | Formatted Diff | Diff
little mistake (7.05 KB, patch)
2008-01-04 08:10 PST, Luca Bruno
no flags Details | Formatted Diff | Diff
introduce chunked transfer (10.57 KB, patch)
2008-01-04 09:40 PST, Luca Bruno
no flags Details | Formatted Diff | Diff
file checks (11.24 KB, patch)
2008-01-04 14:14 PST, Luca Bruno
no flags Details | Formatted Diff | Diff
take care of the post size (11.18 KB, patch)
2008-01-04 15:12 PST, Luca Bruno
no flags Details | Formatted Diff | Diff
do not stream simple POSTs (11.84 KB, patch)
2008-01-07 06:18 PST, Luca Bruno
no flags Details | Formatted Diff | Diff
return if empty POST (11.88 KB, patch)
2008-01-07 06:29 PST, Luca Bruno
alp: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Luca Bruno 2008-01-04 02:38:52 PST
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".
Comment 1 Luca Bruno 2008-01-04 02:44:56 PST
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.
Comment 2 Luca Bruno 2008-01-04 02:47:59 PST
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?
Comment 3 Mark Rowe (bdash) 2008-01-04 03:32:51 PST
Comment on attachment 18259 [details]
proof

Clearing review flag as this patch is not in a reviewable state.
Comment 4 Luca Bruno 2008-01-04 07:17:29 PST
Created attachment 18263 [details]
use read callback

Use read callback for sending data, and setupPOST to set the size of the POST.
Comment 5 Luca Bruno 2008-01-04 08:07:23 PST
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.
Comment 6 Luca Bruno 2008-01-04 08:10:21 PST
Created attachment 18265 [details]
little mistake

Read the previous comment, thanks.
Comment 7 Luca Bruno 2008-01-04 09:40:23 PST
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.
Comment 8 Mark Rowe (bdash) 2008-01-04 09:58:09 PST
Comment on attachment 18270 [details]
introduce chunked transfer

Unsetting review flag as this patch has known issues that prevent it from being reviewable.
Comment 9 Luca Bruno 2008-01-04 14:14:02 PST
Created attachment 18277 [details]
file checks

Check for file opening and reading, and in case cancel the job.
Comment 10 Luca Bruno 2008-01-04 15:12:53 PST
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.
Comment 11 Luca Bruno 2008-01-07 06:18:02 PST
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.
Comment 12 Luca Bruno 2008-01-07 06:29:58 PST
Created attachment 18313 [details]
return if empty POST
Comment 13 Alp Toker 2008-01-11 17:28:53 PST
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!
Comment 14 Alp Toker 2008-01-11 17:36:25 PST
Landed in r29423.