[SOUP] willSendRequest doesn't work after a redirect
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 31 Dec 2013 08:28:17 +0000 (08:28 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 31 Dec 2013 08:28:17 +0000 (08:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=126290

Reviewed by Martin Robinson.

Source/WebCore:

The problem is that we are creating the new soup request for the
redirect before calling ResourceHandleClient::willSendRequest() so
that any change made to the request by the client is ignored.

* platform/network/soup/ResourceHandleSoup.cpp:
(WebCore::doRedirect): Create the new soup request and soup
message for the redirect after calling willSendRequest() on the
client.

Source/WebKit2:

Add test cases to test send-request signal in case of
redirection.

* UIProcess/API/gtk/tests/TestResources.cpp:
(testWebResourceSendRequest):
(serverCallback):

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

Source/WebCore/ChangeLog
Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/API/gtk/tests/TestResources.cpp

index bf7f389..d1ea11b 100644 (file)
@@ -1,3 +1,19 @@
+2013-12-30  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [SOUP] willSendRequest doesn't work after a redirect
+        https://bugs.webkit.org/show_bug.cgi?id=126290
+
+        Reviewed by Martin Robinson.
+
+        The problem is that we are creating the new soup request for the
+        redirect before calling ResourceHandleClient::willSendRequest() so
+        that any change made to the request by the client is ignored.
+
+        * platform/network/soup/ResourceHandleSoup.cpp:
+        (WebCore::doRedirect): Create the new soup request and soup
+        message for the redirect after calling willSendRequest() on the
+        client.
+
 2013-12-30  Andreas Kling  <akling@apple.com>
 
         InputType should return input renderers wrapped in RenderPtr.
index de78902..9d6a144 100644 (file)
@@ -502,12 +502,6 @@ static void doRedirect(ResourceHandle* handle)
     } else
         applyAuthenticationToRequest(handle, newRequest, true);
 
-    cleanupSoupRequestOperation(handle);
-    if (!createSoupRequestAndMessageForHandle(handle, newRequest, true)) {
-        d->client()->cannotShowURL(handle);
-        return;
-    }
-
     // If we sent credentials with this request's URL, we don't want the response to carry them to
     // the WebKit layer. They were only placed in the URL for the benefit of libsoup.
     newRequest.removeCredentials();
@@ -516,6 +510,18 @@ static void doRedirect(ResourceHandle* handle)
         d->client()->willSendRequestAsync(handle, newRequest, d->m_response);
     else
         d->client()->willSendRequest(handle, newRequest, d->m_response);
+
+    cleanupSoupRequestOperation(handle);
+
+    // willSendRequest might cancel the load.
+    if (handle->cancelledOrClientless())
+        return;
+
+    if (!createSoupRequestAndMessageForHandle(handle, newRequest, true)) {
+        d->client()->cannotShowURL(handle);
+        return;
+    }
+
     handle->sendPendingRequest();
 }
 
index 4f19aca..84e63c2 100644 (file)
@@ -1,3 +1,17 @@
+2013-12-30  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [SOUP] willSendRequest doesn't work after a redirect
+        https://bugs.webkit.org/show_bug.cgi?id=126290
+
+        Reviewed by Martin Robinson.
+
+        Add test cases to test send-request signal in case of
+        redirection.
+
+        * UIProcess/API/gtk/tests/TestResources.cpp:
+        (testWebResourceSendRequest):
+        (serverCallback):
+
 2013-12-30  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r161157, r161158, r161160, r161161,
index a59fe7c..57f182c 100644 (file)
@@ -568,7 +568,10 @@ public:
         if (resource != m_resource)
             return;
 
-        g_assert_cmpstr(webkit_uri_request_get_uri(request), ==, m_expectedNewResourceURI.data());
+        if (redirectResponse)
+            g_assert_cmpstr(webkit_uri_request_get_uri(request), ==, m_expectedNewResourceURIAfterRedirection.data());
+        else
+            g_assert_cmpstr(webkit_uri_request_get_uri(request), ==, m_expectedNewResourceURI.data());
         g_assert_cmpstr(webkit_uri_request_get_uri(request), ==, webkit_web_resource_get_uri(resource));
 
         SingleResourceLoadTest::resourceSentRequest(resource, request, redirectResponse);
@@ -595,8 +598,14 @@ public:
         m_expectedCancelledResourceURI = uri;
     }
 
+    void setExpectedNewResourceURIAfterRedirection(const CString& uri)
+    {
+        m_expectedNewResourceURIAfterRedirection = uri;
+    }
+
     CString m_expectedNewResourceURI;
     CString m_expectedCancelledResourceURI;
+    CString m_expectedNewResourceURIAfterRedirection;
 };
 
 static void testWebResourceSendRequest(SendRequestTest* test, gconstpointer)
