Servo: Implement referrer policy

[paulrouget]

See https://w3c.github.io/webappsec-referrer-policy/

Necessary for https://github.com/servo/servo/issues/10309#issuecomment-203929226

[jdm]

Tests: tests/wpt/web-platform-tests/referrer-policy/

[jdm]

The goal of this specification is to allow webpages to control the behaviour of the Referer header (sic) when network requests are initiated.

The basic pieces here are the following:

• create an enum representing a referrer policy in net_traits/lib.rs
• introduce a place where referrer policies are stored for global environments (ie. Document and WorkerGlobalScope, in document.rs and workerglobalscope.rs respectively)
• propagate this policy to the actual HTTP network request code in http_loader.rs and modify the Referer header accordingly (via document_loader.rs, fetch the policy from the global environment)
• introduce per-element attributes for controlling the associated requests, ensure those get propagated accordingly (eg. htmlscriptelement.rs, htmliframeelement.rs, etc.)
• integrate with the Fetch implementation (components/net/fetch/methods.rs) and write unit tests (tests/unit/net/fetch.rs)
• support setting the initial global policy via the Referrer-Policy HTTP header in the initial network response for a worker/document (in http_loader.rs, add a new field to the Metadata type and extract that when creating the new Document in script_thread.rs)

[rebstar6]

I'm going to work on this issue!

[jdm]

I've added some file pointers to my previous comment. Let us know if anything's unclear!

[rebstar6]

I'm a little confused on scope - the spec refers to the "settings object" as having a referrer policy - is this specific to the Document? What do you mean above by "global environment"?

Also, I've worked with the document file a little, but not workerglobalscope - can you give a quick explanation of what that file is and how that and document interact (if they interact).

[jdm]

A "global environment" refers to the root object for executing JavaScript code. You can think of this as a per-tab global object. The "settings object" is a concept that we don't have an equivalent for in Servo yet, so we'll treat it as "data that resides in the global object". WorkerGlobalScope is the global object for a web worker; you should feel free to ignore workers at first and focus on how the specification applies to ordinary documents.

[rebstar6]

On the policy propagation piece - I'm not totally clear what has access to what.

Looking at the files, what seems to be happening is that when you click on a link that should load a new page, the document_loader 'prepare_async_load' method gets called, and that, in theory, is where things can be passed between the document and the http_loader methods (most notably, load). If I have a document with some referrer policy, the load method would need both the current document's referrer policy and the current document's URL (and maybe some other stuff) in order to properly set the referer header. Right now, load doesn't have any access to the current doc(?). Is this right?

[jdm]

That is correct; the desired referrer policy would need to be passed as part of the LoadData. Passing it to the PendingAsyncLoad constructor via the methods in Document that invoke the DocumentLoader code would make sense to me.

[rebstar6]

But beyond that, if I just have the referrer policy, I still cant set the header because I also need the previous document's URL to populate it - the loaddata currently just has the url to be loaded, right?

[jdm]

Yep. We should probably add that to LoadData as well!

[rebstar6]

Alright, so just wanna confirm I'm on the right track here: https://github.com/rebstar6/servo/commit/8c91f8abad39debdf77228669c94969865739fc0

I added Option for referrer policy and referrer URL to both PendingAsyncLoad and LoadData. From document, they get passed 1st to PendingAsyncLoad which passes them forward to LoadData (where they will be picked up by the http_loader when setting headers). With the exception of the prepare_async_load/load_async methods, these are None.

Just to confirm - document.rs has both prepare_async_load and load_async, each of which call their respective methods in document_loader...but the document_loader's load_async calls prepare_async_load. Is the prepare getting called twice? What is the point of prepare_async_load within the document class?

[jdm]

Yes, that looks like it's on the right track. The LoadData constructors in XMLHttpRequest and ScriptThread::start_page_load should have non-None values, but those can be dealt with later as long as there's a TODO marking them.

As for the second question, prepare doesn't get called twice for the same value. The difference between prepare_async_load and async_load is that the former returns a structure that encapsulates a network request that can be started at some point in the future, while the second initiates a network request immediately.

