Bug 227795 – >4K Referer should have tailing /

• Component: Page Loading
• Status: RESOLVED FIXED

[Sam Sneddon [:gsnedders]]

Per referrer policy we reduce long referrers to just the origin; however, this should be serialised as per URL which means that it should have a trailing /

This is a relatively big gain on WPT given the large number of referrer policy tests.

[Chris Dumez]

Example of failing WPT test(s)?

[Alex Christensen]

https://wpt.fyi/results/referrer-policy?label=experimental&label=master&aligned

[Sam Sneddon [:gsnedders]]

Specifically something like https://wpt.fyi/results/referrer-policy/4K+1/gen/top.http-rp/no-referrer-when-downgrade/a-tag.http.html?label=experimental&label=master&aligned is the trailing-/ case

[Alex Christensen]

Created attachment 433431 [details]
Patch

[Chris Dumez]

Comment on attachment 433431 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Covered by existing tests and web platform tests we haven't imported yet.

It's too bad that we haven't imported those tests yet.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm:79
> +    checkReferer([NSURL URLWithString:longPath], "http://webkit.org/");

It is a bit confusing because this referrer is not > 4K.

Also, it seems like this should be easily writable as a layout test?

[Alex Christensen]

Comment on attachment 433431 [details]
Patch

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

>> Tools/TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm:79
>> +    checkReferer([NSURL URLWithString:longPath], "http://webkit.org/");
> 
> It is a bit confusing because this referrer is not > 4K.
> 
> Also, it seems like this should be easily writable as a layout test?

This is the trimmed referrer.

[Chris Dumez]

Comment on attachment 433431 [details]
Patch

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

r=me assuming the bots are happy.

>>> Tools/TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm:79
>>> +    checkReferer([NSURL URLWithString:longPath], "http://webkit.org/");
>> 
>> It is a bit confusing because this referrer is not > 4K.
>> 
>> Also, it seems like this should be easily writable as a layout test?
> 
> This is the trimmed referrer.

Oh, I get it now.

I would have liked if we had a layout test for this (as we can run in other browsers and other ports of WebKit). But technically, you have test coverage so r+.

[Alex Christensen]

Comment on attachment 433431 [details]
Patch

The web platform tests have done that for me!

[EWS]

Committed r279886 (239638@main): <https://commits.webkit.org/239638@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433431 [details].

[Radar WebKit Bug Importer]

<rdar://problem/80532911>