ResourceLoaderOptions also must be updated by updateRequestForAccessControl()

Source/core/loader/DocumentThreadableLoader.cpp

Index: Source/core/loader/DocumentThreadableLoader.cpp
diff --git a/Source/core/loader/DocumentThreadableLoader.cpp b/Source/core/loader/DocumentThreadableLoader.cpp
index 399abe64ff7f54abe96f17223c524fd269a76411..a2dd6875e603b5dc6799ad2316e65adacd03797f 100644
--- a/Source/core/loader/DocumentThreadableLoader.cpp
+++ b/Source/core/loader/DocumentThreadableLoader.cpp
@@ -73,7 +73,7 @@ DocumentThreadableLoader::DocumentThreadableLoader(Document& document, Threadabl
     , m_document(document)
     , m_options(options)
     , m_resourceLoaderOptions(resourceLoaderOptions)
-    , m_allowCredentials(m_resourceLoaderOptions.allowCredentials)
+    , m_forceDoNotAllowStoredCredentials(false)
     , m_securityOrigin(m_resourceLoaderOptions.securityOrigin)
     , m_sameOriginRequest(securityOrigin()->canRequest(request.url()))
     , m_simpleRequest(true)
@@ -94,7 +94,7 @@ DocumentThreadableLoader::DocumentThreadableLoader(Document& document, Threadabl
     }
 
     if (m_sameOriginRequest || m_options.crossOriginRequestPolicy == AllowCrossOriginRequests) {
-        loadRequest(request);
+        loadRequest(request, m_resourceLoaderOptions);
         return;
     }
 
@@ -118,21 +118,27 @@ void DocumentThreadableLoader::makeCrossOriginAccessRequest(const ResourceReques
         }
 
         ResourceRequest crossOriginRequest(request);
-        updateRequestForAccessControl(crossOriginRequest, securityOrigin(), m_allowCredentials);
-        loadRequest(crossOriginRequest);
+        ResourceLoaderOptions crossOriginOptions(m_resourceLoaderOptions);
+        updateRequestForAccessControl(crossOriginRequest, securityOrigin(), effectiveAllowCredentials());
+        loadRequest(crossOriginRequest, crossOriginOptions);
     } else {
         m_simpleRequest = false;
 
         OwnPtr<ResourceRequest> crossOriginRequest = adoptPtr(new ResourceRequest(request));
+        OwnPtr<ResourceLoaderOptions> crossOriginOptions = adoptPtr(new ResourceLoaderOptions(m_resourceLoaderOptions));
         // Do not set the Origin header for preflight requests.
-        updateRequestForAccessControl(*crossOriginRequest, 0, m_allowCredentials);
+        updateRequestForAccessControl(*crossOriginRequest.get(), 0, effectiveAllowCredentials());
         m_actualRequest = crossOriginRequest.release();
+        m_actualOptions = crossOriginOptions.release();
 
-        if (CrossOriginPreflightResultCache::shared().canSkipPreflight(securityOrigin()->toString(), m_actualRequest->url(), m_allowCredentials, m_actualRequest->httpMethod(), m_actualRequest->httpHeaderFields())) {
+        if (CrossOriginPreflightResultCache::shared().canSkipPreflight(securityOrigin()->toString(), m_actualRequest->url(), effectiveAllowCredentials(), m_actualRequest->httpMethod(), m_actualRequest->httpHeaderFields())) {
             loadActualRequest();
         } else {
-            ResourceRequest preflightRequest = createAccessControlPreflightRequest(*m_actualRequest, securityOrigin());
-            loadRequest(preflightRequest);
+            ResourceRequest preflightRequest = createAccessControlPreflightRequest(*m_actualRequest.get(), securityOrigin());
+            // Create a ResourceLoaderOptions for preflight.
+            ResourceLoaderOptions preflightOptions = *m_actualOptions.get();
+            preflightOptions.allowCredentials = DoNotAllowStoredCredentials;
+            loadRequest(preflightRequest, preflightOptions);
         }
     }
 }
