Improve console log message for CORS failure

third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp

 /*
  * Copyright (C) 2011, 2012 Google Inc. All rights reserved.
  * Copyright (C) 2013, Intel Corporation
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
@@ skipping 132 matching lines @@
     , m_resourceLoaderOptions(resourceLoaderOptions)
     , m_forceDoNotAllowStoredCredentials(false)
     , m_securityOrigin(m_resourceLoaderOptions.securityOrigin)
     , m_sameOriginRequest(false)
     , m_crossOriginNonSimpleRequest(false)
     , m_isUsingDataConsumerHandle(false)
     , m_async(blockingBehavior == LoadAsynchronously)
     , m_requestContext(WebURLRequest::RequestContextUnspecified)
     , m_timeoutTimer(this, &DocumentThreadableLoader::didTimeout)
     , m_requestStartedSeconds(0.0)
-    , m_corsRedirectLimit(kMaxCORSRedirects)
+    , m_corsRedirectLimit(m_options.crossOriginRequestPolicy == UseAccessControl ? kMaxCORSRedirects : 0)
     , m_redirectMode(WebURLRequest::FetchRedirectModeFollow)
     , m_didRedirect(false)
     , m_weakFactory(this)
 {
     ASSERT(client);
 }
 
 void DocumentThreadableLoader::start(const ResourceRequest& request)
 {
     // Setting an outgoing referer is only supported in the async code path.
@@ skipping 336 matching lines @@
         if (m_client->isDocumentThreadableLoaderClient())
             static_cast<DocumentThreadableLoaderClient*>(m_client)->willFollowRedirect(request, redirectResponse);
         return;
     }
 
     if (m_corsRedirectLimit <= 0) {
         ThreadableLoaderClient* client = m_client;
         clear();
         client->didFailRedirectCheck();
         // |this| may be dead here.
-    } else if (m_options.crossOriginRequestPolicy == UseAccessControl) {
-        --m_corsRedirectLimit;
 
-        InspectorInstrumentation::didReceiveCORSRedirectResponse(document().frame(), resource->identifier(), document().frame()->loader().documentLoader(), redirectResponse, resource);
+        request = ResourceRequest();
 
-        bool allowRedirect = false;
-        String accessControlErrorDescription;
+        return;
+    }
 
+    --m_corsRedirectLimit;
+
+    InspectorInstrumentation::didReceiveCORSRedirectResponse(document().frame(), resource->identifier(), document().frame()->loader().documentLoader(), redirectResponse, resource);
+
+    bool allowRedirect = false;
+    String accessControlErrorDescription;
+
+    if (m_crossOriginNonSimpleRequest) {
         // Non-simple cross origin requests (both preflight and actual one) are
         // not allowed to follow redirect.
-        if (m_crossOriginNonSimpleRequest) {
-            accessControlErrorDescription = "The request was redirected to '"+ request.url().getString() + "', which is disallowed for cross-origin requests that require preflight.";
-        } else {
-            // The redirect response must pass the access control check if the
-            // original request was not same-origin.
-            allowRedirect = CrossOriginAccessControl::isLegalRedirectLocation(request.url(), accessControlErrorDescription)
-                && (m_sameOriginRequest || passesAccessControlCheck(redirectResponse, effectiveAllowCredentials(), getSecurityOrigin(), accessControlErrorDescription, m_requestContext));
-        }
+        accessControlErrorDescription = "Redirect from '" + redirectResponse.url().getString()+ "' to '" + request.url().getString() + "' has been blocked by CORS policy: Request requires preflight, which is disallowed to follow cross-origin redirect.";
+    } else if (!CrossOriginAccessControl::isLegalRedirectLocation(request.url(), accessControlErrorDescription)) {
+        accessControlErrorDescription = "Redirect from '" + redirectResponse.url().getString() + "' has been blocked by CORS policy: " + accessControlErrorDescription;
+    } else if (!m_sameOriginRequest && !passesAccessControlCheck(redirectResponse, effectiveAllowCredentials(), getSecurityOrigin(), accessControlErrorDescription, m_requestContext)) {
+        // The redirect response must pass the access control check if the
+        // original request was not same-origin.
+        accessControlErrorDescription = "Redirect from '" + redirectResponse.url().getString()+ "' to '" + request.url().getString() + "' has been blocked by CORS policy: Redirect response does not pass CORS check: " + accessControlErrorDescription;
+    } else {
+        allowRedirect = true;
+    }
 
-        if (allowRedirect) {
-            // FIXME: consider combining this with CORS redirect handling performed by
-            // CrossOriginAccessControl::handleRedirect().
-            clearResource();
-
-            RefPtr<SecurityOrigin> originalOrigin = SecurityOrigin::create(redirectResponse.url());
-            RefPtr<SecurityOrigin> requestOrigin = SecurityOrigin::create(request.url());
-            // If the original request wasn't same-origin, then if the request URL origin is not same origin with the original URL origin,
-            // set the source origin to a globally unique identifier. (If the original request was same-origin, the origin of the new request
-            // should be the original URL origin.)
-            if (!m_sameOriginRequest && !originalOrigin->isSameSchemeHostPort(requestOrigin.get()))
-                m_securityOrigin = SecurityOrigin::createUnique();
-            // Force any subsequent requests to use these checks.
-            m_sameOriginRequest = false;
-
-            // Since the request is no longer same-origin, if the user didn't request credentials in
-            // the first place, update our state so we neither request them nor expect they must be allowed.
-            if (m_resourceLoaderOptions.credentialsRequested == ClientDidNotRequestCredentials)
-                m_forceDoNotAllowStoredCredentials = true;
-
-            // Save the referrer to use when following the redirect.
-            m_didRedirect = true;
-            m_referrerAfterRedirect = Referrer(request.httpReferrer(), request.getReferrerPolicy());
-
-            // Remove any headers that may have been added by the network layer that cause access control to fail.
-            request.clearHTTPReferrer();
-            request.clearHTTPOrigin();
-            request.clearHTTPUserAgent();
-            // Add any CORS simple request headers which we previously saved from the original request.
-            for (const auto& header : m_simpleRequestHeaders)
-                request.setHTTPHeaderField(header.key, header.value);
-            makeCrossOriginAccessRequest(request);
-            // |this| may be dead here.
-            return;
-        }
-
+    if (!allowRedirect) {
         ThreadableLoaderClient* client = m_client;
         clear();
         client->didFailAccessControlCheck(ResourceError(errorDomainBlinkInternal, 0, redirectResponse.url().getString(), accessControlErrorDescription));
         // |this| may be dead here.
-    } else {
-        ThreadableLoaderClient* client = m_client;
-        clear();
-        client->didFailRedirectCheck();
-        // |this| may be dead here.
+
+        request = ResourceRequest();
+
+        return;
     }
 
-    request = ResourceRequest();
+    // FIXME: consider combining this with CORS redirect handling performed by
+    // CrossOriginAccessControl::handleRedirect().
+    clearResource();
+
+    RefPtr<SecurityOrigin> originalOrigin = SecurityOrigin::create(redirectResponse.url());

[sof, 2016/07/27 11:51:03]

Having complained a bit about RefPtr<SecurityOrigin> allocations.. :), let's
restructure this one a bit:

 if (!m_sameOriginRequest) {
     RefPtr<SecurityOrigin> originalOrigin = ...;
     RefPtr<SecurityOrigin> requestOrigin = ....;
     if (!originalOrigin->isSameSchemeHostPort(requestOrigin.get())) 
         ...
 }

[tyoshino (SeeGerritForStatus), 2016/07/28 12:23:06]

Oh, nice :)

+    RefPtr<SecurityOrigin> requestOrigin = SecurityOrigin::create(request.url());
+    // If the original request wasn't same-origin, then if the request URL origin is not same origin with the original URL origin,
+    // set the source origin to a globally unique identifier. (If the original request was same-origin, the origin of the new request
+    // should be the original URL origin.)
+    if (!m_sameOriginRequest && !originalOrigin->isSameSchemeHostPort(requestOrigin.get()))
+        m_securityOrigin = SecurityOrigin::createUnique();
+    // Force any subsequent requests to use these checks.
+    m_sameOriginRequest = false;
+
+    // Since the request is no longer same-origin, if the user didn't request credentials in
+    // the first place, update our state so we neither request them nor expect they must be allowed.
+    if (m_resourceLoaderOptions.credentialsRequested == ClientDidNotRequestCredentials)
+        m_forceDoNotAllowStoredCredentials = true;
+
+    // Save the referrer to use when following the redirect.
+    m_didRedirect = true;
+    m_referrerAfterRedirect = Referrer(request.httpReferrer(), request.getReferrerPolicy());
+
+    // Remove any headers that may have been added by the network layer that cause access control to fail.
+    request.clearHTTPReferrer();
+    request.clearHTTPOrigin();
+    request.clearHTTPUserAgent();
+    // Add any CORS simple request headers which we previously saved from the original request.
+    for (const auto& header : m_simpleRequestHeaders)
+        request.setHTTPHeaderField(header.key, header.value);
+    makeCrossOriginAccessRequest(request);
+    // |this| may be dead here.
 }
 
 void DocumentThreadableLoader::redirectBlocked()
 {
     // Tells the client that a redirect was received but not followed (for an unknown reason).
     ThreadableLoaderClient* client = m_client;
     clear();
     client->didFailRedirectCheck();
     // |this| may be dead here
 }
@@ skipping 428 matching lines @@
     return m_securityOrigin ? m_securityOrigin.get() : document().getSecurityOrigin();
 }
 
 Document& DocumentThreadableLoader::document() const
 {
     ASSERT(m_document);
     return *m_document;
 }
 
 } // namespace blink