Currently, if a TLS error occurs (particularly certificate errors), the certificate and libsoup flags are lost.
Created attachment 225732 [details] Patch
Attachment 225732 [details] did not pass style-queue: Total errors found: 0 in 0 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 225732 [details] Patch The patch is empty.
Oops. Seems I botched the Tools/Scripts tool I used (on a different machine). I'm using the git repo; what's the proper command to use for submitting a single patch from a branch (I think I used webkit-patch -g HEAD~ 129653 patch-name)? I'll respin once I get home.
Created attachment 225858 [details] Set-the-TLS-error-information-if-a-TLS-error-occurred FWIW, here's the command I used to attach to the bug: "Tools/Scripts/webkit-patch attach-to-bug -g HEAD -m Patch 129653 TLS-error-propagation.patch".
Attachment 225858 [details] did not pass style-queue: ERROR: Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp:61: Declaration has space between type name and * in GTlsCertificate *certificate [whitespace/declaration] [3] ERROR: Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp:64: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp:57: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] Total errors found: 3 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 225865 [details] Set-the-TLS-error-information-if-a-TLS-error-occurred (2) Style errors fixed.
Comment on attachment 225865 [details] Set-the-TLS-error-information-if-a-TLS-error-occurred (2) View in context: https://bugs.webkit.org/attachment.cgi?id=225865&action=review What does this fix or how does this improve things? Are you fixing a specific bug? > Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp:57 > + if (message && SOUP_STATUS_IS_TRANSPORT_ERROR(message->status_code)) { This should be turned into an early return, so reverse the check and return generic error immediately. > Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp:61 > + ResourceError error = transportError(request, message->status_code, > String::fromUTF8(message->reason_phrase)); > - else > - return genericGError(error, request); > + if (message->status_code == SOUP_STATUS_SSL_FAILED) { > + GTlsCertificate* certificate; Nit: an empty line before the if will make this more readable I think.
Thanks for the review. (In reply to comment #8) > What does this fix or how does this improve things? Are you fixing a specific bug? That's filed in bugzilla? No. The problem is that when an SSL error occurs, there's no information available for a browser to say "here's the certificate that failed; do you want to ignore the error?". Currently, there's nothing there since the generic error doesn't contain such information. Of course, things may have changed since I last checked; I'll see if I can get my WebKit repository up to speed tonight and double check the state of things (i.e., whether the patch is necessary and still provides the correct information). > > Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp:57 > > + if (message && SOUP_STATUS_IS_TRANSPORT_ERROR(message->status_code)) { > > This should be turned into an early return, so reverse the check and return generic error immediately. Sounds good. > > Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp:61 > > + ResourceError error = transportError(request, message->status_code, > > String::fromUTF8(message->reason_phrase)); > > - else > > - return genericGError(error, request); > > + if (message->status_code == SOUP_STATUS_SSL_FAILED) { > > + GTlsCertificate* certificate; > > Nit: an empty line before the if will make this more readable I think. Will change.