| Summary: | Fix about:blank document.referrer initialization | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Dominic Farolino <domfarolino> |
| Component: | Frames | Assignee: | sideshowbarker <mike> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | ahmad.saleem792, cdumez, karlcow, mike, webkit-bug-importer |
| Priority: | P2 | Keywords: | BrowserCompat, InRadar, WPTImpact |
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| URL: | http://wpt.live/html/browsers/windows/browsing-context.html | ||
|
Description
Dominic Farolino
2022-07-20 16:15:17 PDT
We are failing the sub-test added by Blink commit. Added 'WPTImpact' tag and also test URL for quick testing. Howdy Dom, Do you have a spec link handy? (To the part of the spec which states the requirement that document.referrer must be the full URL) Some links referenced in Blink's commit: [1]: https://html.spec.whatwg.org/multipage/dom.html#dom-document-referrer and https://html.spec.whatwg.org/multipage/dom.html#the-document's-referrer [2]: https://html.spec.whatwg.org/multipage/browsers.html#creating-a-new-browsing-context [3]: https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigating-across-documents:the-document's-referrer-2 ___ Full Commit message: The HTML Standard assigns `document.referrer` [1] in two ways: 1.) During Document/browsing context creation 2.) Upon navigation to a new Document For creation (1): Per the HTML Standard's "create a new browsing context" algorithm [2] step 3, all new browsing contexts with a non-null creator (always same-BCG) have their "creator URL" set to the creator document's full URL, unredacted and subjected to no referrer policy sanitization. Later in step 20, we set the referrer of the newly-created document to the newly-created browsing context's creator URL. For navigation (2): In "create and initialize a Document object", with the referrer from "navigation params" [3] ---- In Blink, we handle `document.referrer` assignment the same for both creation and navigation; we simply set `DocumentLoader::referrer_` to what the resulting `WebNavigationParams` carries [4], and we don't do anything special for the browsing context creation case. This leads to the final `document.referrer` on Documents being created in new BCs actually being incorrectly subjected to referrer policy sanitization/redaction. Due to existing issues, Chrome does not perfectly follow the HTML Standard for creation of new browsing contexts. In short, for iframes we create a new browsing context/document [5] and immediately navigate it to about:blank [6]. This navigation commits synchronously via a special path that tries to make up for Blink's mismatch with the spec [7]. During this synchronous about:blank navigation, the request's referrer is computed as "" (in FrameLoadRequest::ctor), because the origin of the "about:blank" URL is opaque and therefore cross-origin with the embedder, and the default referrer policy is `strict-origin-when-cross-origin`. For newly created windows (with openers, since this is only relevant for windows with "creators"), something very similar happens. We create a FrameLoadRequest whose referrer gets set to "" (in FrameLoadRequest::ctor as well as [8]), and we perform the special-case synchronous navigation which ends up with the resulting document having an empty referrer, just like in the iframe case. ---- This CL fixes the above issues by modifying `RenderFrameImpl::SynchronouslyCommitAboutBlankForBug778318()` to find the initiator frame (should be equivalent to the spec's "creator" Document), if it is local (should be the case for about:blank subframes and new windows w/o "noopener"), and sets the navigation params's referrer member to the initiator Document's full unredacted URL. This will be used to set the eventual `DocumentLoader::referrer_` member when the synchronous navigation commits, which will be used to reflect the correct value to `document.referrer`. In cases where we don't go through Blink's special sync navigation case, `document.referrer` is unchanged. ---- [1]: https://html.spec.whatwg.org/multipage/dom.html#dom-document-referrer and https://html.spec.whatwg.org/multipage/dom.html#the-document's-referrer [2]: https://html.spec.whatwg.org/multipage/browsers.html#creating-a-new-browsing-context [3]: https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigating-across-documents:the-document's-referrer-2 [4]: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/loader/document_loader.cc;l=322;drc=5dffcc4991ab32edc7dbc99c3b4685882c029ab0 [5]: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_frame_owner_element.cc;l=827-828;drc=6fd9edfbb85bc5fb2ed8f9300abe83cd03ab54b7 [6]: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_frame_owner_element.cc;l=862-863;drc=6fd9edfbb85bc5fb2ed8f9300abe83cd03ab54b7 [7]: https://source.chromium.org/chromium/chromium/src/+/main:content/renderer/render_frame_impl.h;l=929-953;drc=5575ddb21068841ba19e1ea957b4fdb5adcd0250 [8]: https://source.chromium.org/chromium/chromium/src/+/main:content/renderer/render_view_impl.cc;l=290-292;drc=5dffcc4991ab32edc7dbc99c3b4685882c029ab0 Ahmad — Thanks much 👍 So for the record here, after looking through the spec links that Ahmad posted in https://bugs.webkit.org/show_bug.cgi?id=242965#c4 (thanks again), it seems the specific spec requirement that’s relevant to this issue is Step 16 of the “To create a new browsing context and document” algorithm https://html.spec.whatwg.org/multipage/document-sequences.html#creating-a-new-browsing-context — which says: > Set document's referrer to the serialization of creator's URL. https://bugzilla.mozilla.org/show_bug.cgi?id=1780481 is the related Gecko bug Pull request: https://github.com/WebKit/WebKit/pull/20158 Committed 273830@main (fd0640d8536f): <https://commits.webkit.org/273830@main> Reviewed commits have been landed. Closing PR #20158 and removing active labels. |