Prevent calling didReceiveData()/didFinishLoading() after didFailAccessControlCheck()

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 74 matching lines @@
     , m_options(options)
     , m_resourceLoaderOptions(resourceLoaderOptions)
     , m_forceDoNotAllowStoredCredentials(false)
     , m_securityOrigin(m_resourceLoaderOptions.securityOrigin)
     , m_sameOriginRequest(securityOrigin()->canRequest(request.url()))
     , m_simpleRequest(true)
     , m_async(blockingBehavior == LoadAsynchronously)
     , m_timeoutTimer(this, &DocumentThreadableLoader::didTimeout)
     , m_requestStartedSeconds(0.0)
     , m_corsRedirectLimit(kMaxCORSRedirects)
+    , m_accessControlCheckFailed(false)
 {
     ASSERT(client);
     // Setting an outgoing referer is only supported in the async code path.
     ASSERT(m_async || request.httpReferrer().isEmpty());
 
     if (!m_sameOriginRequest && m_options.crossOriginRequestPolicy == DenyCrossOriginRequests) {
         m_client->didFail(ResourceError(errorDomainBlinkInternal, 0, request.url().string(), "Cross origin requests are not supported."));
         return;
     }
 
@@ skipping 50 matching lines @@
 
 void DocumentThreadableLoader::makeCrossOriginAccessRequest(const ResourceRequest& request)
 {
     ASSERT(m_options.crossOriginRequestPolicy == UseAccessControl);
 
     // Cross-origin requests are only allowed certain registered schemes.
     // We would catch this when checking response headers later, but there
     // is no reason to send a request, preflighted or not, that's guaranteed
     // to be denied.
     if (!SchemeRegistry::shouldTreatURLSchemeAsCORSEnabled(request.url().protocol())) {
-        m_client->didFailAccessControlCheck(ResourceError(errorDomainBlinkInternal, 0, request.url().string(), "Cross origin requests are only supported for protocol schemes: " + SchemeRegistry::listOfCORSEnabledURLSchemes() + "."));
+        handlePreflightFailure(request.url().string(), "Cross origin requests are only supported for protocol schemes: " + SchemeRegistry::listOfCORSEnabledURLSchemes() + ".");
         return;
     }
 
     // We use isSimpleOrForbiddenRequest() here since |request| may have been
     // modified in the process of loading (not from the user's input). For
     // example, referrer. We need to accept them. For security, we must reject
     // forbidden headers/methods at the point we accept user's input. Not here.
     if ((m_options.preflightPolicy == ConsiderPreflight && FetchUtils::isSimpleOrForbiddenRequest(request.httpMethod(), request.httpHeaderFields())) || m_options.preflightPolicy == PreventPreflight) {
         ResourceRequest crossOriginRequest(request);
         ResourceLoaderOptions crossOriginOptions(m_resourceLoaderOptions);
@@ skipping 152 matching lines @@
             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);
             return;
         }
 
-        ResourceError error(errorDomainBlinkInternal, 0, redirectResponse.url().string(), accessControlErrorDescription);
-        m_client->didFailAccessControlCheck(error);
+        handlePreflightFailure(redirectResponse.url().string(), accessControlErrorDescription);
     } else {
         m_client->didFailRedirectCheck();
     }
 
     clearResource();
     request = ResourceRequest();
 
     m_requestStartedSeconds = 0.0;
 }
 
