Revert "Fetch: Plumb request initiator through passthrough service wo…

…rkers." This reverts commit da0a6501cf321579bd46a27ff9fba1bb8ea910bb. Reason for revert: Failure on many bots with the following error message: The service worker navigation preload request was cancelled before 'preloadResponse' settled. If you intend to use 'preloadResponse', use waitUntil() or respondWith() to wait for the promise to settle.", source: (0) Original change's description: > Fetch: Plumb request initiator through passthrough service workers. > > This CL contains essentially two changes: > > 1. The request initiator origin is plumbed through service workers > that do `fetch(evt.request)`. In addition to plumbing, this > requires changes to how we validate navigation requests in the > CorsURLLoaderFactory. > 2. Tracks the original destination of a request passed through a > service worker. This is then used in the network service to force > SameSite=Lax cookies to treat the request as a main frame navigation > where appropriate. > > For more detailed information about these changes please see the > internal design doc at: > > https://docs.google.com/document/d/1KZscujuV7bCFEnzJW-0DaCPU-I40RJimQKoCcI0umTQ/edit?usp=sharing > > In addition, there is some discussion of these features in the following > spec issues: > > whatwg/fetch#1321 > whatwg/fetch#1327 > > The test includes WPT tests that verify navigation headers and SameSite > cookies. Note, chrome has a couple expected failures in the SameSite > cookie tests because of the "lax-allowing-unsafe" intervention that is > currently enabled. See: > > https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/TestExpectations;l=4635;drc=e8133cbf2469adb99c6610483ab78bcfb8cc4c76 > > Bug: 1115847,1241188 > Change-Id: I7e236fa20aeabb705aef40fcf8d5c36da6d2798c > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3115917 > Reviewed-by: Matt Menke <mmenke@chromium.org> > Reviewed-by: Yutaka Hirano <yhirano@chromium.org> > Reviewed-by: Nasko Oskov <nasko@chromium.org> > Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org> > Commit-Queue: Ben Kelly <wanderview@chromium.org> > Cr-Commit-Position: refs/heads/main@{#936029} Bug: 1115847,1241188 Change-Id: I3044a6d20de172b4a8ab7e39a9f26191580003fa No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3251692 Auto-Submit: Alan Screen <awscreen@chromium.org> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: Alan Screen <awscreen@chromium.org> Owners-Override: Alan Screen <awscreen@chromium.org> Cr-Commit-Position: refs/heads/main@{#936125} NOKEYCHECK=True GitOrigin-RevId: a6601b2cf2bb7c0a0ffa3c795a0dbc730ef81d1a
mjfroman · Oct 28, 2021 · 6ea67f2 · 6ea67f2
1 parent 9bf3693
commit 6ea67f2
Show file tree

Hide file tree

Showing 18 changed files with 23 additions and 950 deletions.
diff --git a/blink/public/mojom/fetch/fetch_api_request.mojom b/blink/public/mojom/fetch/fetch_api_request.mojom
@@ -14,7 +14,6 @@ import "services/network/public/mojom/url_request.mojom";
 import "third_party/blink/public/mojom/blob/serialized_blob.mojom";
 import "third_party/blink/public/mojom/loader/request_context_frame_type.mojom";
 import "third_party/blink/public/mojom/loader/referrer.mojom";
-import "url/mojom/origin.mojom";
 import "url/mojom/url.mojom";
 
 