@@ -626,6 +635,36 @@ static void testWebResourceSendRequest(SendRequestTest* test, gconstpointer)
     g_assert_cmpint(events[1], ==, SingleResourceLoadTest::Failed);
     g_assert_cmpint(events[2], ==, SingleResourceLoadTest::Finished);
     events.clear();
+
+    // URI changed after a redirect.
+    test->setExpectedNewResourceURI(kServer->getURIForPath("/redirected.js"));
+    test->setExpectedNewResourceURIAfterRedirection(kServer->getURIForPath("/javascript.js"));
+    test->loadURI(kServer->getURIForPath("redirected-javascript.html").data());
+    test->waitUntilResourceLoadFinished();
+    g_assert(test->m_resource);
+
+    g_assert_cmpint(events.size(), ==, 6);
+    g_assert_cmpint(events[0], ==, SingleResourceLoadTest::Started);
+    g_assert_cmpint(events[1], ==, SingleResourceLoadTest::SentRequest);
+    g_assert_cmpint(events[2], ==, SingleResourceLoadTest::Redirected);
+    g_assert_cmpint(events[3], ==, SingleResourceLoadTest::ReceivedResponse);
+    g_assert_cmpint(events[4], ==, SingleResourceLoadTest::ReceivedData);
+    g_assert_cmpint(events[5], ==, SingleResourceLoadTest::Finished);
+    events.clear();
+
+    // Cancel after a redirect.
+    test->setExpectedNewResourceURI(kServer->getURIForPath("/redirected-to-cancel.js"));
+    test->setExpectedCancelledResourceURI(kServer->getURIForPath("/redirected-to-cancel.js"));
+    test->loadURI(kServer->getURIForPath("/redirected-to-cancel.html").data());
+    test->waitUntilResourceLoadFinished();
+    g_assert(test->m_resource);
+
+    g_assert_cmpint(events.size(), ==, 4);
+    g_assert_cmpint(events[0], ==, SingleResourceLoadTest::Started);
+    g_assert_cmpint(events[1], ==, SingleResourceLoadTest::SentRequest);
+    g_assert_cmpint(events[2], ==, SingleResourceLoadTest::Failed);
+    g_assert_cmpint(events[3], ==, SingleResourceLoadTest::Finished);
+    events.clear();
 }
 
 static void addCacheHTTPHeadersToResponse(SoupMessage* message)
@@ -688,6 +727,12 @@ static void serverCallback(SoupServer* server, SoupMessage* message, const char*
     } else if (g_str_equal(path, "/resource-to-cancel.html")) {
         static const char* resourceToCancelHTML = "<html><head><script language='javascript' src='cancel-this.js'></script></head><body></body></html>";
         soup_message_body_append(message->response_body, SOUP_MEMORY_STATIC, resourceToCancelHTML, strlen(resourceToCancelHTML));
+    } else if (g_str_equal(path, "/redirected-javascript.html")) {
+        static const char* javascriptRelativeHTML = "<html><head><script language='javascript' src='/redirected.js'></script></head><body></body></html>";
+        soup_message_body_append(message->response_body, SOUP_MEMORY_STATIC, javascriptRelativeHTML, strlen(javascriptRelativeHTML));
+    } else if (g_str_equal(path, "/redirected-to-cancel.html")) {
+        static const char* javascriptRelativeHTML = "<html><head><script language='javascript' src='/redirected-to-cancel.js'></script></head><body></body></html>";
+        soup_message_body_append(message->response_body, SOUP_MEMORY_STATIC, javascriptRelativeHTML, strlen(javascriptRelativeHTML));
     } else if (g_str_equal(path, "/blank.ico")) {
         GOwnPtr<char> filePath(g_build_filename(Test::getWebKit1TestResoucesDir().data(), path, NULL));
         char* contents;
@@ -707,6 +752,12 @@ static void serverCallback(SoupServer* server, SoupMessage* message, const char*
     } else if (g_str_equal(path, "/redirected.css")) {
         soup_message_set_status(message, SOUP_STATUS_MOVED_PERMANENTLY);
         soup_message_headers_append(message->response_headers, "Location", "/simple-style.css");
+    } else if (g_str_equal(path, "/redirected.js")) {
+        soup_message_set_status(message, SOUP_STATUS_MOVED_PERMANENTLY);
+        soup_message_headers_append(message->response_headers, "Location", "/remove-this/javascript.js");
+    } else if (g_str_equal(path, "/redirected-to-cancel.js")) {
+        soup_message_set_status(message, SOUP_STATUS_MOVED_PERMANENTLY);
+        soup_message_headers_append(message->response_headers, "Location", "/cancel-this.js");
     } else if (g_str_equal(path, "/invalid.css"))
         soup_message_set_status(message, SOUP_STATUS_CANT_CONNECT);
     else