Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fetch should compute its origin from its context #12169

Merged
merged 1 commit into from
Mar 31, 2023
Merged

fetch should compute its origin from its context #12169

merged 1 commit into from
Mar 31, 2023

Conversation

youennf
Copy link
Contributor

@youennf youennf commented Mar 30, 2023 •
edited by webkit-early-warning-system

878b8e7

fetch should compute its origin from its context
https://bugs.webkit.org/show_bug.cgi?id=254734
rdar://problem/107414766

Reviewed by Chris Dumez.

In case some security features are disabled, the referrer might not be same origin as the context origin for a fetch request.
In that case, we should still stick to the context origin and not rely on referrer to compute the origin.
We update CachedResourceRequest::updateReferrerAndOriginHeaders accordingly for fetch loads.

* LayoutTests/http/wpt/fetch/origin-no-cors-disabled-expected.txt: Added.
* LayoutTests/http/wpt/fetch/origin-no-cors-disabled.html: Added.
* LayoutTests/http/wpt/fetch/resources/echo-origin-cors-disabled.py: Added.
(main):
* Source/WebCore/loader/cache/CachedResourceRequest.cpp:
(WebCore::CachedResourceRequest::updateReferrerAndOriginHeaders):
* Source/WebCore/page/Page.cpp:
(WebCore::Page::addCORSDisablingPatternForTesting):
* Source/WebCore/page/Page.h:
* Source/WebCore/page/SecurityOrigin.h:
* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::grantUniversalAccess):
(WebCore::Internals::disableCORSForURL):
* Source/WebCore/testing/Internals.h:
* Source/WebCore/testing/Internals.idl:

Canonical link: https://commits.webkit.org/262403@main

a208ea5

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-wk2 ❌ 🧪 api-gtk
✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch-sim

@youennf youennf requested a review from cdumez as a code owner March 30, 2023 11:51
@youennf youennf self-assigned this Mar 30, 2023
@youennf youennf added the Page Loading For bugs in page loading, including handling of network callbacks. label Mar 30, 2023
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 8cf6836)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2
✅ 🧪 webkitperl   🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ❌ 🧪 gtk-wk2
✅ 🛠 tv   🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv-sim   🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 30, 2023
@youennf youennf marked this pull request as draft March 30, 2023 13:29
@youennf youennf changed the title fetch and XHR should compute their origin from their context fetch should compute its origin from its context Mar 30, 2023
@youennf youennf force-pushed the eng/fetch-and-XHR-should-compute-their-origin-from-their-context branch from 8cf6836 to a208ea5 Compare March 30, 2023 13:55
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash a208ea5)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-wk2 ❌ 🧪 api-gtk
✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch-sim

@youennf youennf removed the merging-blocked Applied to prevent a change from being merged label Mar 31, 2023
@youennf youennf marked this pull request as ready for review March 31, 2023 06:16
cdumez
cdumez approved these changes Mar 31, 2023
View reviewed changes
Copy link
Contributor

@cdumez cdumez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@youennf youennf added the merge-queue Applied to send a pull request to merge-queue label Mar 31, 2023
@youennf
fetch should compute its origin from its context
878b8e7 
https://bugs.webkit.org/show_bug.cgi?id=254734
rdar://problem/107414766

Reviewed by Chris Dumez.

In case some security features are disabled, the referrer might not be same origin as the context origin for a fetch request.
In that case, we should still stick to the context origin and not rely on referrer to compute the origin.
We update CachedResourceRequest::updateReferrerAndOriginHeaders accordingly for fetch loads.

* LayoutTests/http/wpt/fetch/origin-no-cors-disabled-expected.txt: Added.
* LayoutTests/http/wpt/fetch/origin-no-cors-disabled.html: Added.
* LayoutTests/http/wpt/fetch/resources/echo-origin-cors-disabled.py: Added.
(main):
* Source/WebCore/loader/cache/CachedResourceRequest.cpp:
(WebCore::CachedResourceRequest::updateReferrerAndOriginHeaders):
* Source/WebCore/page/Page.cpp:
(WebCore::Page::addCORSDisablingPatternForTesting):
* Source/WebCore/page/Page.h:
* Source/WebCore/page/SecurityOrigin.h:
* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::grantUniversalAccess):
(WebCore::Internals::disableCORSForURL):
* Source/WebCore/testing/Internals.h:
* Source/WebCore/testing/Internals.idl:

Canonical link: https://commits.webkit.org/262403@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/fetch-and-XHR-should-compute-their-origin-from-their-context branch from a208ea5 to 878b8e7 Compare March 31, 2023 15:11
@webkit-commit-queue
Copy link
Collaborator

Committed 262403@main (878b8e7): https://commits.webkit.org/262403@main

Reviewed commits have been landed. Closing PR #12169 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 878b8e7 into WebKit:main Mar 31, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdumez cdumez cdumez approved these changes

@achristensen07 achristensen07 Awaiting requested review from achristensen07

Assignees

@youennf youennf

Labels
Page Loading For bugs in page loading, including handling of network callbacks.
Projects
None yet
Milestone
No milestone
5 participants
@youennf @webkit-early-warning-system @webkit-commit-queue @cdumez @webkit-ews-buildbot