@@ -163,14 +162,6 @@ struct FetchAPIRequest {
   SerializedBlob? blob;
   FetchAPIRequestBody? body;
 
-  // `request_initiator` indicates the origin that initiated the request.  See
-  // also `network::ResourceRequest::request_initiator`, and the doc comment
-  // for `request_initiator` in services/network/public/mojom/url_request.mojom.
-  //
-  // Note that the origin may be missing for browser-initiated navigations
-  // (e.g. ones initiated from the Omnibox).
-  url.mojom.Origin? request_initiator;
-
   Referrer? referrer;
   network.mojom.CredentialsMode credentials_mode =
     network.mojom.CredentialsMode.kOmit;

diff --git a/blink/renderer/core/fetch/fetch_manager.cc b/blink/renderer/core/fetch/fetch_manager.cc
@@ -762,7 +762,22 @@ void FetchManager::Loader::PerformHTTPFetch() {
   request.SetHttpMethod(fetch_request_data_->Method());
   request.SetFetchWindowId(fetch_request_data_->WindowId());
   request.SetTrustTokenParams(fetch_request_data_->TrustTokenParams());
-  request.SetMode(fetch_request_data_->Mode());
+
+  switch (fetch_request_data_->Mode()) {
+    case RequestMode::kSameOrigin:
+    case RequestMode::kNoCors:
+    case RequestMode::kCors:
+    case RequestMode::kCorsWithForcedPreflight:
+      request.SetMode(fetch_request_data_->Mode());
+      break;
+    case RequestMode::kNavigate:
+      // NetworkService (i.e. CorsURLLoaderFactory::IsSane) rejects kNavigate
+      // requests coming from renderers, so using kSameOrigin here.
+      // TODO(lukasza): Tweak CorsURLLoaderFactory::IsSane to accept kNavigate
+      // if request_initiator and the target are same-origin.
+      request.SetMode(RequestMode::kSameOrigin);
+      break;
+  }
 
   request.SetCredentialsMode(fetch_request_data_->Credentials());
   for (const auto& header : fetch_request_data_->HeaderList()->List()) {
@@ -810,8 +825,6 @@ void FetchManager::Loader::PerformHTTPFetch() {
     UseCounter::Count(execution_context_, mojom::WebFeature::kFetchKeepalive);
   }
 
-  request.SetOriginalDestination(fetch_request_data_->OriginalDestination());
-
   // "3. Append `Host`, ..."
   // FIXME: Implement this when the spec is fixed.
 

diff --git a/blink/renderer/core/fetch/fetch_request_data.cc b/blink/renderer/core/fetch/fetch_request_data.cc
@@ -164,8 +164,6 @@ FetchRequestData* FetchRequestData::Create(
   // we deprecate SetContext.
 
   request->SetDestination(fetch_api_request->destination);
-  if (fetch_api_request->request_initiator)
-    request->SetOrigin(fetch_api_request->request_initiator);
   request->SetReferrerString(AtomicString(Referrer::NoReferrer()));
   if (fetch_api_request->referrer) {
     if (!fetch_api_request->referrer->url.IsEmpty()) {
@@ -186,7 +184,6 @@ FetchRequestData* FetchRequestData::Create(
       fetch_api_request->priority));
   if (fetch_api_request->fetch_window_id)
     request->SetWindowId(fetch_api_request->fetch_window_id.value());
-
   return request;
 }
 
@@ -208,7 +205,6 @@ FetchRequestData* FetchRequestData::CloneExceptBody() {
   request->integrity_ = integrity_;
   request->priority_ = priority_;
   request->importance_ = importance_;
-  request->original_destination_ = original_destination_;
   request->keepalive_ = keepalive_;
   request->is_history_navigation_ = is_history_navigation_;
   request->window_id_ = window_id_;

diff --git a/blink/renderer/core/fetch/fetch_request_data.h b/blink/renderer/core/fetch/fetch_request_data.h
@@ -108,15 +108,6 @@ class CORE_EXPORT FetchRequestData final
   void SetIntegrity(const String& integrity) { integrity_ = integrity; }
   ResourceLoadPriority Priority() const { return priority_; }
   void SetPriority(ResourceLoadPriority priority) { priority_ = priority; }
-
-  // The original destination of a request passed through by a service worker.
-  void SetOriginalDestination(network::mojom::RequestDestination value) {
-    original_destination_ = value;
-  }
-  network::mojom::RequestDestination OriginalDestination() const {
-    return original_destination_;
-  }
-
   bool Keepalive() const { return keepalive_; }
   void SetKeepalive(bool b) { keepalive_ = b; }
   bool IsHistoryNavigation() const { return is_history_navigation_; }
@@ -183,8 +174,6 @@ class CORE_EXPORT FetchRequestData final
   String mime_type_;
   String integrity_;
   ResourceLoadPriority priority_;
-  network::mojom::RequestDestination original_destination_ =
-      network::mojom::RequestDestination::kEmpty;
   bool keepalive_;
   bool is_history_navigation_ = false;
   // A specific factory that should be used for this request instead of whatever

diff --git a/blink/renderer/core/fetch/request.cc b/blink/renderer/core/fetch/request.cc
@@ -70,8 +70,7 @@ FetchRequestData* CreateCopyOfFetchRequestDataForFetch(
   request->SetURL(original->Url());
   request->SetMethod(original->Method());
   request->SetHeaderList(original->HeaderList()->Clone());
-  request->SetOrigin(original->Origin() ? original->Origin()
-                                        : context->GetSecurityOrigin());
+  request->SetOrigin(context->GetSecurityOrigin());
   // FIXME: Set client.
   DOMWrapperWorld& world = script_state->World();
   if (world.IsIsolatedWorld()) {
@@ -98,18 +97,6 @@ FetchRequestData* CreateCopyOfFetchRequestDataForFetch(
   }
   request->SetWindowId(original->WindowId());
   request->SetTrustTokenParams(original->TrustTokenParams());
-
-  // When a new request is created from another the destination is always reset
-  // to be `kEmpty`.  In order to facilitate some later checks when a service
-  // worker forwards a navigation request we want to keep track of the
-  // destination of the original request.  Therefore record the original
-  // request's destination if its non-empty, otherwise just carry forward
-  // whatever "original destination" value was already set.
-  if (original->Destination() != network::mojom::RequestDestination::kEmpty)
-    request->SetOriginalDestination(original->Destination());
-  else
-    request->SetOriginalDestination(original->OriginalDestination());
-
   return request;
 }
 
@@ -332,9 +319,6 @@ Request* Request::CreateRequestWithRequestOrString(
 
   // "If any of |init|'s members are present, then:"
   if (AreAnyMembersPresent(init)) {
-    request->SetOrigin(execution_context->GetSecurityOrigin());
-    request->SetOriginalDestination(network::mojom::RequestDestination::kEmpty);
-
     // "If |request|'s |mode| is "navigate", then set it to "same-origin".
     if (request->Mode() == network::mojom::RequestMode::kNavigate)
       request->SetMode(network::mojom::RequestMode::kSameOrigin);
@@ -998,7 +982,6 @@ mojom::blink::FetchAPIRequestPtr Request::CreateFetchAPIRequest() const {
   fetch_api_request->integrity = request_->Integrity();
   fetch_api_request->is_history_navigation = request_->IsHistoryNavigation();
   fetch_api_request->destination = request_->Destination();
-  fetch_api_request->request_initiator = request_->Origin();
 
   // Strip off the fragment part of URL. So far, all callers expect the fragment
   // to be excluded.

diff --git a/blink/renderer/modules/cache_storage/inspector_cache_storage_agent.cc b/blink/renderer/modules/cache_storage/inspector_cache_storage_agent.cc
@@ -268,12 +268,11 @@ class ResponsesAccumulator : public RefCounted<ResponsesAccumulator> {
       auto request_clone_without_body = mojom::blink::FetchAPIRequest::New(
           request->mode, request->is_main_resource_load, request->destination,
           request->frame_type, request->url, request->method, request->headers,
-          nullptr /* blob */, ResourceRequestBody(), request->request_initiator,
-          request->referrer.Clone(), request->credentials_mode,
-          request->cache_mode, request->redirect_mode, request->integrity,
-          request->priority, request->fetch_window_id, request->keepalive,
-          request->is_reload, request->is_history_navigation,
-          request->devtools_stack_id);
+          nullptr /* blob */, ResourceRequestBody(), request->referrer.Clone(),
+          request->credentials_mode, request->cache_mode,
+          request->redirect_mode, request->integrity, request->priority,
+          request->fetch_window_id, request->keepalive, request->is_reload,
+          request->is_history_navigation, request->devtools_stack_id);
       cache_remote_->Match(
           std::move(request), mojom::blink::CacheQueryOptions::New(),
           /*in_related_fetch_event=*/false, /*in_range_fetch_event=*/false,

diff --git a/blink/renderer/platform/loader/fetch/resource_request.h b/blink/renderer/platform/loader/fetch/resource_request.h
@@ -513,14 +513,6 @@ class PLATFORM_EXPORT ResourceRequestHead {
     return allowHTTP1ForStreamingUpload_;
   }
 
-  // The original destination of a request passed through by a service worker.
-  network::mojom::RequestDestination GetOriginalDestination() const {
-    return original_destination_;
-  }
-  void SetOriginalDestination(network::mojom::RequestDestination value) {
-    original_destination_ = value;
-  }
-
   const absl::optional<ResourceRequestHead::WebBundleTokenParams>&
   GetWebBundleTokenParams() const {
     return web_bundle_token_params_;
@@ -611,9 +603,6 @@ class PLATFORM_EXPORT ResourceRequestHead {
 
   base::UnguessableToken fetch_window_id_;
 
-  network::mojom::RequestDestination original_destination_ =
-      network::mojom::RequestDestination::kEmpty;
-
   uint64_t inspector_id_ = 0;
 
   bool is_from_origin_dirty_style_sheet_ = false;

diff --git a/blink/renderer/platform/loader/fetch/url_loader/request_conversion.cc b/blink/renderer/platform/loader/fetch/url_loader/request_conversion.cc
@@ -420,8 +420,6 @@ void PopulateResourceRequest(const ResourceRequestHead& src,
     dest->headers.SetHeaderIfMissing(net::HttpRequestHeaders::kAccept,
                                      network::kDefaultAcceptHeaderValue);
   }
-
-  dest->original_destination = src.GetOriginalDestination();
 }
 
 }  // namespace blink