[Webkit-unassigned] [Bug 119436] [curl] Improve detecting and handling of SSL related errors

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=119436


Brent Fulgham <bfulgham at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #216791|review?                     |review-
               Flag|                            |




--- Comment #22 from Brent Fulgham <bfulgham at webkit.org>  2013-11-13 09:16:36 PST ---
(From update of attachment 216791)
View in context: https://bugs.webkit.org/attachment.cgi?id=216791&action=review

Looks very good!  A few style problems need to be cleaned up, but otherwise this looks great!  Thanks for getting this functionality in place.

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:618
> +                ResourceError resourceError = ResourceError(String(), msg->data.result, String(url), String(curl_easy_strerror(msg->data.result)));

This construct is a little funny.  Why not just declare resourceError with construction in place?

ResourceError resourceError(String(), msg->data.result, String(url), String(curl_easy_strerror(msg->data.result));

> Source/WebCore/platform/network/curl/SSLHandle.cpp:122
> +bool pemData(X509_STORE_CTX *ctx, String& certificate)

We write this as X509_STORE_CTX*

> Source/WebCore/platform/network/curl/SSLHandle.cpp:142
> +static int certVerifyCallback(int ok, X509_STORE_CTX *ctx)

X509_STORE_CTX*

> Source/WebCore/platform/network/curl/SSLHandle.cpp:150
> +    SSL* ssl = (SSL*)X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx());

This should be a C++-style cast (prob. a reinterpret_cast<SSL*>)

> Source/WebCore/platform/network/curl/SSLHandle.cpp:152
> +    ResourceHandle* job  = (ResourceHandle*)SSL_CTX_get_app_data(sslctx);

C++ style cast.

> Source/WebCore/platform/network/curl/SSLHandle.cpp:170
> +        // so don't need to curl verifies the authenticity of the peer's certificate

Because of this large comment, you should wrap the comment and curl_easy_setopt in brackets to make the structure clear:

if (ok) {
    // blah blah
    curl_easy_setopt(...)
}

> Source/WebCore/platform/network/curl/SSLHandle.cpp:175
> +static CURLcode sslctxfun(CURL * curl, void * sslctx, void * parm)

CURL* curl, void* sslctx, void* parm

> Source/WebCore/platform/network/curl/SSLHandle.cpp:177
> +    SSL_CTX_set_app_data((SSL_CTX*)sslctx, parm);

C++-style casts, please.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.