[rebstar6]

Alright cool. Also, where does fetch come into play (and what is it, exactly?)?

The next thing I was going to try was to set the referer header within modify_request_headers based on the policy and urls. However, unsure if this should actually go into the fetch code instead (or both if theyre separate things)?

[jdm]

The code in components/net/fetch/ implements the Fetch specification and is designed to replace the ad-hoc network implementation that currently exists in stuff like http_loader.rs. It's only used in unit tests so far; we're going to switch over to using it after it's more complete. So yes, doing it in both would be most correct (and you'll need to add similar referrer data to Request as you did LoadData, since they don't overlap).

[rebstar6]

@SimonSapin - I mentioned this in IRC but I don't think you were on then. At some point for this issue, I will need to strip the username/password, fragment, and in some cases path and query from a URL (see https://w3c.github.io/webappsec-referrer-policy/#strip-url). I was told that you wrote (and are working on) rust-url. Do you have any suggestions about the best way to approach this? Are there changes to the crate that might make some of this easier? Any help appreciated!

[rebstar6]

@jdm - I've noticed that there are a bunch of requests on a page load where http_loader is capturing the referrer_policy/url. However, some are excluded (and tests fail as a result). For example, I added prints in http_loader for referrer_url and the url being loaded. if I do ./mach run https://www.wikipedia.org/ I get some assets receiving the referrer, and some not. Right now, I have no policy logic in place - every request should have the referrer added if there is one:

URL "https://www.wikipedia.org/"
REFERRER UNKNOWN
URL "https://www.wikipedia.org/portal/wikipedia.org/assets/js/abtesting-c8078be6c2.js"
REFERRER: "https://www.wikipedia.org/"
URL "https://www.wikipedia.org/portal/wikipedia.org/assets/img/Wikipedia_wordmark.png"
REFERRER UNKNOWN
URL "https://www.wikipedia.org/portal/wikipedia.org/assets/img/Wikipedia-logo-v2.png"
REFERRER UNKNOWN
URL "https://www.wikipedia.org/portal/wikipedia.org/assets/img/sprite-bookshelf_icons.png?18d8b7f58860758a154b3f1b0d329784d0f4235a"
REFERRER UNKNOWN
URL "https://www.wikipedia.org/portal/wikipedia.org/assets/js/index-1d96a05608.js"
REFERRER: "https://www.wikipedia.org/"
(... and some more, mixed with/without referrer)

Then note, if I click the 'English' link, the next URL to load has no referrer, which I think may be the cause of failing tests for me:

URL "https://en.wikipedia.org/"
REFERRER UNKNOWN
URL "https://en.wikipedia.org/wiki/Main_Page"
REFERRER UNKNOWN
URL "https://en.wikipedia.org/w/load.php?debug=false&lang=en&modules=ext.gadget.DRN-wizard%2CReferenceTooltips%2Ccharinsert%2Cfeatured-articles-links%2CrefToolbar%2Cswitcher%2Cteahouse%7Cext.tmh.thumbnail.styles%7Cext.uls.nojs%7Cext.visualEditor.desktopArticleTarget.noscript%7Cext.wikimediaBadges%7Cmediawiki.legacy.commonPrint%2Cshared%7Cmediawiki.raggett%2CsectionAnchor%7Cmediawiki.skinning.interface%7Cskins.vector.styles&only=styles&skin=vector"
REFERRER: "https://en.wikipedia.org/wiki/Main_Page"
URL "https://en.wikipedia.org/w/load.php?debug=false&lang=en&modules=site&only=styles&skin=vector"
REFERRER: "https://en.wikipedia.org/wiki/Main_Page"
(and more again, assets within the english wiki page)

The point of this is, the referrer_url is definitely not making the transition into http_loader in all cases after this logic is in place: https://github.com/rebstar6/servo/commit/d8c06b40faa576726b405d9c2036f08b13238df6 . Any idea what's failing here? Is this maybe related to the ScriptThread::start_page_load you mentioned above?