@@ -201,7 +207,7 @@ void DocumentThreadableLoader::redirectReceived(Resource* resource, ResourceRequ
 
         if (m_simpleRequest) {
             allowRedirect = CrossOriginAccessControl::isLegalRedirectLocation(request.url(), accessControlErrorDescription)
-                && (m_sameOriginRequest || passesAccessControlCheck(redirectResponse, m_allowCredentials, securityOrigin(), accessControlErrorDescription));
+                && (m_sameOriginRequest || passesAccessControlCheck(redirectResponse, effectiveAllowCredentials(), securityOrigin(), accessControlErrorDescription));
         } else {
             accessControlErrorDescription = "The request was redirected to '"+ request.url().string() + "', which is disallowed for cross-origin requests that require preflight.";
         }
@@ -224,7 +230,7 @@ void DocumentThreadableLoader::redirectReceived(Resource* resource, ResourceRequ
             // 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_allowCredentials = DoNotAllowStoredCredentials;
+                m_forceDoNotAllowStoredCredentials = true;
 
             // Remove any headers that may have been added by the network layer that cause access control to fail.
             request.clearHTTPReferrer();
@@ -282,7 +288,7 @@ void DocumentThreadableLoader::handlePreflightResponse(unsigned long identifier,
 
     String accessControlErrorDescription;
 
-    if (!passesAccessControlCheck(response, m_allowCredentials, securityOrigin(), accessControlErrorDescription)) {
+    if (!passesAccessControlCheck(response, effectiveAllowCredentials(), securityOrigin(), accessControlErrorDescription)) {
         handlePreflightFailure(response.url().string(), accessControlErrorDescription);
         return;
     }
@@ -292,7 +298,7 @@ void DocumentThreadableLoader::handlePreflightResponse(unsigned long identifier,
         return;
     }
 
-    OwnPtr<CrossOriginPreflightResultCacheItem> preflightResult = adoptPtr(new CrossOriginPreflightResultCacheItem(m_allowCredentials));
+    OwnPtr<CrossOriginPreflightResultCacheItem> preflightResult = adoptPtr(new CrossOriginPreflightResultCacheItem(effectiveAllowCredentials()));
     if (!preflightResult->parse(response, accessControlErrorDescription)
         || !preflightResult->allowsCrossOriginMethod(m_actualRequest->httpMethod(), accessControlErrorDescription)
         || !preflightResult->allowsCrossOriginHeaders(m_actualRequest->httpHeaderFields(), accessControlErrorDescription)) {
@@ -314,7 +320,7 @@ void DocumentThreadableLoader::handleResponse(unsigned long identifier, const Re
 
     if (!m_sameOriginRequest && m_options.crossOriginRequestPolicy == UseAccessControl) {
         String accessControlErrorDescription;
-        if (!passesAccessControlCheck(response, m_allowCredentials, securityOrigin(), accessControlErrorDescription)) {
+        if (!passesAccessControlCheck(response, effectiveAllowCredentials(), securityOrigin(), accessControlErrorDescription)) {
             m_client->didFailAccessControlCheck(ResourceError(errorDomainBlinkInternal, 0, response.url().string(), accessControlErrorDescription));
             return;
         }
@@ -376,12 +382,14 @@ void DocumentThreadableLoader::loadActualRequest()
 {
     OwnPtr<ResourceRequest> actualRequest;
     actualRequest.swap(m_actualRequest);
+    OwnPtr<ResourceLoaderOptions> actualOptions;
+    actualOptions.swap(m_actualOptions);
 
     actualRequest->setHTTPOrigin(securityOrigin()->toAtomicString());
 
     clearResource();
 
-    loadRequest(*actualRequest);
+    loadRequest(*actualRequest.get(), *actualOptions.get());

[Nate Chapin, 2014/06/04 16:08:21]

I think I lead you astray :(

The cases were we use .get() with OwnPtrs are usually cases where we want a raw
pointer, not a reference. The *actualRequest method of getting a reference is
probably actually the right way.

The real weird thing is that we're OwnPtr'ing classes that usually aren't
handled as pointers. So maybe these should return to the old way.

[tyoshino (SeeGerritForStatus), 2014/06/05 02:10:49]

Ah, ok. Reverted.
Yeah. Looks over optimized :)

 }
 
 void DocumentThreadableLoader::handlePreflightFailure(const String& url, const String& errorDescription)
@@ -394,16 +402,16 @@ void DocumentThreadableLoader::handlePreflightFailure(const String& url, const S
     m_client->didFailAccessControlCheck(error);
 }
 
-void DocumentThreadableLoader::loadRequest(const ResourceRequest& request)
+void DocumentThreadableLoader::loadRequest(const ResourceRequest& request, ResourceLoaderOptions resourceLoaderOptions)
 {
     // Any credential should have been removed from the cross-site requests.
     const KURL& requestURL = request.url();
     ASSERT(m_sameOriginRequest || requestURL.user().isEmpty());
     ASSERT(m_sameOriginRequest || requestURL.pass().isEmpty());
 
-    ResourceLoaderOptions resourceLoaderOptions = m_resourceLoaderOptions;
     // Update resourceLoaderOptions with enforced values.
-    resourceLoaderOptions.allowCredentials = m_allowCredentials;
+    if (m_forceDoNotAllowStoredCredentials)
+        resourceLoaderOptions.allowCredentials = DoNotAllowStoredCredentials;
     resourceLoaderOptions.securityOrigin = m_securityOrigin;
     if (m_async) {
         if (m_actualRequest) {
@@ -479,6 +487,13 @@ bool DocumentThreadableLoader::isAllowedByPolicy(const KURL& url) const
     return m_document.contentSecurityPolicy()->allowConnectToSource(url);
 }
 
+StoredCredentials DocumentThreadableLoader::effectiveAllowCredentials() const
+{
+    if (m_forceDoNotAllowStoredCredentials)
+        return DoNotAllowStoredCredentials;
+    return m_resourceLoaderOptions.allowCredentials;
+}
+
 SecurityOrigin* DocumentThreadableLoader::securityOrigin() const
 {
     return m_securityOrigin ? m_securityOrigin.get() : m_document.securityOrigin();