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?
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?
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.
Comment on attachment 221460 [details] Patch Clearing flags on attachment: 221460 Committed r162351: <http://trac.webkit.org/changeset/162351>
All reviewed patches have been landed. Closing bug.
(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.