[GTK] WebKitWebView::load-failed-with-tls-errors should receive the failing URI inste...
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Sep 2014 16:11:12 +0000 (16:11 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Sep 2014 16:11:12 +0000 (16:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=136889

Reviewed by Gustavo Noronha Silva.

Source/WebKit2:

We were passing a host for two different reasons, first because
it's more convenient to add an exception with
webkit_web_context_allow_tls_certificate_for_host(), but also
because we were assuming the active URI is the failing URI in case
of failure. This assumption is correct because our current code
does that, but I'm not sure we are doing it on purpose. That
behaviour is not documented anywhere and it's not what WebKit2 does
internaly.

* UIProcess/API/gtk/WebKitWebView.cpp:
(webkit_web_view_class_init):
(webkitWebViewLoadFailedWithTLSErrors):
* UIProcess/API/gtk/WebKitWebView.h:

Tools:

* TestWebKitAPI/Tests/WebKit2Gtk/TestSSL.cpp:
(testLoadFailedWithTLSErrors): Check that LoadFailedWithTLSErrors
event was added to the events vector.
* TestWebKitAPI/gtk/WebKit2Gtk/LoadTrackingTest.cpp:
(loadFailedCallback): Do not assume the web view URI is the
failing URI when the load fails before the committed state.
(loadFailedWithTLSErrorsCallback): Handle the case of load failure
because of TLS errors can call LoadTrackingTest::loadFailedWithTLSErrors.
(LoadTrackingTest::LoadTrackingTest): Connect to WebKitWebView::load-failed-with-tls-errors.
(LoadTrackingTest::loadFailedWithTLSErrors): Add
LoadFailedWithTLSErrors event to the events vector.
* TestWebKitAPI/gtk/WebKit2Gtk/LoadTrackingTest.h:

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

Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestSSL.cpp
Tools/TestWebKitAPI/gtk/WebKit2Gtk/LoadTrackingTest.cpp
Tools/TestWebKitAPI/gtk/WebKit2Gtk/LoadTrackingTest.h

index 0b5cdff..3104333 100644 (file)
@@ -1,3 +1,24 @@
+2014-09-17  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] WebKitWebView::load-failed-with-tls-errors should receive the failing URI instead of a host
+        https://bugs.webkit.org/show_bug.cgi?id=136889
+
+        Reviewed by Gustavo Noronha Silva.
+
+        We were passing a host for two different reasons, first because
+        it's more convenient to add an exception with
+        webkit_web_context_allow_tls_certificate_for_host(), but also
+        because we were assuming the active URI is the failing URI in case
+        of failure. This assumption is correct because our current code
+        does that, but I'm not sure we are doing it on purpose. That
+        behaviour is not documented anywhere and it's not what WebKit2 does
+        internaly.
+
+        * UIProcess/API/gtk/WebKitWebView.cpp:
+        (webkit_web_view_class_init):
+        (webkitWebViewLoadFailedWithTLSErrors):
+        * UIProcess/API/gtk/WebKitWebView.h:
+
 2014-09-16  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] Fix layering violations in PasteboardGtk
index a8df4bc..9057c34 100644 (file)
@@ -873,13 +873,13 @@ static void webkit_web_view_class_init(WebKitWebViewClass* webViewClass)
     /**
      * WebKitWebView::load-failed-with-tls-errors:
      * @web_view: the #WebKitWebView on which the signal is emitted
+     * @failing_uri: the URI that failed to load
      * @certificate: a #GTlsCertificate
      * @errors: a #GTlsCertificateFlags with the verification status of @certificate
-     * @host: the host on which the error occurred
      *
      * Emitted when a TLS error occurs during a load operation.
-     * To allow an exception for this certificate
-     * and this host use webkit_web_context_allow_tls_certificate_for_host().
+     * To allow an exception for this @certificate
+     * and the host of @failing_uri use webkit_web_context_allow_tls_certificate_for_host().
      *
      * To handle this signal asynchronously you should call g_object_ref() on @certificate
      * and return %TRUE.
@@ -900,9 +900,9 @@ static void webkit_web_view_class_init(WebKitWebViewClass* webViewClass)
             g_signal_accumulator_true_handled, 0 /* accumulator data */,
             g_cclosure_marshal_generic,
             G_TYPE_BOOLEAN, 3,
+            G_TYPE_STRING,
             G_TYPE_TLS_CERTIFICATE,
