REGRESSION (r112217): H&R Block tax site won't load

https://bugs.webkit.org/show_bug.cgi?id=82964

Source/WebCore:

Modifies the redirect checking code to first check if the security origin can
request the redirect URL before invoking the CORS check.

Reviewed by Adam Barth.

http/tests/xmlhttprequest/access-control-and-redirects-async.html

* loader/DocumentThreadableLoader.cpp:
* loader/DocumentThreadableLoader.h:

LayoutTests:

Add a test case for a same origin request with a custom header that receives a
same origin redirect, and therefore should pass the redirect check.

Reviewed by Adam Barth.

* http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt:
* http/tests/xmlhttprequest/access-control-and-redirects-async.html:

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index ceb127a..871c14c 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2012-04-03  Bill Budge  <bbudge@chromium.org>
+
+        REGRESSION (r112217): H&R Block tax site won't load
+        https://bugs.webkit.org/show_bug.cgi?id=82964
+
+        Add a test case for a same origin request with a custom header that receives a
+        same origin redirect, and therefore should pass the redirect check.
+
+        Reviewed by Adam Barth.
+
+        * http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt:
+        * http/tests/xmlhttprequest/access-control-and-redirects-async.html:
+
 2012-04-03  Philippe Normand  <pnormand@igalia.com>
 
         Unreviewed, baseline update for new test.

diff --git a/LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt b/LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt
index d7195dc..062da2d 100644 (file)
--- a/LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt
+++ b/LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt
@@ -1,30 +1,33 @@
 Tests that asynchronous XMLHttpRequests handle redirects according to the CORS standard.
 
-Testing resources/redirect-cors.php?url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi
+Testing resources/redirect-cors.php?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi
 Expecting success: false
 PASS: 0
-Testing resources/redirect-cors.php?url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&  access-control-allow-origin=http://localhost:8000&  access-control-allow-credentials=true
+Testing resources/redirect-cors.php?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&  access-control-allow-origin=http://localhost:8000&  access-control-allow-credentials=true
 Expecting success: false
 PASS: 0
-Testing resources/redirect-cors.php?url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow.cgi&  access-control-allow-origin=http://localhost:8000&  access-control-allow-credentials=true
+Testing resources/redirect-cors.php?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi&  access-control-allow-origin=http://localhost:8000&  access-control-allow-credentials=true
 Expecting success: false
 PASS: 0
-Testing http://127.0.0.1:8000/xmlhttprequest/resources/redirect-cors.php?url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi
+Testing http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi
 Expecting success: false
 PASS: 0
-Testing http://127.0.0.1:8000/xmlhttprequest/resources/redirect-cors.php?url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&  access-control-allow-origin=http://localhost:8000
+Testing http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&  access-control-allow-origin=http://localhost:8000
 Expecting success: true
 FAIL: 0
-Testing http://127.0.0.1:8000/xmlhttprequest/resources/redirect-cors.php?url=http://username:password@127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&  access-control-allow-origin=http://localhost:8000
+Testing http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?url=http://username:password@localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&  access-control-allow-origin=http://localhost:8000
 Expecting success: false
 PASS: 0
-Testing http://127.0.0.1:8000/xmlhttprequest/resources/redirect-cors.php?url=foo://bar.cgi&  access-control-allow-origin=http://localhost:8000
+Testing http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?url=foo://bar.cgi&  access-control-allow-origin=http://localhost:8000
 Expecting success: false
 PASS: 0
-Testing http://127.0.0.1:8000/xmlhttprequest/resources/redirect-cors.php?redirect-preflight=true&  url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&  access-control-allow-origin=*
+Testing http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?redirect-preflight=true&  url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&  access-control-allow-origin=*
 Expecting success: false
 PASS: 0
-Testing http://127.0.0.1:8000/xmlhttprequest/resources/redirect-cors.php?redirect-preflight=false&  url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&  access-control-allow-origin=*&  access-control-allow-headers=x-webkit
+Testing http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?redirect-preflight=false&  url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&  access-control-allow-origin=*&  access-control-allow-headers=x-webkit
 Expecting success: false
 PASS: 0
+Testing resources/redirect-cors.php?url=http://127.0.0.1:8000/xmlhttprequest/resources/get.txt
+Expecting success: true
+PASS: PASS
 

diff --git a/LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-async.html b/LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-async.html
index d6fa1d3..bd37ec8 100644 (file)
--- a/LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-async.html
+++ b/LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-async.html
@@ -12,13 +12,13 @@ function log(message)
     document.getElementById('console').appendChild(document.createTextNode(message + '\n'));
 }
 
