Changeset 287353 in webkit

Timestamp:

Dec 22, 2021, 2:13:07 AM (5 years ago)

Author:

youenn@apple.com

Message:

Use requester when calling updateRequestAndAddExtraFields
​https://bugs.webkit.org/show_bug.cgi?id=234507
<rdar://problem/85049490>

Reviewed by Brent Fulgham.

Source/WebCore:

In case of navigation loads, the requester is the fetch request client.
As such, it should be the one used to determine whether the request is cross-origin is not.
Pass the requester to updateRequestAndAddExtraFields to do the correct computation.
Validate that some request fields are now correctly observed in service workers.

Test: http/wpt/service-workers/navigation-iframe-site.https.html

• loader/FrameLoader.cpp:
• loader/FrameLoader.h:
• testing/ServiceWorkerInternals.cpp:
• testing/ServiceWorkerInternals.h:
• testing/ServiceWorkerInternals.idl:

LayoutTests:

• http/wpt/service-workers/navigation-iframe-site-worker.js: Added.
• http/wpt/service-workers/navigation-iframe-site.https-expected.txt: Added.
• http/wpt/service-workers/navigation-iframe-site.https.html: Added.
• http/wpt/service-workers/resources/navigation-iframe-site-frame.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/wpt/service-workers/navigation-iframe-site-worker.js (added)
• LayoutTests/http/wpt/service-workers/navigation-iframe-site.https-expected.txt (added)
• LayoutTests/http/wpt/service-workers/navigation-iframe-site.https.html (added)
• LayoutTests/http/wpt/service-workers/resources/navigation-iframe-site-frame.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/FrameLoader.cpp (modified) (4 diffs)
• Source/WebCore/loader/FrameLoader.h (modified) (1 diff)
• Source/WebCore/testing/ServiceWorkerInternals.cpp (modified) (1 diff)
• Source/WebCore/testing/ServiceWorkerInternals.h (modified) (1 diff)
• Source/WebCore/testing/ServiceWorkerInternals.idl (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-12-22  Youenn Fablet  <youenn@apple.com>
+
+        Use requester when calling updateRequestAndAddExtraFields
+        https://bugs.webkit.org/show_bug.cgi?id=234507
+        <rdar://problem/85049490>
+
+        Reviewed by Brent Fulgham.
+
+        * http/wpt/service-workers/navigation-iframe-site-worker.js: Added.
+        * http/wpt/service-workers/navigation-iframe-site.https-expected.txt: Added.
+        * http/wpt/service-workers/navigation-iframe-site.https.html: Added.
+        * http/wpt/service-workers/resources/navigation-iframe-site-frame.html: Added.
+
 2021-12-22  Arcady Goldmints-Orlov  <agoldmints@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-12-22  Youenn Fablet  <youenn@apple.com>
+
+        Use requester when calling updateRequestAndAddExtraFields
+        https://bugs.webkit.org/show_bug.cgi?id=234507
+        <rdar://problem/85049490>
+
+        Reviewed by Brent Fulgham.
+
+        In case of navigation loads, the requester is the fetch request client.
+        As such, it should be the one used to determine whether the request is cross-origin is not.
+        Pass the requester to updateRequestAndAddExtraFields to do the correct computation.
+        Validate that some request fields are now correctly observed in service workers.
+
+        Test: http/wpt/service-workers/navigation-iframe-site.https.html
+
+        * loader/FrameLoader.cpp:
+        * loader/FrameLoader.h:
+        * testing/ServiceWorkerInternals.cpp:
+        * testing/ServiceWorkerInternals.h:
+        * testing/ServiceWorkerInternals.idl:
+
 2021-12-22  Philippe Normand  <pnormand@igalia.com>
 

trunk/Source/WebCore/loader/FrameLoader.cpp

@@ -1343 +1343 @@
         request.setHTTPReferrer(referrer);
 
-    updateRequestAndAddExtraFields(request, IsMainResource::Yes, newLoadType);
+    updateRequestAndAddExtraFields(request, IsMainResource::Yes, newLoadType, ShouldUpdateAppInitiatedValue::Yes, FrameLoader::IsServiceWorkerNavigationLoad::No, &frameLoadRequest.requester());
 
     ASSERT(newLoadType != FrameLoadType::Same);
@@ -2904 +2904 @@
 }
 