-            G_TYPE_TLS_CERTIFICATE_FLAGS,
-            G_TYPE_STRING);
+            G_TYPE_TLS_CERTIFICATE_FLAGS);
 
     /**
      * WebKitWebView::create:
@@ -1584,9 +1584,8 @@ void webkitWebViewLoadFailedWithTLSErrors(WebKitWebView* webView, const char* fa
 
     WebKitTLSErrorsPolicy tlsErrorsPolicy = webkit_web_context_get_tls_errors_policy(webView->priv->context);
     if (tlsErrorsPolicy == WEBKIT_TLS_ERRORS_POLICY_FAIL) {
-        GUniquePtr<SoupURI> soupURI(soup_uri_new(failingURI));
         gboolean returnValue;
-        g_signal_emit(webView, signals[LOAD_FAILED_WITH_TLS_ERRORS], 0, certificate, tlsErrors, soupURI->host, &returnValue);
+        g_signal_emit(webView, signals[LOAD_FAILED_WITH_TLS_ERRORS], 0, failingURI, certificate, tlsErrors, &returnValue);
         if (!returnValue)
             g_signal_emit(webView, signals[LOAD_FAILED], 0, WEBKIT_LOAD_STARTED, failingURI, error, &returnValue);
     }
index 4eba0cb..f3f059c 100644 (file)
@@ -235,9 +235,9 @@ struct _WebKitWebViewClass {
     gboolean   (* authenticate)                (WebKitWebView               *web_view,
                                                 WebKitAuthenticationRequest *request);
     gboolean   (* load_failed_with_tls_errors) (WebKitWebView               *web_view,
+                                                const gchar                 *failing_uri,
                                                 GTlsCertificate             *certificate,
-                                                GTlsCertificateFlags         errors,
-                                                const gchar                 *host);
+                                                GTlsCertificateFlags         errors);
     void (*_webkit_reserved0) (void);
     void (*_webkit_reserved1) (void);
     void (*_webkit_reserved2) (void);
index e06b23c..5fa51c3 100644 (file)
@@ -1,3 +1,23 @@
+2014-09-17  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] WebKitWebView::load-failed-with-tls-errors should receive the failing URI instead of a host
+        https://bugs.webkit.org/show_bug.cgi?id=136889
+
+        Reviewed by Gustavo Noronha Silva.
+
+        * TestWebKitAPI/Tests/WebKit2Gtk/TestSSL.cpp:
+        (testLoadFailedWithTLSErrors): Check that LoadFailedWithTLSErrors
+        event was added to the events vector.
+        * TestWebKitAPI/gtk/WebKit2Gtk/LoadTrackingTest.cpp:
+        (loadFailedCallback): Do not assume the web view URI is the
+        failing URI when the load fails before the committed state.
+        (loadFailedWithTLSErrorsCallback): Handle the case of load failure
+        because of TLS errors can call LoadTrackingTest::loadFailedWithTLSErrors.
+        (LoadTrackingTest::LoadTrackingTest): Connect to WebKitWebView::load-failed-with-tls-errors.
+        (LoadTrackingTest::loadFailedWithTLSErrors): Add
+        LoadFailedWithTLSErrors event to the events vector.
+        * TestWebKitAPI/gtk/WebKit2Gtk/LoadTrackingTest.h:
+
 2014-09-17  Renato Nagy  <rnagy@inf.u-szeged.hu>
 
         [EFL][GTK] Remove WebKit1 related codes
index b351725..b2ad543 100644 (file)
@@ -150,43 +150,37 @@ public:
 
     TLSErrorsTest()
         : m_tlsErrors(static_cast<GTlsCertificateFlags>(0))
+        , m_failingURI(nullptr)
     {
-        g_signal_connect(m_webView, "load-failed-with-tls-errors", G_CALLBACK(runLoadFailedWithTLSErrorsCallback), this);
     }
 
     ~TLSErrorsTest()
     {
-        g_signal_handlers_disconnect_matched(m_webView, G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, this);
+        if (m_failingURI)
+            soup_uri_free(m_failingURI);
     }
 
-    static gboolean runLoadFailedWithTLSErrorsCallback(WebKitWebView*, GTlsCertificate* certificate, GTlsCertificateFlags tlsErrors, const char* host, TLSErrorsTest* test)
+    bool loadFailedWithTLSErrors(const char* failingURI, GTlsCertificate* certificate, GTlsCertificateFlags tlsErrors) override
     {
-        test->runLoadFailedWithTLSErrors(certificate, tlsErrors, host);
-        return TRUE;
-    }
+        LoadTrackingTest::loadFailedWithTLSErrors(failingURI, certificate, tlsErrors);
 
-    void runLoadFailedWithTLSErrors(GTlsCertificate* certificate, GTlsCertificateFlags tlsErrors, const char* host)
-    {
         assertObjectIsDeletedWhenTestFinishes(G_OBJECT(certificate));
         m_certificate = certificate;
         m_tlsErrors = tlsErrors;
-        m_host.reset(g_strdup(host));
-        g_main_loop_quit(m_mainLoop);
-    }
-
-    void waitUntilLoadFailedWithTLSErrors()
-    {
-        g_main_loop_run(m_mainLoop);
+        if (m_failingURI)
+            soup_uri_free(m_failingURI);
+        m_failingURI = soup_uri_new(failingURI);
+        return true;
     }
 
     GTlsCertificate* certificate() const { return m_certificate.get(); }
     GTlsCertificateFlags tlsErrors() const { return m_tlsErrors; }
-    const char* host() const { return m_host.get(); }
+    const char* host() const { return m_failingURI->host; }
 
 private:
     GRefPtr<GTlsCertificate> m_certificate;
     GTlsCertificateFlags m_tlsErrors;
-    GUniquePtr<char> m_host;
+    SoupURI* m_failingURI;
 };
 
 static void testLoadFailedWithTLSErrors(TLSErrorsTest* test, gconstpointer)
@@ -196,13 +190,13 @@ static void testLoadFailedWithTLSErrors(TLSErrorsTest* test, gconstpointer)
 
     // The load-failed-with-tls-errors signal should be emitted when there is a TLS failure.
     test->loadURI(kHttpsServer->getURIForPath("/test-tls/").data());
-    test->waitUntilLoadFailedWithTLSErrors();
-    // Test the WebKitCertificateInfo API.
+    test->waitUntilLoadFinished();
     g_assert(G_IS_TLS_CERTIFICATE(test->certificate()));
     g_assert_cmpuint(test->tlsErrors(), ==, G_TLS_CERTIFICATE_UNKNOWN_CA);
     g_assert_cmpstr(test->host(), ==, soup_uri_get_host(kHttpsServer->baseURI()));
     g_assert_cmpint(test->m_loadEvents[0], ==, LoadTrackingTest::ProvisionalLoadStarted);
-    g_assert_cmpint(test->m_loadEvents[1], ==, LoadTrackingTest::LoadFinished);
+    g_assert_cmpint(test->m_loadEvents[1], ==, LoadTrackingTest::LoadFailedWithTLSErrors);
+    g_assert_cmpint(test->m_loadEvents[2], ==, LoadTrackingTest::LoadFinished);
 
     // Test allowing an exception for this certificate on this host.
     webkit_web_context_allow_tls_certificate_for_host(context, test->certificate(), test->host());
index 026a79f..9aa3238 100644 (file)
@@ -68,7 +68,7 @@ static void loadFailedCallback(WebKitWebView* webView, WebKitLoadEvent loadEvent
     switch (loadEvent) {
     case WEBKIT_LOAD_STARTED:
         g_assert(!webkit_web_view_is_loading(webView));
-        g_assert_cmpstr(test->m_activeURI.data(), ==, webkit_web_view_get_uri(webView));
+        g_assert_cmpstr(test->m_activeURI.data(), ==, failingURI);
         g_assert(error);
         test->provisionalLoadFailed(failingURI, error);
         break;
@@ -83,6 +83,16 @@ static void loadFailedCallback(WebKitWebView* webView, WebKitLoadEvent loadEvent
     }
 }
 
+static gboolean loadFailedWithTLSErrorsCallback(WebKitWebView* webView, const char* failingURI, GTlsCertificate* certificate, GTlsCertificateFlags tlsErrors, LoadTrackingTest* test)
+{
+    test->m_loadFailed = true;
+    g_assert(!webkit_web_view_is_loading(webView));
+    g_assert_cmpstr(test->m_activeURI.data(), ==, failingURI);
+    g_assert(G_IS_TLS_CERTIFICATE(certificate));
+    g_assert(tlsErrors);
+    return test->loadFailedWithTLSErrors(failingURI, certificate, tlsErrors);
+}
+
 static void estimatedProgressChangedCallback(GObject*, GParamSpec*, LoadTrackingTest* test)
 {
     test->estimatedProgressChanged();
@@ -94,6 +104,7 @@ LoadTrackingTest::LoadTrackingTest()
 {
     g_signal_connect(m_webView, "load-changed", G_CALLBACK(loadChangedCallback), this);
     g_signal_connect(m_webView, "load-failed", G_CALLBACK(loadFailedCallback), this);
+    g_signal_connect(m_webView, "load-failed-with-tls-errors", G_CALLBACK(loadFailedWithTLSErrorsCallback), this);
     g_signal_connect(m_webView, "notify::estimated-load-progress", G_CALLBACK(estimatedProgressChangedCallback), this);
 
     g_assert(!webkit_web_view_get_uri(m_webView));
@@ -143,6 +154,12 @@ void LoadTrackingTest::loadFailed(const gchar* failingURI, GError* error)
     m_loadEvents.append(LoadFailed);
 }
 
+bool LoadTrackingTest::loadFailedWithTLSErrors(const gchar* /*failingURI*/, GTlsCertificate*, GTlsCertificateFlags)
+{
+    m_loadEvents.append(LoadFailedWithTLSErrors);
+    return false;
+}
+
 void LoadTrackingTest::estimatedProgressChanged()
 {
     double progress = webkit_web_view_get_estimated_load_progress(m_webView);
index cab884f..c921847 100644 (file)
@@ -34,6 +34,7 @@ public:
     virtual void provisionalLoadStarted();
     virtual void provisionalLoadReceivedServerRedirect();
     virtual void provisionalLoadFailed(const gchar* failingURI, GError*);
+    virtual bool loadFailedWithTLSErrors(const gchar* failingURI, GTlsCertificate*, GTlsCertificateFlags);
     virtual void loadCommitted();
     virtual void loadFinished();
     virtual void loadFailed(const char* failingURI, GError*);
@@ -56,7 +57,8 @@ public:
         ProvisionalLoadFailed,
         LoadCommitted,
         LoadFinished,
-        LoadFailed
+        LoadFailed,
+        LoadFailedWithTLSErrors
     };
     bool m_runLoadUntilCompletion;
     bool m_loadFailed;