-function runTestAsync(url, forcePreflight, expectSuccess) {
+function runTestAsync(url, addCustomHeader, expectSuccess) {
     log("Testing " + url);
     log("Expecting success: " + expectSuccess);
 
     xhr = new XMLHttpRequest();
     xhr.open("GET", url, true);
-    if (forcePreflight)
+    if (addCustomHeader)
         xhr.setRequestHeader("x-webkit", "foo");
 
     xhr.onload = function() {
@@ -32,8 +32,8 @@ function runTestAsync(url, forcePreflight, expectSuccess) {
     xhr.send(null);
 }
 
-var simple = false;
-var preflight = true;
+var noCustomHeader = false;
+var addCustomHeader = true;
 var succeeds = true;
 var fails = false;
 
@@ -41,59 +41,63 @@ var tests = [
 // 1) Test simple same origin requests that receive cross origin redirects.
 
 // Request receives a cross-origin redirect response without CORS headers. The redirect response fails the access check.
-["resources/redirect-cors.php?url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi",
-  simple, fails],
+["resources/redirect-cors.php?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi",
+  noCustomHeader, fails],
 
 // Request receives a cross-origin redirect response with CORS headers. The redirect response passes the access check,
 // but  the resource response fails its access check because the security origin is a globally unique identifier after
 // the redirect and the same origin XHR has 'allowCredentials' true.
-["resources/redirect-cors.php?url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&\
+["resources/redirect-cors.php?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&\
   access-control-allow-origin=http://localhost:8000&\
   access-control-allow-credentials=true",
-  simple, fails],
+  noCustomHeader, fails],
 
 // Same as above, but to a less permissive resource that only allows the requesting origin.
-["resources/redirect-cors.php?url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow.cgi&\
+["resources/redirect-cors.php?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi&\
   access-control-allow-origin=http://localhost:8000&\
   access-control-allow-credentials=true",
-  simple, fails],
+  noCustomHeader, fails],
 
 // 2) Test simple cross origin requests that receive redirects.
 
 // Receives a redirect response without CORS headers. The redirect response fails the access check.
-["http://127.0.0.1:8000/xmlhttprequest/resources/redirect-cors.php?url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi",
-  simple, fails],
+["http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi",
+  noCustomHeader, fails],
 
 // Receives a redirect response with CORS headers. The redirect response passes the access check and the resource response
 // passes the access check.
-["http://127.0.0.1:8000/xmlhttprequest/resources/redirect-cors.php?url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&\
+["http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&\
   access-control-allow-origin=http://localhost:8000",
-  simple, succeeds],
+  noCustomHeader, succeeds],
 
 // Receives a redirect response with a URL containing the userinfo production.
-["http://127.0.0.1:8000/xmlhttprequest/resources/redirect-cors.php?url=http://username:password@127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&\
+["http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?url=http://username:password@localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&\
   access-control-allow-origin=http://localhost:8000",
-  simple, fails],
+  noCustomHeader, fails],
 
 // Receives a redirect response with a URL with an unsupported scheme.
-["http://127.0.0.1:8000/xmlhttprequest/resources/redirect-cors.php?url=foo://bar.cgi&\
+["http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?url=foo://bar.cgi&\
   access-control-allow-origin=http://localhost:8000",
-  simple, fails],
+  noCustomHeader, fails],
 
 // 3) Test preflighted cross origin requests that receive redirects.
 
 // Receives a redirect response to the preflight request and fails.
-["http://127.0.0.1:8000/xmlhttprequest/resources/redirect-cors.php?redirect-preflight=true&\
-  url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&\
+["http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?redirect-preflight=true&\
+  url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&\
   access-control-allow-origin=*",
-  preflight, fails],
+  addCustomHeader, fails],
 
 // Successful preflight and receives a redirect response to the actual request and fails.
-["http://127.0.0.1:8000/xmlhttprequest/resources/redirect-cors.php?redirect-preflight=false&\
-  url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&\
+["http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?redirect-preflight=false&\
+  url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&\
   access-control-allow-origin=*&\
   access-control-allow-headers=x-webkit",
-  preflight, fails],
+  addCustomHeader, fails],
+
+// 4) Test same origin requests with a custom header that receive a same origin redirect.
+["resources/redirect-cors.php?url=http://127.0.0.1:8000/xmlhttprequest/resources/get.txt",
+  addCustomHeader, succeeds],
 ]
 
 var currentTest = 0;

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 34a199b..6b56d46 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,18 @@
+2012-04-03  Bill Budge  <bbudge@chromium.org>
+
+        REGRESSION (r112217): H&R Block tax site won't load
+        https://bugs.webkit.org/show_bug.cgi?id=82964
+
+        Modifies the redirect checking code to first check if the security origin can
+        request the redirect URL before invoking the CORS check.
+
+        Reviewed by Adam Barth.
+
+        http/tests/xmlhttprequest/access-control-and-redirects-async.html
+
+        * loader/DocumentThreadableLoader.cpp:
+        * loader/DocumentThreadableLoader.h:
+
 2012-04-03  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r112994.

