Allow range requests to pass through a service worker

This change implements the following edits to the Fetch spec: whatwg/fetch#560 whatwg/fetch#1076 Existing web tests cover the new functionality. Bug: 847428 Change-Id: Ie63704769e99d4b8d26d8903edf4cc4e3466c124 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339761 Commit-Queue: Nicolas Arciniega <niarci@microsoft.com> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Ben Kelly <wanderview@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Cr-Commit-Position: refs/heads/master@{#800663}
Alan-love · Aug 21, 2020 · 5da0ed1 · 5da0ed1
1 parent fadc13d
commit 5da0ed1
Show file tree

Hide file tree

Showing 31 changed files with 198 additions and 44 deletions.
diff --git a/content/browser/cache_storage/cache_storage_cache_unittest.cc b/content/browser/cache_storage/cache_storage_cache_unittest.cc
@@ -672,7 +672,8 @@ class CacheStorageCacheTest : public testing::Test {
         network::mojom::ParsedHeaders::New(),
         net::HttpResponseInfo::CONNECTION_INFO_UNKNOWN,
         "unknown" /* alpn_negotiated_protocol */,
-        false /* loaded_with_credentials */, false /* was_fetched_via_spdy */);
+        false /* loaded_with_credentials */, false /* was_fetched_via_spdy */,
+        false /* has_range_requested */);
   }
 
   void CopySideDataToResponse(const std::string& uuid,

diff --git a/content/browser/cache_storage/cache_storage_manager_unittest.cc b/content/browser/cache_storage/cache_storage_manager_unittest.cc
@@ -669,7 +669,8 @@ class CacheStorageManagerTest : public testing::Test {
         network::mojom::ParsedHeaders::New(),
         net::HttpResponseInfo::CONNECTION_INFO_UNKNOWN,
         "unknown" /* alpn_negotiated_protocol */,
-        false /* loaded_with_credentials */, false /* was_fetched_via_spdy */);
+        false /* loaded_with_credentials */, false /* was_fetched_via_spdy */,
+        false /* has_range_requested */);
 
     blink::mojom::BatchOperationPtr operation =
         blink::mojom::BatchOperation::New();

diff --git a/content/browser/cache_storage/legacy/legacy_cache_storage_cache.cc b/content/browser/cache_storage/legacy/legacy_cache_storage_cache.cc
@@ -49,6 +49,8 @@
 #include "net/base/net_errors.h"
 #include "net/disk_cache/disk_cache.h"
 #include "net/http/http_request_headers.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_status_code.h"
 #include "services/network/public/mojom/fetch_api.mojom.h"
 #include "storage/browser/blob/blob_storage_context.h"
 #include "storage/browser/quota/padding_key.h"
@@ -425,6 +427,10 @@ blink::mojom::FetchAPIResponsePtr CreateResponse(
   if (metadata.response().has_request_method())
     request_method = metadata.response().request_method();
 
+  // Note that |has_range_requested| can be safely set to false since it only
+  // affects HTTP 206 (Partial) responses, which are blocked from cache storage.
+  // See https://fetch.spec.whatwg.org/#main-fetch for usage of
+  // |has_range_requested|.
   return blink::mojom::FetchAPIResponse::New(
       url_list, metadata.response().status_code(),
       metadata.response().status_text(),
@@ -443,7 +449,8 @@ blink::mojom::FetchAPIResponsePtr CreateResponse(
       static_cast<net::HttpResponseInfo::ConnectionInfo>(
           metadata.response().connection_info()),
       alpn_negotiated_protocol, metadata.response().loaded_with_credentials(),
-      metadata.response().was_fetched_via_spdy());
+      metadata.response().was_fetched_via_spdy(),
+      /* has_range_requested */ false);
 }
 
 // The size of opaque (non-cors) resource responses are padded in order
@@ -1796,6 +1803,7 @@ void LegacyCacheStorageCache::PutDidCreateEntry(
   }
 
   proto::CacheResponse* response_metadata = metadata.mutable_response();
+  DCHECK_NE(put_context->response->status_code, net::HTTP_PARTIAL_CONTENT);
   response_metadata->set_status_code(put_context->response->status_code);
   response_metadata->set_status_text(put_context->response->status_text);
   response_metadata->set_response_type(FetchResponseTypeToProtoResponseType(

diff --git a/content/common/background_fetch/background_fetch_types.cc b/content/common/background_fetch/background_fetch_types.cc
@@ -40,7 +40,7 @@ blink::mojom::FetchAPIResponsePtr BackgroundFetchSettledFetch::CloneResponse(
       CloneSerializedBlob(response->side_data_blob_for_cache_put),
       mojo::Clone(response->parsed_headers), response->connection_info,
       response->alpn_negotiated_protocol, response->loaded_with_credentials,
-      response->was_fetched_via_spdy);
+      response->was_fetched_via_spdy, response->has_range_requested);
 }
 
 // static