@@ skipping 79 matching lines @@
         m_client->didReceiveResponse(identifier, response, handle);
         return;
     }
 
     ASSERT(!m_fallbackRequestForServiceWorker);
 
     if (!m_sameOriginRequest && m_options.crossOriginRequestPolicy == UseAccessControl) {
         String accessControlErrorDescription;
         if (!passesAccessControlCheck(&m_document, response, effectiveAllowCredentials(), securityOrigin(), accessControlErrorDescription)) {
             reportResponseReceived(identifier, response);
-            m_client->didFailAccessControlCheck(ResourceError(errorDomainBlinkInternal, 0, response.url().string(), accessControlErrorDescription));
+            handlePreflightFailure(response.url().string(), accessControlErrorDescription);
             return;
         }
     }
 
     m_client->didReceiveResponse(identifier, response, handle);
 }
 
 void DocumentThreadableLoader::dataReceived(Resource* resource, const char* data, unsigned dataLength)
 {
     ASSERT_UNUSED(resource, resource == this->resource());
     ASSERT(m_async);
 
     handleReceivedData(data, dataLength);
 }
 
 void DocumentThreadableLoader::handleReceivedData(const char* data, unsigned dataLength)
 {
     ASSERT(m_client);
 
     // Preflight data should be invisible to clients.
     if (m_actualRequest)
         return;
 
     ASSERT(!m_fallbackRequestForServiceWorker);
 
-    m_client->didReceiveData(data, dataLength);
+    if (!m_accessControlCheckFailed)
+        m_client->didReceiveData(data, dataLength);
 }
 
 void DocumentThreadableLoader::notifyFinished(Resource* resource)
 {
     ASSERT(m_client);
     ASSERT(resource == this->resource());
     ASSERT(m_async);
 
     m_timeoutTimer.stop();
 
     if (resource->errorOccurred())
         m_client->didFail(resource->resourceError());
     else
         handleSuccessfulFinish(resource->identifier(), resource->loadFinishTime());
 }
 
 void DocumentThreadableLoader::handleSuccessfulFinish(unsigned long identifier, double finishTime)
 {
     ASSERT(!m_fallbackRequestForServiceWorker);
 
     if (m_actualRequest) {
         ASSERT(!m_sameOriginRequest);
         ASSERT(m_options.crossOriginRequestPolicy == UseAccessControl);
         loadActualRequest();
     } else {
         // FIXME: Should prevent timeout from being overridden after finished loading, without
         // resetting m_requestStartedSeconds to 0.0
-        m_client->didFinishLoading(identifier, finishTime);
+        if (!m_accessControlCheckFailed)
+            m_client->didFinishLoading(identifier, finishTime);
     }
 }
 
 void DocumentThreadableLoader::didTimeout(Timer<DocumentThreadableLoader>* timer)
 {
     ASSERT_UNUSED(timer, timer == &m_timeoutTimer);
 
     // Using values from net/base/net_error_list.h ERR_TIMED_OUT,
     // Same as existing FIXME above - this error should be coming from FrameLoaderClient to be identifiable.
     static const int timeoutError = -7;
@@ skipping 16 matching lines @@
     OwnPtr<ResourceLoaderOptions> actualOptions;
     actualOptions.swap(m_actualOptions);
 
     actualRequest->setHTTPOrigin(securityOrigin()->toAtomicString());
 
     clearResource();
 
     loadRequest(*actualRequest, *actualOptions);
 }
 
+// Call handlePreflightFailure() instead of didFailAccessControlCheck()
+// from other places in DocumentThreadableLoader.

[kinuko, 2015/02/03 02:35:36]

nit: putting this comment in the header feels more useful / common

[hiroshige, 2015/02/03 08:19:44]

Done.

 void DocumentThreadableLoader::handlePreflightFailure(const String& url, const String& errorDescription)

[tyoshino (SeeGerritForStatus), 2015/02/03 06:22:02]

rename this as it's no longer specific to preflight handling.

[hiroshige, 2015/02/03 08:19:43]

Done.

 {
     ResourceError error(errorDomainBlinkInternal, 0, url, errorDescription);
 
     // Prevent handleSuccessfulFinish() from bypassing access check.
     m_actualRequest = nullptr;
 
+    // Prevent m_client->didReceiveData()/didFinishLoading() from being called
+    // after m_client->didFailAccessControlCheck() is called here.
+    m_accessControlCheckFailed = true;
+
     // FIXME: Should prevent timeout from being overridden after preflight failure, without
     // resetting m_requestStartedSeconds to 0.0
     m_client->didFailAccessControlCheck(error);
 }
 
 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());
@@ skipping 85 matching lines @@
         return DoNotAllowStoredCredentials;
     return m_resourceLoaderOptions.allowCredentials;
 }
 
 SecurityOrigin* DocumentThreadableLoader::securityOrigin() const
 {
     return m_securityOrigin ? m_securityOrigin.get() : m_document.securityOrigin();
 }
 
 } // namespace blink