[jdm]

The image loader code isn't providing a referrer because the current design of the image cache makes it impossible to do so correctly. We can ignore any failures to do with images, accordingly. The failure for the top-level Document is start_page_load, as you correctly intuited.

[SimonSapin]

@rebstar6 Yes I’m the main author and maintainer of rust-url. You can find the API documentation for the Url struct in the version of rust-url used by Servo at https://servo.github.io/rust-url/url/struct.Url.html

As you can see there, each component is stored separately. You can reset them with code like:

if let Some(relative) = url.relative_scheme_data_mut() {
    // String::clear sets the length of a string to zero, effectively removing its content.
    relative.username.clear();
    relative.password = None;
}
url.fragment = None;

I also have PR #9840 that updates Servo to a version of rust-url where the Url struct is completely different. But a number of thing still need to happen before it can be merged, so maybe your PR will be merged first and you won’t have to worry about it. If #9840 _is_ merged first you’ll have to adapt your PR to the new API. In that API the Url struct will then have a single String internally but as a private field. To manipulate it, go through the setter methods:

url.set_fragment(None);
let _ = url.set_password(None);
let _ = url.set_username("");

Since strings are contiguous in memory, touching something in the middle that changes the length mean the rest of the string afterwards needs to be moved. For that reason it’s slightly more efficient to remove components at the _end_ of the URL first.

let _ = is used to ignore the returned Result<(), ()>. This result can signal an error since some URLs like data:text/html,<p>Example can’t have a username or password, but since you were trying to remove them anyway that’s OK. (This is by the way when the previous code would not go into the if let block.)

Feel free to ask more questions here or on IRC. I’m in the central Europe time zone, but if you mention my nickname on IRC while I’m away I’ll still see it later and can answer then (or maybe someone else can answer), so leave your IRC client online if you can.

[rebstar6]

@jdm a few more questions...

1. Everything in tests/wpt/web-platform-tests/referrer-policy/ seems to be testing img, iframe, script tags, xhr-requests (not, or fetch. I'm not seeing anything that tests the basic case (I'm on a page and click a link). Is this group of tests just geared towards these cases for some reason? Is there anything elsewhere I can use (she asks hopefully)?
2. I followed the script methods backwards as best I could, and settled on load_url in window.rs as the place that the referrer info can be passed along (load_url function - see https://github.com/rebstar6/servo/commit/f48fff9245f24c338bcfa772d7fb0515591b9c41 if curious). This seems to work for my wikipedia case above. Is it too broad though (read: does window.load_url() get called in cases where we may not want that referrer info sent, like if you type a url into the browser? With my change, it will always send the referrer info for whatever that window's current document is)

[jdm]

1) Unfortunately, since the test harness must be loaded in the top level document, any code that tests navigation must rely on an iframe so the tested document doesn't replace the test harness. That being said, it may be possible to test some cases by having the framed document call a method (eg. parent.navigation_complete(document.referrer)) instead of using postMessage, which we don't support.
2) That's an interesting question. I'll have to get back to you on that.

[rebstar6]

Gotcha. I'm definitely to the point where having some tests to code from would be helpful. If you could give me a little more guidance on how to edit what we have (or create new?), that would be great.

I think I have the Referer header setting piece mostly in place (read: http-loader will set the referer header given the some policy and url provided by the calling document). This assumes some policy given though, I'm not yet pulling the policy from anywhere. I could, at a minimum, write some unit tests for this to verify it, but those will be moot if we can get the full html tests working.

Also, thinking in terms of eventual review, is there any way you want me split this up for PRs? I don't want it to get too big and stuck in review. (code at https://github.com/rebstar6/servo/tree/referrerPolicy)

[jdm]

Whoops, didn't get back to you earlier. Window.load_URL looks like a fine choke point.

[jdm]

In terms of tests, we can't write unit tests for network code that interacts with code in script, so the best we can do there is write tests for the network code given LoadData with certain characteristics. Those would be tests in unit/net/http_loader.rs that verify that the expected Referer header is present in the request that would be sent to the server. For those iframe WPT tests, I'll take a look and see if there's a way to copy and rewrite some of them to not timeout.

