Bug 28171 - [cURL] Support https protocol in cURL builds
Summary: [cURL] Support https protocol in cURL builds
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-10 23:46 PDT by Brent Fulgham
Modified: 2009-08-11 16:15 PDT (History)
0 users

See Also:


Attachments
Activates SSL support if cURL build supports it. (3.29 KB, patch)
2009-08-10 23:52 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Enable SSL support in cURL builds with SSL enabled. (3.43 KB, patch)
2009-08-11 13:49 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Small revision to ChangeLog to point to the right bug. (3.44 KB, patch)
2009-08-11 13:53 PDT, Brent Fulgham
gustavo: review+
gustavo: commit-queue+
Details | Formatted Diff | Diff
Revised to improve efficiency and thread safety. (4.09 KB, patch)
2009-08-11 15:20 PDT, Brent Fulgham
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2009-08-10 23:46:12 PDT
The WinCairo port of WebKit does not currently support the https protocol.  The attached patch corrects this problem (thanks to the work of the Appcelerator team).
Comment 1 Brent Fulgham 2009-08-10 23:52:15 PDT
Created attachment 34542 [details]
Activates SSL support if cURL build supports it.
Comment 2 Brent Fulgham 2009-08-10 23:53:10 PDT
Note:  This might resolve Bug 19146, as the certificates were never being specified, so no SSL-supporting cURL would have been able to negotiate the HTTPS protocol.
Comment 3 Brent Fulgham 2009-08-11 13:49:18 PDT
Created attachment 34588 [details]
Enable SSL support in cURL builds with SSL enabled.
Comment 4 Brent Fulgham 2009-08-11 13:53:26 PDT
Created attachment 34589 [details]
Small revision to ChangeLog to point to the right bug.
Comment 5 Gustavo Noronha (kov) 2009-08-11 14:31:01 PDT
Comment on attachment 34589 [details]
Small revision to ChangeLog to point to the right bug.

Notice that I don't really know CF enough to review this properly. The patch looks straight-forward enough though, and I can see no obvious problems, so rs=me that.
Comment 6 Brent Fulgham 2009-08-11 15:20:11 PDT
Created attachment 34603 [details]
Revised to improve efficiency and thread safety.

Modified based on suggestions from ap and aroben.
Comment 7 Adam Roben (:aroben) 2009-08-11 15:26:37 PDT
Comment on attachment 34603 [details]
Revised to improve efficiency and thread safety.

> +static CString certificateBundlePath()

"certificateBundlePath" is a little confusing. This doesn't return a path to a "certificate bundle"; it returns a path to a certificate within the WebKit bundle. I think "certificatePath" would be clearer.

> +{
> +#if PLATFORM(CF)
> +    CFBundleRef webKitBundle = CFBundleGetBundleWithIdentifier(CFSTR("com.apple.WebKit"));
> +    RetainPtr<CFURLRef> certURLRef(AdoptCF, CFBundleCopyResourceURL(webKitBundle, CFSTR("cacert"), CFSTR("pem"), CFSTR("certificates")));
> +    if (certURLRef) {
> +        char path[MAX_PATH];
> +        CFURLGetFileSystemRepresentation(certURLRef.get(), false, (UInt8*)path, MAX_PATH);

reinterpret_cast is preferred to a C-style cast.

> +    , m_certPath (certificateBundlePath())

You have an extra space after "m_certPath".

> +    if (!m_certPath.isNull())
> +       curl_easy_setopt(d->m_handle, CURLOPT_CAINFO, m_certPath.data());

Checking isEmpty() seems a bit better than checking isNull().

> +    const CString m_certPath;

m_certificatePath seems a little clearer. I don't think the abbreviation adds anything here.

r=me
Comment 8 Brent Fulgham 2009-08-11 16:15:49 PDT
Landed in http://trac.webkit.org/changeset/47071.