diff --git a/Source/WebCore/loader/DocumentThreadableLoader.cpp b/Source/WebCore/loader/DocumentThreadableLoader.cpp
index a10fb70..436cb5f 100644 (file)
--- a/Source/WebCore/loader/DocumentThreadableLoader.cpp
+++ b/Source/WebCore/loader/DocumentThreadableLoader.cpp
@@ -85,11 +85,6 @@ DocumentThreadableLoader::DocumentThreadableLoader(Document* document, Threadabl
     // Setting an outgoing referer is only supported in the async code path.
     ASSERT(m_async || request.httpReferrer().isEmpty());
 
-    makeRequest(request);
-}
-
-void DocumentThreadableLoader::makeRequest(const ResourceRequest& request)
-{
     if (m_sameOriginRequest || m_options.crossOriginRequestPolicy == AllowCrossOriginRequests) {
         loadRequest(request, DoSecurityCheck);
         return;
@@ -100,6 +95,11 @@ void DocumentThreadableLoader::makeRequest(const ResourceRequest& request)
         return;
     }
 
+    makeCrossOriginAccessRequest(request);
+}
+
+void DocumentThreadableLoader::makeCrossOriginAccessRequest(const ResourceRequest& request)
+{
     ASSERT(m_options.crossOriginRequestPolicy == UseAccessControl);
 
     OwnPtr<ResourceRequest> crossOriginRequest = adoptPtr(new ResourceRequest(request));
@@ -175,10 +175,17 @@ void DocumentThreadableLoader::redirectReceived(CachedResource* resource, Resour
     ASSERT_UNUSED(resource, resource == m_resource);
 
     RefPtr<DocumentThreadableLoader> protect(this);
-    bool allowRedirect = false;
+    // Allow same origin requests to continue after allowing clients to audit the redirect.
+    if (isAllowedRedirect(request.url())) {
+        if (m_client->isDocumentThreadableLoaderClient())
+            static_cast<DocumentThreadableLoaderClient*>(m_client)->willSendRequest(request, redirectResponse);
+        return;
+    }
+
+    // When using access control, only simple cross origin requests are allowed to redirect. The new request URL must have a supported
+    // scheme and not contain the userinfo production. In addition, the redirect response must pass the access control check.
     if (m_options.crossOriginRequestPolicy == UseAccessControl) {
-        // When using access control, only simple cross origin requests are allowed to redirect. The new request URL must have a supported
-        // scheme and not contain the userinfo production. In addition, the redirect response must pass the access control check.
+        bool allowRedirect = false;
         if (m_simpleRequest) {
             String accessControlErrorDescription;
             allowRedirect = SchemeRegistry::shouldTreatURLSchemeAsCORSEnabled(request.url().protocol())
@@ -186,11 +193,8 @@ void DocumentThreadableLoader::redirectReceived(CachedResource* resource, Resour
                             && request.url().pass().isEmpty()
                             && passesAccessControlCheck(redirectResponse, m_options.allowCredentials, securityOrigin(), accessControlErrorDescription);
         }
-    } else
-        allowRedirect = isAllowedRedirect(request.url());
 
-    if (allowRedirect) {
-        if (m_options.crossOriginRequestPolicy == UseAccessControl) {
+        if (allowRedirect) {
             if (m_resource)
                 clearResource();
 
@@ -199,7 +203,8 @@ void DocumentThreadableLoader::redirectReceived(CachedResource* resource, Resour
             // If the request URL origin is not same origin with the original URL origin, set source origin to a globally unique identifier.
             if (!originalOrigin->isSameSchemeHostPort(requestOrigin.get()))
                 m_options.securityOrigin = SecurityOrigin::createUnique();
-            m_sameOriginRequest = securityOrigin()->canRequest(request.url());
+            // Force any subsequent requests to use these checks.
+            m_sameOriginRequest = false;
 
             // Remove any headers that may have been added by the network layer that cause access control to fail.
             request.clearHTTPContentType();
@@ -207,16 +212,13 @@ void DocumentThreadableLoader::redirectReceived(CachedResource* resource, Resour
             request.clearHTTPOrigin();
             request.clearHTTPUserAgent();
             request.clearHTTPAccept();
-            makeRequest(request);
-        } else {
-            // If not using access control, allow clients to audit the redirect.
-            if (m_client->isDocumentThreadableLoaderClient())
-                static_cast<DocumentThreadableLoaderClient*>(m_client)->willSendRequest(request, redirectResponse);
+            makeCrossOriginAccessRequest(request);
+            return;
         }
-    } else {
-        m_client->didFailRedirectCheck();
-        request = ResourceRequest();
     }
+
+    m_client->didFailRedirectCheck();
+    request = ResourceRequest();
 }
 
 void DocumentThreadableLoader::dataSent(CachedResource* resource, unsigned long long bytesSent, unsigned long long totalBytesToBeSent)

diff --git a/Source/WebCore/loader/DocumentThreadableLoader.h b/Source/WebCore/loader/DocumentThreadableLoader.h
index 5a0b6c5..879919f 100644 (file)
--- a/Source/WebCore/loader/DocumentThreadableLoader.h
+++ b/Source/WebCore/loader/DocumentThreadableLoader.h
@@ -89,7 +89,7 @@ namespace WebCore {
         void didReceiveResponse(unsigned long identifier, const ResourceResponse&);
         void didFinishLoading(unsigned long identifier, double finishTime);
         void didFail(const ResourceError&);
-        void makeRequest(const ResourceRequest&);
+        void makeCrossOriginAccessRequest(const ResourceRequest&);
         void makeSimpleCrossOriginAccessRequest(const ResourceRequest& request);
         void makeCrossOriginAccessRequestWithPreflight(const ResourceRequest& request);
         void preflightSuccess();