Bug 202506 - Resource Load Statistics: Downgrade document.referrer for all third-party iframes
Summary: Resource Load Statistics: Downgrade document.referrer for all third-party ifr...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: John Wilander
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-02 18:10 PDT by John Wilander
Modified: 2019-10-03 13:50 PDT (History)
15 users (show)

See Also:


Attachments
Patch (66.13 KB, patch)
2019-10-02 18:16 PDT, John Wilander
no flags Details | Formatted Diff | Diff
Patch (67.31 KB, patch)
2019-10-03 10:37 PDT, John Wilander
no flags Details | Formatted Diff | Diff
Patch (67.31 KB, patch)
2019-10-03 11:14 PDT, John Wilander
no flags Details | Formatted Diff | Diff
Patch for landing (67.31 KB, patch)
2019-10-03 13:10 PDT, John Wilander
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Wilander 2019-10-02 18:10:51 PDT
We should downgrade document.referrer for all third-party iframes to match the downgrade of all HTTP referrers for third-party resources.
Comment 1 John Wilander 2019-10-02 18:11:12 PDT
<rdar://problem/55786397>
Comment 2 John Wilander 2019-10-02 18:16:27 PDT
Created attachment 380074 [details]
Patch
Comment 3 John Wilander 2019-10-03 10:37:56 PDT
Created attachment 380129 [details]
Patch
Comment 4 John Wilander 2019-10-03 10:38:34 PDT
Fixed the one remaining test failure (http/tests/media/media-stream/enumerate-devices-source-id.html).
Comment 5 John Wilander 2019-10-03 11:14:41 PDT
Created attachment 380144 [details]
Patch
Comment 6 John Wilander 2019-10-03 11:15:04 PDT
Fixed style error.
Comment 7 Brent Fulgham 2019-10-03 11:23:52 PDT
Comment on attachment 380144 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380144&action=review

Looks like a good change. It seems like some code could be consolidated.

> Source/WebCore/dom/Document.cpp:4858
> +                return m_referrerOverride;

Could this just call 'Document::downgradeReferrerToRegistrableDomain()'? Or could this code be shared with that method?

> LayoutTests/http/tests/media/media-stream/enumerate-devices-source-id.html:12
> +                internals.setResourceLoadStatisticsEnabled(false);

Lol. So many tests need ITP off. :-)
Comment 8 John Wilander 2019-10-03 12:03:28 PDT
(In reply to Brent Fulgham from comment #7)
> Comment on attachment 380144 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=380144&action=review
> 
> Looks like a good change. It seems like some code could be consolidated.

Thanks!
> > Source/WebCore/dom/Document.cpp:4858
> > +                return m_referrerOverride;
> 
> Could this just call 'Document::downgradeReferrerToRegistrableDomain()'? Or
> could this code be shared with that method?

I thought about that but removing subdomains is more aggressive so I'd like to start with just the origin. downgradeReferrerToRegistrableDomain() is for the ITP 2.3 change after certain navigations whereas this change is across the board.

> > LayoutTests/http/tests/media/media-stream/enumerate-devices-source-id.html:12
> > +                internals.setResourceLoadStatisticsEnabled(false);
> 
> Lol. So many tests need ITP off. :-)

Yes. 🙄 We should remember to file an issue with WHATWG and/or W3C to allow downgraded referrers by default in web platform tests.
Comment 9 John Wilander 2019-10-03 13:10:58 PDT
Created attachment 380155 [details]
Patch for landing
Comment 10 WebKit Commit Bot 2019-10-03 13:50:42 PDT
Comment on attachment 380155 [details]
Patch for landing

Clearing flags on attachment: 380155

Committed r250676: <https://trac.webkit.org/changeset/250676>
Comment 11 WebKit Commit Bot 2019-10-03 13:50:43 PDT
All reviewed patches have been landed.  Closing bug.