Fix DCHECK failure at WebRequestProxyingURLLoaderFactory::OnLoaderCreated

When CORS is involved there may be multiple network::URLLoader associated
with one InProgressRequest, because CorsURLLoader may create a new
network::URLLoader for the same request id in redirect handling - see
CorsURLLoader::FollowRedirect. In such a case the old network::URLLoader
is going to be detached fairly soon, so we don't need to take care of it.

Bug: 1043874
Change-Id: Icd91931d817898f59980fbadbb17ad6e63df07cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2011781Reviewed-by: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Matt Mueller <mattm@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#734458}

chrome/test/data/extensions/api_test/webrequest/cors/fetch.html

@@ -8,7 +8,9 @@ const params = (new URL(location.href)).searchParams;
 const hostname = 'cors.example.com';
 const origin = location.protocol + '//' + hostname + ':' + location.port;
 const path = params.get('path');
-const url = origin + '/extensions/api_test/webrequest/cors/' + path;
+const abspath = params.has('abspath') ? params.get('abspath') : undefined;
+const url =
+      origin + (abspath || ('/extensions/api_test/webrequest/cors/' + path));
 const withPreflight = params.has('with-preflight');
 const headers = withPreflight? {'x-foo': 'x-bar'} : undefined;
 

chrome/test/data/extensions/api_test/webrequest/test_cors.js

@@ -638,5 +638,20 @@ runTests([
     registerOnBeforeRequestAndOnErrorOcurredListeners();
     navigateAndWait(getServerURL(
         BASE + 'fetch.html?path=accept&with-preflight'));
-  }
+  },
+  function testCorsServerRedirect() {
+    const url = getServerURL('server-redirect?whatever', 'cors.example.com');
+
+    const callback = callbackPass(() => {});
+    chrome.webRequest.onHeadersReceived.addListener((details) => {
+      if (details.url === url && details.method === 'GET') {
+        callback();
+      }
+    }, {urls: ["http://*/*"]}, ['extraHeaders']);
+
+    const absPath =
+          encodeURIComponent(`/server-redirect?${encodeURIComponent(url)}`);
+    navigateAndWait(getServerURL(
+        BASE + `fetch.html?abspath=${absPath}&with-preflight`));
+  },
 ]);

extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc

@@ -389,6 +389,14 @@ bool WebRequestProxyingURLLoaderFactory::IsForDownload() const {
 
 void WebRequestProxyingURLLoaderFactory::InProgressRequest::OnLoaderCreated(
     mojo::PendingReceiver<network::mojom::TrustedHeaderClient> receiver) {
+  // When CORS is involved there may be multiple network::URLLoader associated
+  // with this InProgressRequest, because CorsURLLoader may create a new
+  // network::URLLoader for the same request id in redirect handling - see
+  // CorsURLLoader::FollowRedirect. In such a case the old network::URLLoader
+  // is going to be detached fairly soon, so we don't need to take care of it.
+  // We need this explicit reset to avoid a DCHECK failure in mojo::Receiver.
+  header_client_receiver_.reset();
+
   header_client_receiver_.Bind(std::move(receiver));
   if (for_cors_preflight_) {
     // In this case we don't have |target_loader_| and

net/test/embedded_test_server/default_handlers.cc

@@ -552,9 +552,19 @@ std::unique_ptr<HttpResponse> HandleServerRedirect(HttpStatusCode redirect_code,
   std::string dest = UnescapeBinaryURLComponent(request_url.query_piece());
   RequestQuery query = ParseQuery(request_url);
 
+  if (request.method == METHOD_OPTIONS) {
+    auto http_response = std::make_unique<BasicHttpResponse>();
+    http_response->set_code(HTTP_OK);
+    http_response->AddCustomHeader("Access-Control-Allow-Origin", "*");
+    http_response->AddCustomHeader("Access-Control-Allow-Methods", "*");
+    http_response->AddCustomHeader("Access-Control-Allow-Headers", "*");
+    return http_response;
+  }
+
   auto http_response = std::make_unique<BasicHttpResponse>();
   http_response->set_code(redirect_code);
   http_response->AddCustomHeader("Location", dest);
+  http_response->AddCustomHeader("Access-Control-Allow-Origin", "*");
   http_response->set_content_type("text/html");
   http_response->set_content(base::StringPrintf(
       "<html><head></head><body>Redirecting to %s</body></html>",