[Soup] Clean up ResourceError creation
authormrobinson@webkit.org <mrobinson@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Oct 2012 00:35:35 +0000 (00:35 +0000)
committermrobinson@webkit.org <mrobinson@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Oct 2012 00:35:35 +0000 (00:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=98521

Reviewed by Carlos Garcia Campos.

Simplify the creation of ResourcErrors in ResourceHandleSoup. This is
part of a process to make the libsoup networking backend more hackable.

No new tests. This shouldn't change functionality.

* GNUmakefile.list.am: Added new file.
* PlatformEfl.cmake: Added new file.
* platform/network/soup/ResourceError.h:
(ResourceError): Added new factories.
* platform/network/soup/ResourceErrorSoup.cpp: Added.
(WebCore::failingURI): Added this helper.
(WebCore::ResourceError::httpError): New factory.
(WebCore::ResourceError::genericIOError): Ditto.
(WebCore::ResourceError::tlsError): Ditto.
(WebCore::ResourceError::timeoutError): Ditto.
* platform/network/soup/ResourceHandleSoup.cpp:
(WebCore::handleUnignoredTLSErrors): Created this helper which merges
some of the logic from sendRequestCallback.
(WebCore::sendRequestCallback): Use the new helper.
(WebCore::requestTimeoutCallback): Use the new factory.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@130613 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/GNUmakefile.list.am
Source/WebCore/PlatformEfl.cmake
Source/WebCore/platform/network/soup/ResourceError.h
Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp [new file with mode: 0644]
Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp

index 83d15f7..6f04da2 100644 (file)
@@ -1,3 +1,31 @@
+2012-10-07  Martin Robinson  <mrobinson@igalia.com>
+
+        [Soup] Clean up ResourceError creation
+        https://bugs.webkit.org/show_bug.cgi?id=98521
+
+        Reviewed by Carlos Garcia Campos.
+
+        Simplify the creation of ResourcErrors in ResourceHandleSoup. This is
+        part of a process to make the libsoup networking backend more hackable.
+
+        No new tests. This shouldn't change functionality.
+
+        * GNUmakefile.list.am: Added new file.
+        * PlatformEfl.cmake: Added new file.
+        * platform/network/soup/ResourceError.h:
+        (ResourceError): Added new factories.
+        * platform/network/soup/ResourceErrorSoup.cpp: Added.
+        (WebCore::failingURI): Added this helper.
+        (WebCore::ResourceError::httpError): New factory.
+        (WebCore::ResourceError::genericIOError): Ditto.
+        (WebCore::ResourceError::tlsError): Ditto.
+        (WebCore::ResourceError::timeoutError): Ditto.
+        * platform/network/soup/ResourceHandleSoup.cpp:
+        (WebCore::handleUnignoredTLSErrors): Created this helper which merges
+        some of the logic from sendRequestCallback.
+        (WebCore::sendRequestCallback): Use the new helper.
+        (WebCore::requestTimeoutCallback): Use the new factory.
+
 2012-10-07  Caio Marcelo de Oliveira Filho  <caio.oliveira@openbossa.org>
 
         Rename first/second to key/value in HashMap iterators
index 1e97b57..d7a4ede 100644 (file)
@@ -4701,6 +4701,7 @@ webcore_sources += \
        Source/WebCore/platform/network/soup/GOwnPtrSoup.h \
        Source/WebCore/platform/network/soup/ProxyServerSoup.cpp \
        Source/WebCore/platform/network/soup/ResourceError.h \
+       Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp \
        Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp \
        Source/WebCore/platform/network/soup/ResourceRequest.h \
        Source/WebCore/platform/network/soup/ResourceRequestSoup.cpp \
index 3d16015..450c249 100644 (file)
@@ -75,6 +75,7 @@ LIST(APPEND WebCore_SOURCES
   platform/network/soup/GOwnPtrSoup.cpp
   platform/network/soup/ProxyResolverSoup.cpp
   platform/network/soup/ProxyServerSoup.cpp
+  platform/network/soup/ResourceErrorSoup.cpp
   platform/network/soup/ResourceHandleSoup.cpp
   platform/network/soup/ResourceRequestSoup.cpp
   platform/network/soup/ResourceResponseSoup.cpp
index 37880e5..766da0b 100644 (file)
 #include <wtf/gobject/GRefPtr.h>
 
 typedef struct _GTlsCertificate GTlsCertificate;
+typedef struct _SoupRequest SoupRequest;
+typedef struct _SoupMessage SoupMessage;
 
 namespace WebCore {
 
 class ResourceError : public ResourceErrorBase
 {
 public:
-    ResourceError()
-        : m_tlsErrors(0)
-    {
-    }
-
     ResourceError(const String& domain, int errorCode, const String& failingURL, const String& localizedDescription)
         : ResourceErrorBase(domain, errorCode, failingURL, localizedDescription)
         , m_tlsErrors(0)
     {
     }
 
-    ResourceError(const String& domain, int errorCode, const String& failingURL, const String& localizedDescription, unsigned tlsErrors, GTlsCertificate* certificate)
-        : ResourceErrorBase(domain, errorCode, failingURL, localizedDescription)
-        , m_tlsErrors(tlsErrors)
-        , m_certificate(certificate)
+    ResourceError()
+        : m_tlsErrors(0)
     {
     }
 
+    static ResourceError httpError(SoupMessage*, GError*, SoupRequest*);
+    static ResourceError genericIOError(GError*, SoupRequest*);
+    static ResourceError tlsError(SoupRequest*, unsigned tlsErrors, GTlsCertificate*);
+    static ResourceError timeoutError(const String& failingURL);
+
     unsigned tlsErrors() const { return m_tlsErrors; }
     GTlsCertificate* certificate() const { return m_certificate.get(); }
 
diff --git a/Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp b/Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp
new file mode 100644 (file)
index 0000000..ee748dc
--- /dev/null
@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2012 Igalia S.L.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY IGALIA S.L. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "ResourceError.h"
+
+#include "LocalizedStrings.h"
+#define LIBSOUP_USE_UNSTABLE_REQUEST_API
+#include <libsoup/soup-request.h>
+#include <libsoup/soup.h>
+#include <wtf/gobject/GOwnPtr.h>
+#include <wtf/text/CString.h>
+
+namespace WebCore {
+
+static String failingURI(SoupRequest* request)
+{
+    ASSERT(request);
+    GOwnPtr<char> uri(soup_uri_to_string(soup_request_get_uri(request), FALSE));
+    return uri.get();
+}
+
+ResourceError ResourceError::httpError(SoupMessage* message, GError* error, SoupRequest* request)
+{
+    if (!message || !SOUP_STATUS_IS_TRANSPORT_ERROR(message->status_code))
+        return genericIOError(error, request);
+    return ResourceError(g_quark_to_string(SOUP_HTTP_ERROR), message->status_code,
+        failingURI(request), String::fromUTF8(message->reason_phrase));
+}
+
+ResourceError ResourceError::genericIOError(GError* error, SoupRequest* request)
+{
+    return ResourceError(g_quark_to_string(G_IO_ERROR), error->code,
+        failingURI(request), String::fromUTF8(error->message));
+}
+
+ResourceError ResourceError::tlsError(SoupRequest* request, unsigned tlsErrors, GTlsCertificate* certificate)
+{
+    return ResourceError(g_quark_to_string(SOUP_HTTP_ERROR), SOUP_STATUS_SSL_FAILED,
+        failingURI(request), unacceptableTLSCertificate());
+}
+
+ResourceError ResourceError::timeoutError(const String& failingURL)
+{
+    // FIXME: This should probably either be integrated into Errors(Gtk/EFL).h or the
+    // networking errors from those files should be moved here.
+
+    // Use the same value as in NSURLError.h
+    static const int timeoutError = -1001;
+    static const char* const  errorDomain = "WebKitNetworkError";
+    ResourceError error = ResourceError(errorDomain, timeoutError, failingURL, "Request timed out");
+    error.setIsTimeout(true);
+    return error;
+}
+
+} // namespace WebCore
index 2111621..fbcbe5b 100644 (file)
@@ -72,9 +72,6 @@ namespace WebCore {
 
 #define READ_BUFFER_SIZE 8192
 
-// Use the same value as in NSURLError.h
-static const int gTimeoutError = -1001;
-
 static bool loadingSynchronousRequest = false;
 
 class WebCoreSynchronousLoader : public ResourceHandleClient {
@@ -399,31 +396,25 @@ static void cleanupSoupRequestOperation(ResourceHandle* handle, bool isDestroyin
         handle->deref();
 }
 
-static ResourceError convertSoupErrorToResourceError(GError* error, SoupRequest* request, SoupMessage* message = 0)
+static bool handleUnignoredTLSErrors(ResourceHandle* handle)
 {
-    ASSERT(error);
-    ASSERT(request);
-
-    GOwnPtr<char> uri(soup_uri_to_string(soup_request_get_uri(request), FALSE));
-    if (message && SOUP_STATUS_IS_TRANSPORT_ERROR(message->status_code)) {
-        return ResourceError(g_quark_to_string(SOUP_HTTP_ERROR),
-                             static_cast<gint>(message->status_code),
-                             uri.get(),
-                             String::fromUTF8(message->reason_phrase));
-    }
+    ResourceHandleInternal* d = handle->getInternal();
+    const ResourceResponse& response = d->m_response;
 
-    // Non-transport errors are handled differently.
-    return ResourceError(g_quark_to_string(G_IO_ERROR),
-                         error->code,
-                         uri.get(),
-                         String::fromUTF8(error->message));
-}
+    if (!response.soupMessageTLSErrors() || gIgnoreSSLErrors)
+        return false;
 
-static inline bool hasUnignoredTLSErrors(ResourceHandle* handle)
-{
-    return handle->getInternal()->m_response.soupMessageTLSErrors()
-        && !gIgnoreSSLErrors
-        && !allowsAnyHTTPSCertificateHosts().contains(handle->firstRequest().url().host().lower());
+    String lowercaseHostURL = handle->firstRequest().url().host().lower();
+    if (allowsAnyHTTPSCertificateHosts().contains(lowercaseHostURL))
+        return false;
+
+    // We aren't ignoring errors globally, but the user may have already decided to accept this certificate.
+    CertificatesMap::iterator i = clientCertificates().find(lowercaseHostURL);
+    if (i != clientCertificates().end() && i->value.contains(response.soupMessageCertificate()))
+        return false;
+
+    handle->client()->didFail(handle, ResourceError::tlsError(d->m_soupRequest.get(), response.soupMessageTLSErrors(), response.soupMessageCertificate()));
+    return true;
 }
 
 static void sendRequestCallback(GObject*, GAsyncResult* res, gpointer data)
@@ -447,7 +438,7 @@ static void sendRequestCallback(GObject*, GAsyncResult* res, gpointer data)
     GOwnPtr<GError> error;
     GInputStream* in = soup_request_send_finish(d->m_soupRequest.get(), res, &error.outPtr());
     if (error) {
-        client->didFail(handle.get(), convertSoupErrorToResourceError(error.get(), d->m_soupRequest.get(), soupMessage));
+        client->didFail(handle.get(), ResourceError::httpError(soupMessage, error.get(), d->m_soupRequest.get()));
         cleanupSoupRequestOperation(handle.get());
         return;
     }
@@ -462,17 +453,11 @@ static void sendRequestCallback(GObject*, GAsyncResult* res, gpointer data)
         }
         d->m_response.updateFromSoupMessage(soupMessage);
 
-        if (hasUnignoredTLSErrors(handle.get())) {
-            CertificatesMap::iterator iter = clientCertificates().find(handle->firstRequest().url().host().lower());
-            if (iter == clientCertificates().end() || !iter->value.contains(d->m_response.soupMessageCertificate())) {
-                GOwnPtr<char> uri(soup_uri_to_string(soup_request_get_uri(d->m_soupRequest.get()), FALSE));
-                client->didFail(handle.get(), ResourceError(g_quark_to_string(SOUP_HTTP_ERROR), SOUP_STATUS_SSL_FAILED,
-                                                            uri.get(), unacceptableTLSCertificate(),
-                                                            d->m_response.soupMessageTLSErrors(), d->m_response.soupMessageCertificate()));
-                cleanupSoupRequestOperation(handle.get());
-                return;
-            }
+        if (handleUnignoredTLSErrors(handle.get())) {
+            cleanupSoupRequestOperation(handle.get());
+            return;
         }
+
     } else {
         d->m_response.setURL(handle->firstRequest().url());
         const gchar* contentType = soup_request_get_content_type(d->m_soupRequest.get());
@@ -982,7 +967,7 @@ static void readCallback(GObject*, GAsyncResult* asyncResult, gpointer data)
     GOwnPtr<GError> error;
     gssize bytesRead = g_input_stream_read_finish(d->m_inputStream.get(), asyncResult, &error.outPtr());
     if (error) {
-        client->didFail(handle.get(), convertSoupErrorToResourceError(error.get(), d->m_soupRequest.get()));
+        client->didFail(handle.get(), ResourceError::genericIOError(error.get(), d->m_soupRequest.get()));
         cleanupSoupRequestOperation(handle.get());
         return;
     }
@@ -1019,12 +1004,8 @@ static void readCallback(GObject*, GAsyncResult* asyncResult, gpointer data)
 static gboolean requestTimeoutCallback(gpointer data)
 {
     RefPtr<ResourceHandle> handle = static_cast<ResourceHandle*>(data);
-    ResourceHandleInternal* d = handle->getInternal();
-    ResourceHandleClient* client = handle->client();
-
-    ResourceError timeoutError("WebKitNetworkError", gTimeoutError, d->m_firstRequest.url().string(), "Request timed out");
-    timeoutError.setIsTimeout(true);
-    client->didFail(handle.get(), timeoutError);
+    handle->client()->didFail(handle.get(), ResourceError::timeoutError(handle->getInternal()->m_firstRequest.url().string()));
+    cleanupSoupRequestOperation(handle.get());
     handle->cancel();
 
     return FALSE;