[jdm]

As for reviewing, if you'd like to split it up like:

• enabling relevant tests with lots of initial failures
• data types, fields for storing relevant data
• passing around referrer policy and referrer values, storing appropriately
• setting referrer based on policy, updating expected test results

That should make each part straightforward to review.

[rebstar6]

So I guess the question is, is there any point in writing some unit tests and PR-ing what I have now so I can get an early review on it?

That would include:

• passing policy from document to httploader (think of it as 1-way. The policy will still have to flow backwards, as it can be set via a header)
• setting referrer based on policy
• Note: the policy being passed is set to 'no-referrer' for all document's, so this code should have no impact in production, but is in place for later

The unit tests, as you suggested, would actually validate what I have so far - given a policy, does http-loader set the right header?. The iframe tests that we want to edit become necessary only in the next piece, which is setting the policy as delivered by: https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-delivery

[jdm]

Sounds like a PR including unit tests would make sense, given the current policy settings :)

[jdm]

@rebstar6 Just to be sure, are you continuing to work on this, or is your project period complete?

[rebstar6]

Technically the project is over, but I'm gonna still work on it - the next week is exam time, so likely no progress, but after May 4 I plan to jump back into it. Let me know if that's ok!

[jdm]

Totally fine 💃

[rebstar6]

Alright I'm back! The next thing to do here is referrer policy delivery - https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-delivery. I'll probably start with one, so each will be its own PR (maybe meta tag? seems most important for https://github.com/servo/servo/issues/10309#issuecomment-203929226).
At this point, it becomes important that I have some way to test this. I assume the tests still can't be run due to other non-implemented items. Is there a good way to validate what I'm doing here?

[jdm]

I suspect it's only the iframe tests that are missing fundamental pieces right now, so I think it should be possible to rely on the attr-referrer/ and meta-referrer/ tests for the referrerpolicy, noreferrer and meta delivery mechanisms respectively (excluding iframes, obviously). We don't have any support for CSP yet, so you can ignore those.

[jdm]

Oh, foo, looks like the tests for <link> and <a> also rely on postMessage, as does <script>. That means that <img>, XMLHttpRequest, and Worker tests should work (although I'm having trouble finding any instances of the Worker ones so far). Anyways, if you focus on supporting the first two, we can figure out what to do about postMessage.

[rebstar6]

img is still giving me issues. try:
./mach test-wpt tests/wpt/web-platform-tests/referrer-policy/no-referrer/meta-referrer/same-origin/http-https/img-tag/
I'm getting timeouts with InvalidStateError.
This is no-referrer, which should in theory be 'working' (as in, nothing is sent).

[jdm]

Ok, I figured out the img issue. Any tests that involve https will fail in Servo right now, so ignore any of the directories with http-https or https-http. The http-http ones should work, and appear to do so in my local testing. Additionally, I investigated the postMessage thing over the weekend and came up with https://github.com/jdm/servo/commit/afc7f7b18942bd6921135848291a11a11b2028a7 . This commit allows the iframe tests to pass, but is highly unlikely to be allowed to merge as-written. It should enable you to at least test the code you're writing while we come up with a more maintainable replacement, though!

[rebstar6]

Cool, started my branch off of that one. Should be helpful. 2 more testing things -
I started trying to include/exclude the appropriate tests. The include.ini gets nuts when I use that approach (see gist - right now this is just 2 policies, img only. Adding the other 3 policies will be even more). Is there a better way to do this? (see https://gist.github.com/rebstar6/4026c0574e976179c8046a9042556e40)

Also, any idea why these tests only seem to have http-http and http-https? Specifically, we almost definitely want a test for https to http for the 'no referrer when downgrade' policy, since the point of that is to not send the referrer when you 'downgrade' (ie, go https to http).

I suppose if https wont work now anyway, that doesnt matter in the short term, but calling it out for later.

[jdm]

I recommend using skip: false on the toplevel directory, then adding __dir__.ini files to disable subdirectories as necessary. That's a good question about the tests; I'll raise it upstream.

[jdm]

Filed https://github.com/w3c/web-platform-tests/issues/2968.

[rebstar6]

Thanks! And sure, went the directory disabling route. Because of all the file nesting, were still talking a bunch of files, but oh well - https://github.com/rebstar6/servo/commit/21104c433a77d33a8340f5607c48090b4c73dc40

Now I guess I should actually get things to work.

[rebstar6]

Hrm, started, but I kinda hate this direction, so polling for suggestions. I'm just going to go ahead and point out all the issues with my approach so far (see https://github.com/rebstar6/servo/commit/e82efa0fc407220811d8770715d8297ad9fad86d)

This doesn't compile, and even if it did, things are super inefficient. I'm trying to implement this logic., ideally using methods we have already. So I start off pulling all the meta referrer elements, though that's silly (and maybe slow?) because we really only want the ones that are children of the head element. Then from there, I want to iterate though them, but I keep getting stuck in rooted/js elements and im not sure how to get out of that. As it stands, it gives the error:
"error: Expression of type std::vec::Vecdom::bindings::js::JS must be rooted, #[deny(unrooted_must_root)] on by default"
...though if not mut I can't call pop(). Sigh.

Also, ideally the policy is set the 1st time get_referrer_policy is called and never again (is that even possible?). If not, it's probably best on document creation so we aren't re-running the method on each link out. Until the document is created though, I dont have use of the document methods I'm using here, which is why it lives where it lives for now.

Thoughts? Suggestions?

[jdm]

I'd probably recommend implementing the bind_to_tree method for HTMLMetaElement and making it call a method on its owning document with the associated referrer policy that it contains, if applicable. This turns it into a push notification at creation, rather than making the document poll. I'm assuming that we don't need to care about updates to the attribute later, but if we did we could use after_set_attr for that in a similar fashion.

[rebstar6]

Striking out today: see https://pastebin.mozilla.org/8870552
Basically, this is what I'm printing when running tests/wpt/web-platform-tests/referrer-policy/origin-only/meta-referrer/same-origin/http-http/iframe-tag/generic.no-redirect.http.html

Looks like there are a few requests happening in this test. Most of them do have the referer getting set. But for the one we care about (lines 27-31), http_loader::load seems to be getting called with None/None for referrer url/policy. I have to assume there’s another load_data somewhere that's doing this. This is an iframe test - any ideas?

[rebstar6]

Actually...does an iframe call not go through window::load_url? Would make sense. That may be the issue as that was the pass-through point

[jdm]

Oh yeah, http://mxr.mozilla.org/servo/source/components/compositing/constellation.rs#1006 inside handle_script_loaded_url_in_iframe_msg is something I explicitly mentioned in the original PR :)

[jdm]

Sorry, I'm confusing myself - http://mxr.mozilla.org/servo/source/components/script/dom/htmliframeelement.rs#151 is the proper one.

[rebstar6]

Yep, there's that TODO. Alright, welp.

[jdm]

Since that now lives in code that has direct access to a document, it should be straightforward to fix, at least!

[rebstar6]

Yeah much better, got myself a referer header!

[rebstar6]

One more test issue, this one should be easier:

According to https://w3c.github.io/webappsec-referrer-policy/#determine-policy-for-token, the origin when cross origin string is "origin-when-cross-origin". The tests use "origin-when-crossorigin", so this policy fails.

Can I change the tests, or is this another issue for w3c?

[jdm]

According to the mailing list, this was a typo in the old specification that shipped in some versions of Chrome. Let's modify the tests locally and file an issue upsteam about it as well. Would you like to do that?

[jdm]

Actually it looks like the Firefox developers are making the same modifications, so I guess we can race them and not bother with the upstream issue: https://bugzilla.mozilla.org/show_bug.cgi?id=1270050

[rebstar6]

Wow, it is somewhat unbelievable how you keep track of all these things.

A race it is!

[rebstar6]

Ok, all of the iframe/http-http tests are working for meta-referrer except:
tests/wpt/web-platform-tests/referrer-policy/origin-when-cross-origin/meta-referrer/cross-origin/http-http/iframe-tag/cross-origin.swap-origin-redirect.http.html

Honestly, I can't figure out why the expected output is what it is, and the spec page seems to have changed and now has barely anything on it about redirects (https://w3c.github.io/webappsec-referrer-policy/)

If I'm following it right, this test goes from:
http://web-platform.test:8000/somepath
to
http://www1.web-platform.test:8000/somepath
which redirects to
http://web-platform.test:8000/somepath

Given that, with origin-when-cross-origin (ie origin-only for a cross-origin request, full url for a same-origin request), I can see an argument for referer = {full 1st link} as it is not 'cross-origin' for the final page, or referer = "http://www1.web-platform.test:8000/" if you count the intermediate step as the referer.

The expected output is "http://web-platform.test:8000/" (origin of the 1st page)...neither of the above. Is there anywhere to get more details on the spec and/or why this test expects what it does?

[jdm]

Yes, the relevant specification bits here are https://fetch.spec.whatwg.org/ (which describes the behaviour for redirections and any modifications of referrer policies that are required) and the "Otherwise" part of step 13 of the navigate algorithm from the HTML specification.

[jdm]

Actually, the fetch specification isn't too important here, since it mainly just dictates when to obtain the referrer policy and when to return a response value. In the case of navigation, the fetch algorithm doesn't follow the redirection transparently (because the request redirect mode is "manual"), so the HTML specification starts the navigation algorithm over again in step 17. I assume this is why the referrer is expected to remain as the original document that initiated the navigation...

[rebstar6]

Yeah, being the original document makes sense. In that case though, I still don't get why it's just sending the origin, not the full url, since it's then no longer is a cross-origin request.

[rebstar6]

Seems like there was conversation related to this when redirect tests were added. What I'm getting from this conclusion is that the referrer header value is stripped on the 1st request, and remains as such on the 2nd. I guess kind of like a funnel, it can get filtered down with successive requests, but not built back up.

This seems like a lot of assuming from one comment on an issue a year ago though.

[jdm]

That does make sense to me. I would proceed under that assumption and I'll ask around.

[jdm]

I filed https://github.com/whatwg/html/issues/1234 to clarify this issue.

[rebstar6]

Thanks - updated to funnel approach in https://github.com/rebstar6/servo/commit/cec1fde03ac2ad8861d0537e0cccf78ef33a7767
This should be it for meta-referrer support (!). I would PR now, but the code/tests in my branch rely on your iframe commit, so not sure what the best approach is there.

[jdm]

I propose the following:

• duplicate the iframe tests and referrer-policy test harness inside tests/wpt/mozilla/tests/mozilla and apply this diff to the resulting files. This allows them to run without requiring postMessage.
• make a PR with your changes and the additional duplicated tests
• we extract the straightforward parts of my branch (ie. the implementation of Window.postMessage) and make a PR; this should allow running the script-tag tests.

[jdm]

To braindump after looking at the infrastructure for other tests:

• img-tag won't be meaningful until #7708 is fixed
• iframe-tag needs either cross-origin postMessage or https://gist.github.com/jdm/5da497f6da4ac3d12bf824675d2f458f
• xhr-request should Just Work (modulo missing referrer policy hooks in xmlhttprequest.rs)
• fetch-request requires the JS Fetch API
• script-tag requires same-origin Window.postMessage

[rebstar6]

xhr DOES just work! Made that update - it's in there now. I'll make the new iframe tests, and PR with those plus xhr.

Woot!

[jdm]

Filed #11161 for the script-tag related pieces.

[rebstar6]

Alright, added the new tests - everything is here: https://github.com/rebstar6/servo/commits/refPol2
I'm still getting some errors after implementing your diff above. 1st error was no document.referrer method, so I added the shell for that back, and now it's complaining that parent.postMessage is not a function.
Is it possible the sourcing is messed up for the new tests at tests/wpt/mozilla/tests/mozilla/referrer-policy/? I feel like the errors I'm seeing should be addressed by the changes you made.

[jdm]

Oh, you'll need to implement the referrer attribute on Document like I did in my original postmessage branch. And yes, my diff was modifying the original files under web-platform-tests/referrer-policy; it should be applied to your duplicates under mozilla/ instead.

[rebstar6]

I think that's what I did though -
This commit adds referrer: https://github.com/rebstar6/servo/commit/35454e8dec57a198d403fb5bfd952501d53cee1c
This commit adds the tests in mozilla/ with your changes:
https://github.com/rebstar6/servo/commit/e810e4148afcd451016f4e38ba8d8ebbf0dc1e77

With those, the error I'm getting is:

ERROR:js::rust: Error at http://www1.web-platform.test:8000/referrer-policy/generic/subresource/document.py:12:7: parent.postMessage is not a function

[jdm]

Ugh. Every URL in those tests and scripts that refers to /referrer-policy/ (note leading /) is resolving to the files in web-platform-tests/referrer-policy/, rather than mozilla/tests/mozilla/referrer-policy/. :(

[jdm]

It might be possible to run a find-and-replace operation on that directory to replace "/referrer-policy with "/_mozilla/mozilla/referrer-policy.

[rebstar6]

Still not working :/
New error message though! Unfortunately generic and hard to decipher.
129:130: syntax error: Expected end of line but found unknown token. (-2741)

[jdm]

I've seen that every time I've run the tests and it hasn't impacted the actual test results.

[rebstar6]

For me it's not running them though - I get a timeout

On Tuesday, May 17, 2016, Josh Matthews [email protected] wrote:

I've seen that every time I've run the tests and it hasn't impacted the
actual test results.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/servo/servo/issues/10311#issuecomment-219742094

[jdm]

Looks like a replacement was missed due to single quotes vs double quotes:

diff --git a/tests/wpt/mozilla/tests/mozilla/referrer-policy/generic/common.js b/tests/wpt/mozilla/tests/mozilla/referrer-policy/generic/common.js
index 42aaed5..a24e90c 100644
--- a/tests/wpt/mozilla/tests/mozilla/referrer-policy/generic/common.js
+++ b/tests/wpt/mozilla/tests/mozilla/referrer-policy/generic/common.js
@@ -111,7 +111,7 @@ function queryIframe(url, callback, referrer_policy) {
     var iframe = appendIframeToBody(url + "&id=" + id, referrer_policy);
     iframe.addEventListener("load", function listener() {
       var xhr = new XMLHttpRequest();
-      xhr.open('GET', '/referrer-policy/generic/subresource/stash.py?id=' + id, true);
+      xhr.open('GET', '/_mozilla/mozilla/referrer-policy/generic/subresource/stash.py?id=' + id, true);
       xhr.onreadystatechange = function(e) {
         if (this.readyState == 4 && this.status == 200) {
           var server_data = JSON.parse(this.responseText);

[rebstar6]

Yep, got it. Though still giving me problems with that:

  ▶ TIMEOUT [expected OK] /_mozilla/mozilla/referrer-policy/no-referrer/meta-referrer/cross-origin/http-http/iframe-tag/generic.keep-origin-redirect.http.html
  │ 
  │ POLICY STR "no-referrer"
  │ ERROR:js::rust: Error at http://web-platform.test:8000/_mozilla/mozilla/referrer-policy/generic/common.js:116:29: JSON.parse: unexpected end of data at line 1 column 1 of the JSON data
  │ 
  └ 131:132: syntax error: Expected end of line but found unknown token. (-2741)

[jdm]

This fixes it for me. I traced the URL that was being generated by the redirection code in subresource.py and realized that the id query parameter wasn't being included in the final URL.

diff --git a/tests/wpt/mozilla/tests/mozilla/referrer-policy/generic/subresource/subresource.py b/tests/wpt/mozilla/tests/mozilla/referrer-policy/generic/subresource/subresource.py
index fda9ed3..ca688de 100644
--- a/tests/wpt/mozilla/tests/mozilla/referrer-policy/generic/subresource/subresource.py
+++ b/tests/wpt/mozilla/tests/mozilla/referrer-policy/generic/subresource/subresource.py
@@ -24,11 +24,12 @@ def create_redirect_url(request, cross_origin = False):
     if cross_origin:
         destination_netloc = get_swapped_origin_netloc(parsed.netloc)

+    query = filter(lambda x: x.startswith('id='), parsed.query.split('&'))
     destination_url = urlparse.urlunsplit(urlparse.SplitResult(
         scheme = parsed.scheme,
         netloc = destination_netloc,
         path = parsed.path,
-        query = None,
+        query = query[0] if query else None,
         fragment = None))

     return destination_url

[rebstar6]

Awesome! PR out - https://github.com/servo/servo/pull/11238

[rebstar6]

FYI - once meta referrer is in, the tests definitely need more changes for the other delivery policies (more so than we thought...). Referrer-Policy header is the obvious next step, but the tests use Content-Security-Policy, which seems to have been the old way of things at some point.

Not sure how easy a switch this is - I may be able to just change referrer_policy/generic/tools/generate.py for http-csp to add Referrer-Policy not Content-Security-Policy, but that feels hacky. Hrmm.

[jdm]

That is likely the best way to do it, assuming that https://github.com/w3c/webappsec-referrer-policy/commit/fc55d917bee0d5636f52a19a5aefa65f8995c766#commitcomment-17717693 is correct. We should make changes like that in the upstream repository to make sure the right eyes see it.

[paulrouget]

@rebstar6 @jdm once meta referrer PR (3) lands, is it enough to ask the duckduckgo folks to set servo as "trusted"? See what they say here: https://github.com/servo/servo/issues/10309#issuecomment-203929226

[rebstar6]

@paulrouget - I think? You can certainly ask, and if they want more, hopefully they'll say so (and looks like you have anyway).

To summarize, thus far we have implemented the meta-referrer delivery piece of this for Documents only (not workers), and using the policies in the w3c tests (and spec at the time the DuckDuckGo issue happened). The spec changed, so there is an issue to update the policies to the latest, which should be trivial once the tests are changed (https://github.com/servo/servo/issues/11384).

FWIW, we maintain a default of 'No-Referrer' (rather than blank) for now, so that the Referer header isn't sent unless a policy is specified in the right meta element - this is safer than the alternative, since we have not implemented the other delivery methods.

Other delivery types are a TODO, though those also require test changes (seems like those are pretty out of date).

[rebstar6]

Test change for header delivery: https://github.com/w3c/web-platform-tests/pull/3116
More broad test change for cross-origin (not crossorigin) https://github.com/w3c/web-platform-tests/pull/3115 (while I was in there..)

Not sure what review is like on w3c, but those are in there. Once the 1st is approved, work can start on referrer policy delivery via the Referrer-Policy header

[rebstar6]

This set of more manageable issues should cover the remaining work (at least, as of today's spec):

• https://github.com/servo/servo/issues/11384 (maybe e-easy this one?)
• https://github.com/servo/servo/issues/11860
• https://github.com/servo/servo/issues/11861
• https://github.com/servo/servo/issues/11862
• https://github.com/servo/servo/issues/11863

There may also be some fetch work when the other delivery policies are implemented (?).

I'm stepping off this as much with my job starting Monday, so these issues are free for the taking! I can definitely answer questions though for whoever picks it up :)

[nox]

We still need to fix #11861 and #11863.

[Darkspirit]

• https://github.com/w3c/webappsec-referrer-policy/pull/67 removed "Determine policy for token": https://github.com/servo/servo/blob/ad1a4adac56fc86c12f59da92f0539d8e94a4848/components/script/dom/document.rs#L4859-L4873
• https://github.com/w3c/webappsec-referrer-policy/pull/125 changed default policy from NoReferrerWhenDowngrade to StrictOriginWhenCrossOrigin.