-void FrameLoader::updateRequestAndAddExtraFields(ResourceRequest& request, IsMainResource mainResource, FrameLoadType loadType, ShouldUpdateAppInitiatedValue shouldUpdate, IsServiceWorkerNavigationLoad isServiceWorkerNavigationLoad)
+void FrameLoader::updateRequestAndAddExtraFields(ResourceRequest& request, IsMainResource mainResource, FrameLoadType loadType, ShouldUpdateAppInitiatedValue shouldUpdate, IsServiceWorkerNavigationLoad isServiceWorkerNavigationLoad, Document* initiator)
 {
     ASSERT(isServiceWorkerNavigationLoad == IsServiceWorkerNavigationLoad::No || mainResource != IsMainResource::Yes);
@@ -2924 +2924 @@
 
     if (request.isSameSiteUnspecified()) {
-        auto* initiator = m_frame.document();
-        if (isMainResource) {
-            auto* ownerFrame = m_frame.tree().parent();
-            if (!ownerFrame && m_stateMachine.isDisplayingInitialEmptyDocument())
-                ownerFrame = m_opener.get();
-            if (ownerFrame)
-                initiator = ownerFrame->document();
-            ASSERT(ownerFrame || m_frame.isMainFrame());
+        if (!initiator) {
+            initiator = m_frame.document();
+            if (isMainResource) {
+                auto* ownerFrame = m_frame.tree().parent();
+                if (!ownerFrame && m_stateMachine.isDisplayingInitialEmptyDocument())
+                    ownerFrame = m_opener.get();
+                if (ownerFrame)
+                    initiator = ownerFrame->document();
+                ASSERT(ownerFrame || m_frame.isMainFrame());
+            }
         }
         addSameSiteInfoToRequestIfNeeded(request, initiator);
@@ -3068 +3070 @@
     workingResourceRequest.setHTTPBody(inRequest.httpBody());
     workingResourceRequest.setHTTPContentType(contentType);
-    updateRequestAndAddExtraFields(workingResourceRequest, IsMainResource::Yes, loadType);
+    updateRequestAndAddExtraFields(workingResourceRequest, IsMainResource::Yes, loadType, ShouldUpdateAppInitiatedValue::Yes, FrameLoader::IsServiceWorkerNavigationLoad::No, &request.requester());
 
     if (Document* document = m_frame.document())

trunk/Source/WebCore/loader/FrameLoader.h

@@ -320 +320 @@
     enum class IsServiceWorkerNavigationLoad : bool { No, Yes };
     // For subresource requests the FrameLoadType parameter has no effect and can be skipped.
-    void updateRequestAndAddExtraFields(ResourceRequest&, IsMainResource, FrameLoadType = FrameLoadType::Standard, ShouldUpdateAppInitiatedValue = ShouldUpdateAppInitiatedValue::Yes, IsServiceWorkerNavigationLoad = IsServiceWorkerNavigationLoad::No);
+    void updateRequestAndAddExtraFields(ResourceRequest&, IsMainResource, FrameLoadType = FrameLoadType::Standard, ShouldUpdateAppInitiatedValue = ShouldUpdateAppInitiatedValue::Yes, IsServiceWorkerNavigationLoad = IsServiceWorkerNavigationLoad::No, Document* = nullptr);
 
     void scheduleRefreshIfNeeded(Document&, const String& content, IsMetaRefresh);

trunk/Source/WebCore/testing/ServiceWorkerInternals.cpp

@@ -189 +189 @@
 }
 
+bool ServiceWorkerInternals::fetchEventIsSameSite(FetchEvent& event)
+{
+    return event.request().internalRequest().isSameSite();
+}
+
 } // namespace WebCore
 

trunk/Source/WebCore/testing/ServiceWorkerInternals.h

@@ -70 +70 @@
     RefPtr<PushSubscription> createPushSubscription(const String& endpoint, std::optional<EpochTimeStamp> expirationTime, const ArrayBuffer& serverVAPIDPublicKey, const ArrayBuffer& clientECDHPublicKey, const ArrayBuffer& auth);
 
+    bool fetchEventIsSameSite(FetchEvent&);
+
 private:
     explicit ServiceWorkerInternals(ServiceWorkerIdentifier);

trunk/Source/WebCore/testing/ServiceWorkerInternals.idl

@@ -50 +50 @@
     
     PushSubscription createPushSubscription(USVString endpoint, EpochTimeStamp? expirationTime, ArrayBuffer serverVAPIDPublicKey, ArrayBuffer clientECDHPublicKey, ArrayBuffer auth);
+
+    boolean fetchEventIsSameSite(FetchEvent event);
 };