Bug 118468

Summary: [Curl] Download fails for certain urls.
Product: WebKit Reporter: peavo
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, galpeter
Priority: P2 Keywords: Curl
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description peavo 2013-07-08 05:54:32 PDT
The current Curl download implementation has a few shortcomings:

1) Downloading from secure locations fails. We need to provide Curl with the path to a certificate file (.pem file).

2) Cookies are not set in the download request. We need to give Curl the path to the cookie file.

3) When a normal load is converted to a download, some of the headers from the original request is not sent (e.g. Referer, User agent).
Comment 1 peavo 2013-07-08 10:58:45 PDT
Created attachment 206260 [details]
Patch
Comment 2 Brent Fulgham 2013-07-20 18:21:16 PDT
Comment on attachment 206260 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=206260&action=review

Looks good.  Please address my minor comments when you land!

> Source/WebCore/platform/network/curl/CurlDownload.cpp:352
> +            String value = it->value;

This could be a const reference to avoid a copy.

> Source/WebCore/platform/network/curl/CurlDownload.cpp:353
> +            String headerString(key);

You convert an AtomicString to a String in 351, then make a copy here.  Why not just do "String headerString(it->key);"?

> Source/WebCore/platform/network/curl/CurlDownload.cpp:405
> +    writeDataToFile(static_cast<char*>(data), size);

Why are you casting to a char*?  The function signature is "const char*".
Comment 3 peavo 2013-07-22 08:49:09 PDT
Created attachment 207250 [details]
Patch
Comment 4 peavo 2013-07-22 08:51:31 PDT
(In reply to comment #2)

Thanks for reviewing! Updated patch accordingly.

> (From update of attachment 206260 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=206260&action=review
> 
> Looks good.  Please address my minor comments when you land!
> 
> > Source/WebCore/platform/network/curl/CurlDownload.cpp:352
> > +            String value = it->value;
> 
> This could be a const reference to avoid a copy.
> 
> > Source/WebCore/platform/network/curl/CurlDownload.cpp:353
> > +            String headerString(key);
> 
> You convert an AtomicString to a String in 351, then make a copy here.  Why not just do "String headerString(it->key);"?
> 
> > Source/WebCore/platform/network/curl/CurlDownload.cpp:405
> > +    writeDataToFile(static_cast<char*>(data), size);
> 
> Why are you casting to a char*?  The function signature is "const char*".
Comment 5 Brent Fulgham 2013-07-22 08:51:38 PDT
Comment on attachment 207250 [details]
Patch

Nice work!  r=me.
Comment 6 WebKit Commit Bot 2013-07-22 09:34:00 PDT
Comment on attachment 207250 [details]
Patch

Clearing flags on attachment: 207250

Committed r152976: <http://trac.webkit.org/changeset/152976>
Comment 7 WebKit Commit Bot 2013-07-22 09:34:02 PDT
All reviewed patches have been landed.  Closing bug.