Clean up SecurityOrigin handling around CrossOriginAccessControl::handleRedirect()

third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.cpp

 /*
  * Copyright (C) 2008 Apple Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 38 matching lines @@
         "cache-control",
         "content-language",
         "content-type",
         "expires",
         "last-modified",
         "pragma",
     })));
     return allowedCrossOriginResponseHeaders.contains(name);
 }
 
-void updateRequestForAccessControl(ResourceRequest& request, SecurityOrigin* securityOrigin, StoredCredentials allowCredentials)
+void updateRequestForAccessControl(ResourceRequest& request, const SecurityOrigin* securityOrigin, StoredCredentials allowCredentials)
 {
     request.removeCredentials();
     request.setAllowStoredCredentials(allowCredentials == AllowStoredCredentials);
 
     if (securityOrigin)
         request.setHTTPOrigin(securityOrigin);
 }
 
-ResourceRequest createAccessControlPreflightRequest(const ResourceRequest& request, SecurityOrigin* securityOrigin)
+ResourceRequest createAccessControlPreflightRequest(const ResourceRequest& request, const SecurityOrigin* securityOrigin)
 {
     ResourceRequest preflightRequest(request.url());
     updateRequestForAccessControl(preflightRequest, securityOrigin, DoNotAllowStoredCredentials);
     preflightRequest.setHTTPMethod(HTTPNames::OPTIONS);
     preflightRequest.setHTTPHeaderField(HTTPNames::Access_Control_Request_Method, AtomicString(request.httpMethod()));
     preflightRequest.setPriority(request.priority());
     preflightRequest.setRequestContext(request.requestContext());
     preflightRequest.setSkipServiceWorker(WebURLRequest::SkipServiceWorker::All);
 
     if (request.isExternalRequest())
@@ skipping 38 matching lines @@
 }
 
 static bool isInterestingStatusCode(int statusCode)
 {
     // Predicate that gates what status codes should be included in
     // console error messages for responses containing no access
     // control headers.
     return statusCode >= 400;
 }
 
-static String buildAccessControlFailureMessage(const String& detail, SecurityOrigin* securityOrigin)
+static String buildAccessControlFailureMessage(const String& detail, const SecurityOrigin* securityOrigin)
 {
     return detail + " Origin '" + securityOrigin->toString() + "' is therefore not allowed access.";
 }
 
-bool passesAccessControlCheck(const ResourceResponse& response, StoredCredentials includeCredentials, SecurityOrigin* securityOrigin, String& errorDescription, WebURLRequest::RequestContext context)
+bool passesAccessControlCheck(const ResourceResponse& response, StoredCredentials includeCredentials, const SecurityOrigin* securityOrigin, String& errorDescription, WebURLRequest::RequestContext context)
 {
     DEFINE_THREAD_SAFE_STATIC_LOCAL(AtomicString, allowOriginHeaderName, (new AtomicString("access-control-allow-origin")));
     DEFINE_THREAD_SAFE_STATIC_LOCAL(AtomicString, allowCredentialsHeaderName, (new AtomicString("access-control-allow-credentials")));
     DEFINE_THREAD_SAFE_STATIC_LOCAL(AtomicString, allowSuboriginHeaderName, (new AtomicString("access-control-allow-suborigin")));
 
     // TODO(esprehn): This code is using String::append extremely inefficiently
     // causing tons of copies. It should pass around a StringBuilder instead.
 
     int statusCode = response.httpStatusCode();
 
@@ skipping 122 matching lines @@
     if (response.wasFetchedViaServiceWorker()) {
         for (const auto& header : response.corsExposedHeaderNames())
             headerSet.add(header);
         return;
     }
     parseAccessControlExposeHeadersAllowList(response.httpHeaderField(HTTPNames::Access_Control_Expose_Headers), headerSet);
 }
 
 bool CrossOriginAccessControl::isLegalRedirectLocation(const KURL& requestURL, String& errorDescription)
 {
-    // CORS restrictions imposed on Location: URL -- http://www.w3.org/TR/cors/#redirect-steps (steps 2 + 3.)
+    // Block non HTTP(S) schemes as specified in the step 4 in
+    // https://fetch.spec.whatwg.org/#http-redirect-fetch. Chromium also allows
+    // the data scheme.
+    //
+    // TODO(tyoshino): This check should be performed regardless of the CORS
+    // flag and request's mode.
     if (!SchemeRegistry::shouldTreatURLSchemeAsCORSEnabled(requestURL.protocol())) {
         errorDescription = "Redirect location '" + requestURL.getString() + "' has a disallowed scheme for cross-origin requests.";
         return false;
     }
 
+    // Block URLs including credentials as specified in the step 9 in
+    // https://fetch.spec.whatwg.org/#http-redirect-fetch.
+    //
+    // TODO(tyoshino): This check should be performed also when request's
+    // origin is not same origin with the redirect destination's origin.
     if (!(requestURL.user().isEmpty() && requestURL.pass().isEmpty())) {
         errorDescription = "Redirect location '" + requestURL.getString() + "' contains userinfo, which is disallowed for cross-origin requests.";
         return false;
     }
 
     return true;
 }
 
-bool CrossOriginAccessControl::handleRedirect(SecurityOrigin* securityOrigin, ResourceRequest& newRequest, const ResourceResponse& redirectResponse, StoredCredentials withCredentials, ResourceLoaderOptions& options, String& errorMessage)
+bool CrossOriginAccessControl::handleRedirect(const SecurityOrigin* securityOrigin, ResourceRequest& newRequest, const ResourceResponse& redirectResponse, StoredCredentials withCredentials, ResourceLoaderOptions& options, String& errorMessage)
 {
     // http://www.w3.org/TR/cors/#redirect-steps terminology:
-    const KURL& originalURL = redirectResponse.url();
+    const KURL& lastURL = redirectResponse.url();
     const KURL& newURL = newRequest.url();
 
-    bool redirectCrossOrigin = !securityOrigin->canRequest(newURL);
+    const SecurityOrigin* securityOriginForHeader = securityOrigin;
 
-    // Same-origin request URLs that redirect are allowed without checking access.
-    if (!securityOrigin->canRequest(originalURL)) {
+    // TODO(tyoshino): This should be fixed to check not only the last one but
+    // all redirect responses.
+    if (!securityOrigin->canRequest(lastURL)) {
         // Follow http://www.w3.org/TR/cors/#redirect-steps
         String errorDescription;
 
-        // Steps 3 & 4 - check if scheme and other URL restrictions hold.
         if (!isLegalRedirectLocation(newURL, errorDescription)) {
-            errorMessage = "Redirect from '" + originalURL.getString() + "' has been blocked by CORS policy: " + errorDescription;
+            errorMessage = "Redirect from '" + lastURL.getString() + "' has been blocked by CORS policy: " + errorDescription;
             return false;
         }
 
         // Step 5: perform resource sharing access check.
         if (!passesAccessControlCheck(redirectResponse, withCredentials, securityOrigin, errorDescription, newRequest.requestContext())) {
-            errorMessage = "Redirect from '" + originalURL.getString() + "' has been blocked by CORS policy: " + errorDescription;
+            errorMessage = "Redirect from '" + lastURL.getString() + "' has been blocked by CORS policy: " + errorDescription;
             return false;
         }
 
-        RefPtr<SecurityOrigin> originalOrigin = SecurityOrigin::create(originalURL);
-        // Step 6: if the request URL origin is not same origin as the original URL's,
-        // set the source origin to a globally unique identifier.
-        if (!originalOrigin->canRequest(newURL)) {
+        RefPtr<SecurityOrigin> lastOrigin = SecurityOrigin::create(lastURL);
+        // Set request's origin to a globally unique identifier as specified in
+        // the step 10 in https://fetch.spec.whatwg.org/#http-redirect-fetch.
+        if (!lastOrigin->canRequest(newURL)) {
             options.securityOrigin = SecurityOrigin::createUnique();
-            securityOrigin = options.securityOrigin.get();
+            securityOriginForHeader = options.securityOrigin.get();
         }
     }
-    if (redirectCrossOrigin) {
-        // If now to a different origin, update/set Origin:.
+
+    if (!securityOrigin->canRequest(newURL)) {
         newRequest.clearHTTPOrigin();
-        newRequest.setHTTPOrigin(securityOrigin);
-        // 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.
+        newRequest.setHTTPOrigin(securityOriginForHeader);
+
+        // Unset credentials flag if request's credentials mode is
+        // "same-origin" as request's response tainting becomes "cors".
+        //
+        // This is equivalent to the step 2 in
+        // https://fetch.spec.whatwg.org/#http-network-or-cache-fetch
         if (options.credentialsRequested == ClientDidNotRequestCredentials)
             options.allowCredentials = DoNotAllowStoredCredentials;
     }
     return true;
 }
 
 } // namespace blink