Bug 127172 – Never send a non-http(s) referrer header even with a referrer policy

• Component: New Bugs
• Status: RESOLVED FIXED

Never send a non-http(s) referrer header even with a referrer policy

Created attachment 221460 [details]
Patch

Alexey, can you have a look please?

[Alexey Proskuryakov]

Comment on attachment 221460 [details]
Patch

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

It's not cool to have a function with such an ambitious name as shouldHideReferrer, and then have it explained in a header comment what it actually means (that being, do the same thing generateReferrerHeader(ReferrerPolicyDefault), but with a murky restriction of not using the result for Referrer header sending!)

There is only one call site outside SecurityPolicy, and that looks incorrect, as it actually blocks Referer header regardless of policy if shouldHideReferrer said so.

This patch is an improvement, r=me.

> Source/WebCore/page/SecurityPolicy.cpp:75
> +    if (!protocolIs(referrer, "http") && !protocolIs(referrer, "https"))
> +        return String();

This should use url.protocolIsInHTTPFamily().

(In reply to comment #3)
> (From update of attachment 221460 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=221460&action=review
> 
> It's not cool to have a function with such an ambitious name as shouldHideReferrer, and then have it explained in a header comment what it actually means (that being, do the same thing generateReferrerHeader(ReferrerPolicyDefault), but with a murky restriction of not using the result for Referrer header sending!)

Agreed. The ping loader should probably just use generateReferrerHeader()

> 
> There is only one call site outside SecurityPolicy, and that looks incorrect, as it actually blocks Referer header regardless of policy if shouldHideReferrer said so.

Since it's not using the string as referrer header, i'm not sure what to do here. Even if the policy is default, the Ping-From field won't be cleared on a https to http transition in the network stack.

The most reasonable thing to do would be to change the standard to use the referrer header instead of a Ping-From header.


> 
> This patch is an improvement, r=me.
> 
> > Source/WebCore/page/SecurityPolicy.cpp:75
> > +    if (!protocolIs(referrer, "http") && !protocolIs(referrer, "https"))
> > +        return String();
> 
> This should use url.protocolIsInHTTPFamily().

That means converting referrer to a KURL, is that ok with you?

[Alexey Proskuryakov]

Comment on attachment 221460 [details]
Patch

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

>>> Source/WebCore/page/SecurityPolicy.cpp:75
>>> +        return String();
>> 
>> This should use url.protocolIsInHTTPFamily().
> 
> That means converting referrer to a KURL, is that ok with you?

You are right, let's keep it as is then.

[WebKit Commit Bot]

Comment on attachment 221460 [details]
Patch

Clearing flags on attachment: 221460

Committed r162351: <http://trac.webkit.org/changeset/162351>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

(In reply to comment #5)
> (From update of attachment 221460 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=221460&action=review
> 
> >>> Source/WebCore/page/SecurityPolicy.cpp:75
> >>> +        return String();
> >> 
> >> This should use url.protocolIsInHTTPFamily().
> > 
> > That means converting referrer to a KURL, is that ok with you?
> 
> You are right, let's keep it as is then.

We should create a string version of the protocolIsInHTTPFamily function, for the same reason we have a URL version.