Suppress CORB warning for requests not using the GET HTTP method.

Requests using HTTP methods other than GET (e.g. OPTIONS used for CORS preflight or POST used for form submissions) do not need to report warning messages about CORB, because: - For XHR/fetch the *CORS* message should be sufficient - If no-cors mode was forced for XHR/fetch, then the response is opaque and therefore CORB warning message is not needed - Other subresource requests (e.g. for img or script) tag should only use the GET method. Bug: 945767 Change-Id: I9302362aabe407d2bc3cae305e59be0f64f2cb3b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1539206 Commit-Queue: Łukasz Anforowicz <[email protected]> Reviewed-by: Charlie Reis <[email protected]> Cr-Commit-Position: refs/heads/master@{#645322}
feiyue1206 · Mar 28, 2019 · f6fb1b3 · f6fb1b3
1 parent 4a01823
commit f6fb1b3
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 0 deletions.
diff --git a/services/network/cross_origin_read_blocking.cc b/services/network/cross_origin_read_blocking.cc
@@ -577,6 +577,7 @@ CrossOriginReadBlocking::ResponseAnalyzer::ResponseAnalyzer(
   content_length_ = response.head.content_length;
   http_response_code_ =
       response.head.headers ? response.head.headers->response_code() : 0;
+  http_method_ = request.method();
   request_initiator_site_lock_ = request_initiator_site_lock;
 
   should_block_based_on_headers_ =
@@ -897,6 +898,17 @@ bool CrossOriginReadBlocking::ResponseAnalyzer::ShouldReportBlockedResponse()
   if (400 <= http_response_code() && http_response_code() <= 599)
     return false;
 
+  // Requests using HTTP methods other than GET (e.g. OPTIONS used for CORS
+  // preflight or POST used for form submissions) do not need to report warning
+  // messages about CORB, because:
+  // - For XHR/fetch the *CORS* message should be sufficient
+  // - If no-cors mode was forced for XHR/fetch, then the response is opaque
+  //   and therefore CORB warning message is not needed
+  // - Other subresource requests (e.g. for img or script) tag should
+  //   only use the GET method.
+  if (!base::EqualsCaseInsensitiveASCII(http_method_, "GET"))
+    return false;
+
   return true;
 }
 

diff --git a/services/network/cross_origin_read_blocking.h b/services/network/cross_origin_read_blocking.h
@@ -160,6 +160,9 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) CrossOriginReadBlocking {
     // resource request.
     int http_response_code_ = 0;
 
+    // The HTTP method of the request (e.g. "GET", "OPTIONS", "POST", etc.).
+    std::string http_method_;
+
     // Whether |request_initiator| was compatible with
     // |request_initiator_site_lock|.  For safety initialized to kIncorrectLock,
     // but in practice it will always be explicitly set by the constructor.

diff --git a/...b_tests/http/tests/inspector-protocol/network/block_cross_site_document_load-expected.txt b/...b_tests/http/tests/inspector-protocol/network/block_cross_site_document_load-expected.txt
@@ -1,6 +1,12 @@
 Tests the blockedCrossSiteDocumentLoad bit available as a part of the loading finished signal.
 Blocking cross-site document at http://devtools.oopif.test:8000/inspector-protocol/network/resources/nosniff.pl: shouldReportCorbBlocking=true.
+Blocking cross-site document at http://devtools.oopif.test:8000/inspector-protocol/network/resources/nosniff.pl fetched via GET: shouldReportCorbBlocking=true.
+Blocking cross-site document at http://devtools.oopif.test:8000/inspector-protocol/network/resources/nosniff.pl fetched via OPTIONS: shouldReportCorbBlocking=false.
+Blocking cross-site document at http://devtools.oopif.test:8000/inspector-protocol/network/resources/nosniff.pl fetched via POST: shouldReportCorbBlocking=false.
 Blocking cross-site document at http://devtools.oopif.test:8000/inspector-protocol/network/resources/simple-iframe.html: shouldReportCorbBlocking=true.
+Blocking cross-site document at http://devtools.oopif.test:8000/inspector-protocol/network/resources/simple-iframe.html fetched via GET: shouldReportCorbBlocking=true.
+Blocking cross-site document at http://devtools.oopif.test:8000/inspector-protocol/network/resources/simple-iframe.html fetched via OPTIONS: shouldReportCorbBlocking=false.
+Blocking cross-site document at http://devtools.oopif.test:8000/inspector-protocol/network/resources/simple-iframe.html fetched via POST: shouldReportCorbBlocking=false.
 Blocking, but not reporting cross-site document at http://devtools.oopif.test:8000/inspector-protocol/network/resources/204.pl: shouldReportCorbBlocking=false.
 Blocking, but not reporting cross-site document at http://devtools.oopif.test:8000/inspector-protocol/network/resources/404.pl: shouldReportCorbBlocking=false.
 Blocking, but not reporting cross-site document at http://devtools.oopif.test:8000/inspector-protocol/network/resources/content-length-0.pl: shouldReportCorbBlocking=false.

diff --git a/...y/blink/web_tests/http/tests/inspector-protocol/network/block_cross_site_document_load.js b/...y/blink/web_tests/http/tests/inspector-protocol/network/block_cross_site_document_load.js
@@ -16,6 +16,16 @@
     testRunner.log(
         `Blocking cross-site document at ${url}: ` +
         `shouldReportCorbBlocking=${response.params.shouldReportCorbBlocking}.`);
+
+    // Only GET method requests should show a warning message.
+    let http_methods = ['GET', 'OPTIONS', 'POST'];
+    for (const method of http_methods) {
+      session.evaluate(`fetch('${url}', {method: '${method}'});`);
+      const response = await dp.Network.onceLoadingFinished();
+      testRunner.log(
+          `Blocking cross-site document at ${url} fetched via ${method}: ` +
+          `shouldReportCorbBlocking=${response.params.shouldReportCorbBlocking}.`);
+    }
   }
 
   let blocked_unreported_urls = [