diff --git a/content/common/service_worker/service_worker_loader_helpers.cc b/content/common/service_worker/service_worker_loader_helpers.cc
@@ -120,6 +120,7 @@ void ServiceWorkerLoaderHelpers::SaveResponseInfo(
   out_head->connection_info = response.connection_info;
   out_head->alpn_negotiated_protocol = response.alpn_negotiated_protocol;
   out_head->was_fetched_via_spdy = response.was_fetched_via_spdy;
+  out_head->has_range_requested = response.has_range_requested;
 }
 
 // static

diff --git a/content/renderer/loader/web_url_loader_impl.cc b/content/renderer/loader/web_url_loader_impl.cc
@@ -910,6 +910,7 @@ void WebURLLoaderImpl::PopulateURLResponse(
       net::IsCertStatusError(head.cert_status));
   response->SetCTPolicyCompliance(head.ct_policy_compliance);
   response->SetIsLegacyTLSVersion(head.is_legacy_tls_version);
+  response->SetHasRangeRequested(head.has_range_requested);
   response->SetTimingAllowPassed(head.timing_allow_passed);
   response->SetAppCacheID(head.appcache_id);
   response->SetAppCacheManifestURL(head.appcache_manifest_url);

diff --git a/services/network/public/cpp/cors/cors.cc b/services/network/public/cpp/cors/cors.cc
@@ -490,7 +490,14 @@ bool IsNoCorsSafelistedHeaderName(const std::string& name) {
 }
 
 bool IsPrivilegedNoCorsHeaderName(const std::string& name) {
-  return base::ToLowerASCII(name) == "range";
+  const std::string lower_name = base::ToLowerASCII(name);
+  const std::vector<std::string> privileged_no_cors_header_names =
+      PrivilegedNoCorsHeaderNames();
+  for (const auto& header : privileged_no_cors_header_names) {
+    if (lower_name == header)
+      return true;
+  }
+  return false;
 }
 
 bool IsNoCorsSafelistedHeader(const std::string& name,
@@ -560,6 +567,10 @@ std::vector<std::string> CorsUnsafeNotForbiddenRequestHeaderNames(
   return header_names;
 }
 
+std::vector<std::string> PrivilegedNoCorsHeaderNames() {
+  return {"range"};
+}
+
 bool IsForbiddenMethod(const std::string& method) {
   const std::string upper_method = base::ToUpperASCII(method);
   return upper_method == net::HttpRequestHeaders::kConnectMethod ||

diff --git a/services/network/public/cpp/cors/cors.h b/services/network/public/cpp/cors/cors.h
@@ -137,6 +137,12 @@ std::vector<std::string> CorsUnsafeNotForbiddenRequestHeaderNames(
     const net::HttpRequestHeaders::HeaderVector& headers,
     bool is_revalidating);
 
+// https://fetch.spec.whatwg.org/#privileged-no-cors-request-header-name
+// The returned list is NOT sorted.
+// The returned list consists of lower-cased names.
+COMPONENT_EXPORT(NETWORK_CPP)
+std::vector<std::string> PrivilegedNoCorsHeaderNames();
+
 // Checks forbidden method in the fetch spec.
 // See https://fetch.spec.whatwg.org/#forbidden-method.
 // TODO(toyoshim): Move Blink FetchUtils::IsForbiddenMethod to cors:: and use

diff --git a/services/network/public/mojom/url_response_head.mojom b/services/network/public/mojom/url_response_head.mojom
@@ -190,6 +190,9 @@ struct URLResponseHead {
   // will be removed in the future.
   bool is_legacy_tls_version = false;
 
+  // https://fetch.spec.whatwg.org/#concept-response-range-requested-flag
+  bool has_range_requested = false;
+
   // https://fetch.spec.whatwg.org/#concept-response-timing-allow-passed
   bool timing_allow_passed = false;
 

diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc
@@ -131,6 +131,8 @@ void PopulateResourceResponse(net::URLRequest* request,
   response->response_start = base::TimeTicks::Now();
   response->encoded_data_length = request->GetTotalReceivedBytes();
   response->auth_challenge_info = request->auth_challenge_info();
+  response->has_range_requested = request->extra_request_headers().HasHeader(
+      net::HttpRequestHeaders::kRange);
 }
 
 // A subclass of net::UploadBytesElementReader which owns

diff --git a/third_party/blink/public/mojom/fetch/fetch_api_response.mojom b/third_party/blink/public/mojom/fetch/fetch_api_response.mojom
@@ -105,4 +105,7 @@ struct FetchAPIResponse {
   // True if the response was originally loaded via a request fetched over a
   // SPDY channel.
   bool was_fetched_via_spdy = false;
+
+  // https://fetch.spec.whatwg.org/#concept-response-range-requested-flag
+  bool has_range_requested = false;
 };
diff --git a/third_party/blink/public/platform/web_url_response.h b/third_party/blink/public/platform/web_url_response.h
@@ -219,6 +219,7 @@ class WebURLResponse {
   BLINK_PLATFORM_EXPORT void SetHasMajorCertificateErrors(bool);
   BLINK_PLATFORM_EXPORT void SetCTPolicyCompliance(net::ct::CTPolicyCompliance);
   BLINK_PLATFORM_EXPORT void SetIsLegacyTLSVersion(bool);
+  BLINK_PLATFORM_EXPORT void SetHasRangeRequested(bool);
   BLINK_PLATFORM_EXPORT void SetTimingAllowPassed(bool);
 
   BLINK_PLATFORM_EXPORT void SetSecurityStyle(SecurityStyle);

diff --git a/third_party/blink/renderer/core/fetch/fetch_manager.cc b/third_party/blink/renderer/core/fetch/fetch_manager.cc
@@ -714,11 +714,6 @@ void FetchManager::Loader::PerformHTTPFetch() {
 
   request.SetCredentialsMode(fetch_request_data_->Credentials());
   for (const auto& header : fetch_request_data_->HeaderList()->List()) {
-    // Since |fetch_request_data_|'s headers are populated with either of the
-    // "request" guard or "request-no-cors" guard, we can assume that none of
-    // the headers have a name listed in the forbidden header names.
-    DCHECK(!cors::IsForbiddenHeaderName(header.first));
-
     request.AddHttpHeaderField(AtomicString(header.first),
                                AtomicString(header.second));
   }

diff --git a/third_party/blink/renderer/core/fetch/fetch_response_data.cc b/third_party/blink/renderer/core/fetch/fetch_response_data.cc
@@ -141,6 +141,13 @@ const KURL* FetchResponseData::Url() const {
   return &url_list_.back();
 }
 
+uint16_t FetchResponseData::InternalStatus() const {
+  if (internal_response_) {
+    return internal_response_->Status();
+  }
+  return Status();
+}
+
 FetchHeaderList* FetchResponseData::InternalHeaderList() const {
   if (internal_response_) {
     return internal_response_->HeaderList();
@@ -199,6 +206,7 @@ FetchResponseData* FetchResponseData::Clone(ScriptState* script_state,
   new_response->alpn_negotiated_protocol_ = alpn_negotiated_protocol_;
   new_response->loaded_with_credentials_ = loaded_with_credentials_;
   new_response->was_fetched_via_spdy_ = was_fetched_via_spdy_;
+  new_response->has_range_requested_ = has_range_requested_;
 
   switch (type_) {
     case Type::kBasic:
@@ -272,6 +280,7 @@ mojom::blink::FetchAPIResponsePtr FetchResponseData::PopulateFetchAPIResponse(
   response->alpn_negotiated_protocol = alpn_negotiated_protocol_;
   response->loaded_with_credentials = loaded_with_credentials_;
   response->was_fetched_via_spdy = was_fetched_via_spdy_;
+  response->has_range_requested = has_range_requested_;
   for (const auto& header : HeaderList()->List())
     response->headers.insert(header.first, header.second);
   response->parsed_headers = ParseHeaders(
@@ -336,6 +345,8 @@ void FetchResponseData::InitFromResourceResponse(
       request_credentials == network::mojom::CredentialsMode::kInclude ||
       (request_credentials == network::mojom::CredentialsMode::kSameOrigin &&
        tainting == FetchRequestData::kBasicTainting));
+
+  SetHasRangeRequested(response.HasRangeRequested());
 }
 
 FetchResponseData::FetchResponseData(Type type,
@@ -351,7 +362,8 @@ FetchResponseData::FetchResponseData(Type type,
       connection_info_(net::HttpResponseInfo::CONNECTION_INFO_UNKNOWN),
       alpn_negotiated_protocol_("unknown"),
       loaded_with_credentials_(false),
-      was_fetched_via_spdy_(false) {}
+      was_fetched_via_spdy_(false),
+      has_range_requested_(false) {}
 
 void FetchResponseData::ReplaceBodyStreamBuffer(BodyStreamBuffer* buffer) {
   if (type_ == Type::kBasic || type_ == Type::kCors) {

diff --git a/third_party/blink/renderer/core/fetch/fetch_response_data.h b/third_party/blink/renderer/core/fetch/fetch_response_data.h
@@ -68,6 +68,7 @@ class CORE_EXPORT FetchResponseData final
   }
   const KURL* Url() const;
   uint16_t Status() const { return status_; }
+  uint16_t InternalStatus() const;
   AtomicString StatusMessage() const { return status_message_; }
   FetchHeaderList* HeaderList() const { return header_list_.Get(); }
   FetchHeaderList* InternalHeaderList() const;
@@ -82,6 +83,7 @@ class CORE_EXPORT FetchResponseData final
   const HTTPHeaderSet& CorsExposedHeaderNames() const {
     return cors_exposed_header_names_;
   }
+  bool HasRangeRequested() const { return has_range_requested_; }
 
   void SetResponseSource(network::mojom::FetchResponseSource response_source) {
     response_source_ = response_source;
@@ -120,6 +122,9 @@ class CORE_EXPORT FetchResponseData final
   void SetWasFetchedViaSpdy(bool was_fetched_via_spdy) {
     was_fetched_via_spdy_ = was_fetched_via_spdy;
   }
+  void SetHasRangeRequested(bool has_range_requested) {
+    has_range_requested_ = has_range_requested;
+  }
 
   // If the type is Default, replaces |buffer_|.
   // If the type is Basic or CORS, replaces |buffer_| and
@@ -160,6 +165,7 @@ class CORE_EXPORT FetchResponseData final
   AtomicString alpn_negotiated_protocol_;
   bool loaded_with_credentials_;
   bool was_fetched_via_spdy_;
+  bool has_range_requested_;
 
   DISALLOW_COPY_AND_ASSIGN(FetchResponseData);
 };

diff --git a/third_party/blink/renderer/core/fetch/headers.cc b/third_party/blink/renderer/core/fetch/headers.cc
@@ -126,6 +126,10 @@ void Headers::append(const String& name,
   }
   // "7. Append |name|/|value| to header list."
   header_list_->Append(name, normalized_value);
+  // "8. If this’s guard is |request-no-cors|, then remove privileged no-CORS
+  // request headers from this."
+  if (guard_ == kRequestNoCorsGuard)
+    RemovePrivilegedNoCorsRequestHeaders();
 }
 
 void Headers::remove(const String& name, ExceptionState& exception_state) {
@@ -158,8 +162,15 @@ void Headers::remove(const String& name, ExceptionState& exception_state) {
       FetchUtils::IsForbiddenResponseHeaderName(name)) {
     return;
   }
-  // "6. Delete |name| from header list."
+  // "6. If this’s header list does not contain |name|, then return."
+  if (!header_list_->Has(name))
+    return;
+  // "7. Delete |name| from header list."
   header_list_->Remove(name);
+  // "8. If this’s guard is |request-no-cors|, then remove privileged no-CORS
+  // request headers from this."
+  if (guard_ == kRequestNoCorsGuard)
+    RemovePrivilegedNoCorsRequestHeaders();
 }
 
 String Headers::get(const String& name, ExceptionState& exception_state) {
@@ -228,6 +239,10 @@ void Headers::set(const String& name,
   }
   // "7. Set |name|/|value| in header list."
   header_list_->Set(name, normalized_value);
+  // "8. If this’s guard is |request-no-cors|, then remove privileged no-CORS
+  // request headers from this."
+  if (guard_ == kRequestNoCorsGuard)
+    RemovePrivilegedNoCorsRequestHeaders();
 }
 
 // This overload is not called directly by Web APIs, but rather by other C++
@@ -285,6 +300,13 @@ void Headers::FillWith(const Vector<std::pair<String, String>>& object,
   }
 }
 
+void Headers::RemovePrivilegedNoCorsRequestHeaders() {
+  const Vector<String> privileged_no_cors_header_names =
+      cors::PrivilegedNoCorsHeaderNames();
+  for (const auto& header : privileged_no_cors_header_names)
+    header_list_->Remove(header);
+}
+
 Headers::Headers()
     : header_list_(MakeGarbageCollected<FetchHeaderList>()),
       guard_(kNoneGuard) {}

diff --git a/third_party/blink/renderer/core/fetch/headers.h b/third_party/blink/renderer/core/fetch/headers.h
@@ -59,6 +59,9 @@ class CORE_EXPORT Headers final : public ScriptWrappable,
   void FillWith(const Headers*, ExceptionState&);
   void FillWith(const HeadersInit&, ExceptionState&);
 
+  // https://fetch.spec.whatwg.org/#concept-headers-remove-privileged-no-cors-request-headers
+  void RemovePrivilegedNoCorsRequestHeaders();
+
   FetchHeaderList* HeaderList() const { return header_list_; }
   void Trace(Visitor*) const override;
 

diff --git a/third_party/blink/renderer/core/fetch/request.cc b/third_party/blink/renderer/core/fetch/request.cc
@@ -564,19 +564,11 @@ Request* Request::CreateRequestWithRequestOrString(
   // "Let |r| be a new Request object associated with |request| and a new
   // Headers object whose guard is "request"."
   Request* r = Request::Create(script_state, request);
-  // Perform the following steps:
-  // - "Let |headers| be a copy of |r|'s Headers object."
-  // - "If |init|'s headers member is present, set |headers| to |init|'s
-  //   headers member."
-  //
-  // We don't create a copy of r's Headers object when init's headers member
-  // is present.
-  Headers* headers = nullptr;
-  if (!init->hasHeaders()) {
-    headers = r->getHeaders()->Clone();
-  }
-  // "Empty |r|'s request's header list."
-  r->request_->HeaderList()->ClearList();
+
+  // "If |signal| is not null, then make |r|’s signal follow |signal|."
+  if (signal)
+    r->signal_->Follow(signal);
+
   // "If |r|'s request's mode is "no-cors", run these substeps:
   if (r->GetRequest()->Mode() == network::mojom::RequestMode::kNoCors) {
     // "If |r|'s request's method is not a CORS-safelisted method, throw a
@@ -589,19 +581,32 @@ Request* Request::CreateRequestWithRequestOrString(
     // "Set |r|'s Headers object's guard to "request-no-cors"."
     r->getHeaders()->SetGuard(Headers::kRequestNoCorsGuard);
   }
-  // "If |signal| is not null, then make |r|’s signal follow |signal|."
-  if (signal)
-    r->signal_->Follow(signal);
 
-  // "Fill |r|'s Headers object with |headers|. Rethrow any exceptions."
-  if (init->hasHeaders()) {
-    r->getHeaders()->FillWith(init->headers(), exception_state);
-  } else {
-    DCHECK(headers);
-    r->getHeaders()->FillWith(headers, exception_state);
+  if (AreAnyMembersPresent(init)) {
+    // Perform the following steps:
+    // - "Let |headers| be a copy of |r|'s Headers object."
+    // - "If |init|'s headers member is present, set |headers| to |init|'s
+    //   headers member."
+    //
+    // We don't create a copy of r's Headers object when init's headers member
+    // is present.
+    Headers* headers = nullptr;
+    if (!init->hasHeaders()) {
+      headers = r->getHeaders()->Clone();
+    }
+    // "Empty |r|'s request's header list."
+    r->request_->HeaderList()->ClearList();
+
+    // "Fill |r|'s Headers object with |headers|. Rethrow any exceptions."
+    if (init->hasHeaders()) {
+      r->getHeaders()->FillWith(init->headers(), exception_state);
+    } else {
+      DCHECK(headers);
+      r->getHeaders()->FillWith(headers, exception_state);
+    }
+    if (exception_state.HadException())
+      return nullptr;
   }
-  if (exception_state.HadException())
-    return nullptr;
 
   // "Let |inputBody| be |input|'s request's body if |input| is a
   //   Request object, and null otherwise."

diff --git a/third_party/blink/renderer/core/fetch/response.cc b/third_party/blink/renderer/core/fetch/response.cc
@@ -389,6 +389,7 @@ FetchResponseData* Response::CreateUnfilteredFetchResponseDataWithoutBody(
   response->SetLoadedWithCredentials(
       fetch_api_response.loaded_with_credentials);
   response->SetWasFetchedViaSpdy(fetch_api_response.was_fetched_via_spdy);
+  response->SetHasRangeRequested(fetch_api_response.has_range_requested);
 
   for (const auto& header : fetch_api_response.headers)
     response->HeaderList()->Append(header.key, header.value);