Attachment 356934 Details for Bug 186276 – Patch

[patch] Patch

bug-186276-20181209191535.patch (text/plain), 6.53 KB, created by Michael Catanzaro on 2018-12-09 17:15:36 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Michael Catanzaro

Created: 2018-12-09 17:15:36 PST

Size: 6.53 KB

patch

obsolete

>Subversion Revision: 239022
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index db43de90c60fb77274785298acd8054e100c2c2f..cee117d19f450b3c1ccb6364de87e2731fd58474 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,22 @@
>+2018-12-09  Michael Catanzaro  <mcatanzaro@igalia.com>
>+
>+        [WPE][GTK] webkit_web_resource_get_data_finish can return NULL without setting error
>+        https://bugs.webkit.org/show_bug.cgi?id=186276
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Currently it's possible for webkit_web_resource_get_data_finish() to return NULL without
>+        setting the error parameter. This is illegal because it is an API guarantee (and a GObject
>+        convention) that if an error parameter exists, it should be set whenever a function call
>+        returns NULL. Epiphany correctly dereferences the error in this case without checking if it
>+        is NULL, because it knows it does not have to, and crashes. Fix this.
>+
>+        This does not fix bug #186276, in which this function incorrectly returns no data when it
>+        ought to. But that is a different bug, and we should do this regardless.
>+
>+        * UIProcess/API/glib/WebKitWebResource.cpp:
>+        (resourceDataCallback):
>+
> 2018-12-09  Adrian Perez de Castro  <aperez@igalia.com>
> 
>         Build failure due to missing include of APIWebsiteDataStore.h
>diff --git a/Source/WebKit/UIProcess/API/glib/WebKitWebResource.cpp b/Source/WebKit/UIProcess/API/glib/WebKitWebResource.cpp
>index 7beabb07f3a9ba5af3c64ec5d252c4a2af85af33..57cbfbee48462c08c600d45b05c576aa55138b53 100644
>--- a/Source/WebKit/UIProcess/API/glib/WebKitWebResource.cpp
>+++ b/Source/WebKit/UIProcess/API/glib/WebKitWebResource.cpp
>@@ -351,6 +351,10 @@ static void resourceDataCallback(API::Data* wkData, CallbackBase::Error error, G
>     }
>     ResourceGetDataAsyncData* data = static_cast<ResourceGetDataAsyncData*>(g_task_get_task_data(task));
>     data->webData = wkData;
>+    if (!wkData->bytes()) {
>+        g_task_return_new_error(task, G_IO_ERROR, G_IO_ERROR_FAILED, _("Resource data unavailable"));
>+        return;
>+    }
>     g_task_return_boolean(task, TRUE);
> }
> 
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index ecdd761f1a8de4677c633ca82e1a8d4957d0eb29..3b906e0bce46f8add73c47e62cb4a50e6dd9b388 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-12-09  Michael Catanzaro  <mcatanzaro@igalia.com>
>+
>+        [WPE][GTK] webkit_web_resource_get_data_finish can return NULL without setting error
>+        https://bugs.webkit.org/show_bug.cgi?id=186276
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/Tests/WebKitGLib/TestResources.cpp:
>+        (webViewLoadChanged):
>+        (testWebResourceGetDataError):
>+        (testWebResourceGetDataEmpty):
>+        (beforeAll):
>+        (webViewloadChanged): Deleted.
>+
> 2018-12-08  Daniel Bates  <dabates@apple.com>
> 
>         Fix lldb_webkit.py following r238771
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitGLib/TestResources.cpp b/Tools/TestWebKitAPI/Tests/WebKitGLib/TestResources.cpp
>index 3470720f57db0e999338c8ed455dc096bc0813f0..1b366836e1c74b1cd02bad62ca85ca158ec49244 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitGLib/TestResources.cpp
>+++ b/Tools/TestWebKitAPI/Tests/WebKitGLib/TestResources.cpp
>@@ -536,11 +536,11 @@ static void testWebResourceGetData(ResourcesTest* test, gconstpointer)
>         test->checkResourceData(WEBKIT_WEB_RESOURCE(item->data));
> }
> 
>-static void webViewloadChanged(WebKitWebView* webView, WebKitLoadEvent loadEvent, GMainLoop* mainLoop)
>+static void webViewLoadChanged(WebKitWebView* webView, WebKitLoadEvent loadEvent, GMainLoop* mainLoop)
> {
>     if (loadEvent != WEBKIT_LOAD_FINISHED)
>         return;
>-    g_signal_handlers_disconnect_by_func(webView, reinterpret_cast<void*>(webViewloadChanged), mainLoop);
>+    g_signal_handlers_disconnect_by_func(webView, reinterpret_cast<void*>(webViewLoadChanged), mainLoop);
>     g_main_loop_quit(mainLoop);
> }
> 
>@@ -549,7 +549,7 @@ static void testWebResourceGetDataError(Test* test, gconstpointer)
>     GRefPtr<GMainLoop> mainLoop = adoptGRef(g_main_loop_new(nullptr, FALSE));
>     GRefPtr<WebKitWebView> webView = WEBKIT_WEB_VIEW(Test::createWebView(test->m_webContext.get()));
>     webkit_web_view_load_html(webView.get(), "<html></html>", nullptr);
>-    g_signal_connect(webView.get(), "load-changed", G_CALLBACK(webViewloadChanged), mainLoop.get());
>+    g_signal_connect(webView.get(), "load-changed", G_CALLBACK(webViewLoadChanged), mainLoop.get());
>     g_main_loop_run(mainLoop.get());
> 
>     auto* resource = webkit_web_view_get_main_resource(webView.get());
>@@ -566,6 +566,27 @@ static void testWebResourceGetDataError(Test* test, gconstpointer)
>     g_main_loop_run(mainLoop.get());
> }
> 
>+static void testWebResourceGetDataEmpty(Test* test, gconstpointer)
>+{
>+    GRefPtr<GMainLoop> mainLoop = adoptGRef(g_main_loop_new(nullptr, FALSE));
>+    GRefPtr<WebKitWebView> webView = WEBKIT_WEB_VIEW(Test::createWebView(test->m_webContext.get()));
>+    webkit_web_view_load_html(webView.get(), "", nullptr);
>+    g_signal_connect(webView.get(), "load-changed", G_CALLBACK(webViewLoadChanged), mainLoop.get());
>+    g_main_loop_run(mainLoop.get());
>+
>+    auto* resource = webkit_web_view_get_main_resource(webView.get());
>+    test->assertObjectIsDeletedWhenTestFinishes(G_OBJECT(resource));
>+    webkit_web_resource_get_data(resource, nullptr, [](GObject* source, GAsyncResult* result, gpointer userData) {
>+        size_t dataSize;
>+        GUniqueOutPtr<GError> error;
>+        auto* data = webkit_web_resource_get_data_finish(WEBKIT_WEB_RESOURCE(source), result, &dataSize, &error.outPtr());
>+        g_assert_null(data);
>+        g_assert_error(error.get(), G_IO_ERROR, G_IO_ERROR_FAILED);
>+        g_main_loop_quit(static_cast<GMainLoop*>(userData));
>+    }, mainLoop.get());
>+    g_main_loop_run(mainLoop.get());
>+}
>+
> static void testWebViewResourcesHistoryCache(SingleResourceLoadTest* test, gconstpointer)
> {
>     CString javascriptURI = kServer->getURIForPath("/javascript.html");
>@@ -897,6 +918,7 @@ void beforeAll()
>     ResourceURITrackingTest::add("WebKitWebResource", "active-uri", testWebResourceActiveURI);
>     ResourcesTest::add("WebKitWebResource", "get-data", testWebResourceGetData);
>     Test::add("WebKitWebResource", "get-data-error", testWebResourceGetDataError);
>+    Test::add("WebKitWebResource", "get-data-empty", testWebResourceGetDataEmpty);
>     SingleResourceLoadTest::add("WebKitWebView", "history-cache", testWebViewResourcesHistoryCache);
>     SendRequestTest::add("WebKitWebPage", "send-request", testWebResourceSendRequest);
> #if SOUP_CHECK_VERSION(2, 49, 91)

Actions: View | Formatted Diff | Diff

Attachments on bug 186276: 341910 | 356934 | 366083