Implement referrer policy delivery via header

[rebstar6]

See https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-header for details.

Reference #10311 for servo referrer policy implementation thus far; tests generated in web-platform-tests/wpt#3118.

[jdm]

This entails checking for the header in ScriptThread::load and passing its value to the Document constructor, so that it can set the document's referrer policy to the desired value during at its construction.

[aravind-pg]

I'd like to try taking this on -- hopefully it's not too unsuitable for a beginner. Here's my understanding so far, to make things extremely concrete:

• I see that there's already a Cell<Option<ReferrerPolicy>> field in the dom::Document struct, currently set by default to NoReferrer. I need to modify the constructor(s) (both new and new_inherited, I think, from the way the code is organized?) to accept an Option<ReferrerPolicy> parameter and set this field using that.
  • This will require updating calls to Document::new everywhere. Is that OK? Or should I create a new constructor, or a setter or something?
• In ScriptThread::Load, I need to check if there's a referrer policy header that's part of metadata.headers.
  • Presumably I should be doing this using hyper::header::Headers's nice strongly-typed has and get.
  • This will involve creating a ReferrerPolicyHeader struct implementing hyper::header::Header + HeaderFormat, given that none exists in Hyper right now.
    • How complicated is this?
    • Where should it live, and within Servo or Hyper?
    • This seems good for a smallish standalone PR before the main one.
• If there is indeed such a header, extract the embedded referrer policy and pass it to Document::new.
• Anything further? Because this doesn't yet actually do anything, just populates the field in the struct. Maybe there's already code that looks at this field, and now it'll actually start doing something useful.

Sorry for the wall of text! 😛

[jdm]

Looks like you've got a great handle on the work required here! You're welcome to add the header to hyper and use a cargo override to allow you to keep making progress in Servo while the changes aren't merged to hyper's master branch. Creating new headers are often marked as good first issues for new contributors in hyper; there are lots of examples you can crib from there.

There is lots of code that already looks at the field, so once you start updating it the tests under tests/wpt/web-platform-tests/referrer-policy/ that live under http-rp should start passing. ./mach test-wpt tests/wpt/web-platform-tests/referrer-policy/ is what you'll be looking for there.

[jdm]

And yes, updating existing callers of the Document constructor is fine.

[aravind-pg]

That's great to hear! Thanks a lot :)

[aravind-pg]

A question: if the referrer_value field is Cell::new(Some(ReferrerPolicy::NoReferrer)) by default, i.e. its default value is NoReferrer, then why have the Option<ReferrerPolicy> at all, instead of just ReferrerPolicy? Doesn't look like it's ever None right now. Alternatively maybe we should have None be the default instead of Some(NoReferrer)?

[jdm]

You're right, there doesn't seem to be a reason to store an Option value.

[aravind-pg]

Actually, looking into it a bit more, there definitely is stuff in the spec addressing this issue of the default value. According to sections 3.2 and 7.3, the default should be no-referrer-when-downgrade. And it does look like that's the way we do it in the fetch algorithm right now.

In this context the Option does seem useful in order to distinguish the case of no referrer policy being specified at all (None) from the default one being specified (Some(NoRefWhenDowngrade)). Accordingly the default value in the Document constructor looks like it should be None. @rebstar6, can you comment? Is there some reason the constructor defaults to NoReferrer right now?

[rebstar6]

Yep, it's an option to distinguish between an unset referrer policy (None), and one that has been explicitly set via meta elements or headers or otherwise.

The default should be None in the long run (which is translated to no ref when downgrade in the determine_request_referrer method). It is set to Some(no-referrer) in the short term because we have only semi-implemented referrer policy delivery (this is what the TODOs in the Document code are for). Basically it's safer - if a document should have a more strict referrer policy (like no-referrer), but that was communicated in the header, we wouldn't have picked it up since we only implemented meta, and as a result would be sending info inappropriately...better to not send than send too much. Once the other delivery methods were implemented (read: once the issues you are working on were implemented), the plan was to make the default None as it should be.

Long story short, feel free to change to None when you deem appropriate, and at that point you can also un-ignore the unset-referrer-policy tests, which are currently not being run for this reason.

Let me know if this clears it up :)

[aravind-pg]

@rebstar6 OK, that makes sense, thank you :) I wouldn't have thought we needed to be so conservative on that front but why not.