Bug 118468 - [Curl] Download fails for certain urls.
Summary: [Curl] Download fails for certain urls.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Curl
Depends on:
Blocks:
 
Reported: 2013-07-08 05:54 PDT by peavo
Modified: 2013-07-22 09:34 PDT (History)
3 users (show)

See Also:


Attachments
Patch (9.96 KB, patch)
2013-07-08 10:58 PDT, peavo
no flags Details | Formatted Diff | Diff
Patch (10.05 KB, patch)
2013-07-22 08:49 PDT, peavo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.