Let Origin header honor referrer policy for non CORS request

[JuniorHsu]

Let Origin header honor referrer policy for non CORS request

Tests: web-platform-tests/wpt#14260

---

Preview | Diff

[annevk]

I think we should only take this into account when setting the Origin header, otherwise it will also affect various CORS algorithms and that does not seem like the right thing to do.

Also, for some cross-origin navigation requests the Origin header was always null per the tests, right? That also needs to be reflected somehow.

[JuniorHsu]

I think we should only take this into account when setting the Origin header, otherwise it will also affect various CORS algorithms and that does not seem like the right thing to do.

Okay, it's fair.

Also, for some cross-origin navigation requests the Origin header was always null per the tests, right? That also needs to be reflected somehow.

For form POST navigation,
(a) the redirect case is covered with tainted origin flag
(b) other cases are all the same with non-CORS fetch

Was I missing something?

[yutakahirano]

Sorry for naive questions:

• What is the rationale?
• Why is this only for non CORS requests?

[JuniorHsu]

* What is the rationale?

Please see https://un5h208565ak8emkwgjjkgb49yug.julianrbryant.com/show_bug.cgi?id=1504085#c7
At the beginning, we don't want to leak Origin: for HTTPS->HTTP cases.
And we find out Referrer-Policy is a good target to honor.

* Why is this only for non CORS requests?

Please see web-platform-tests/wpt#15937 (comment)
Tag @annevk who might have some ideas.

[annevk]

CORS is something both sides opt into so it seems reasonable that in that case we leak the origin. We previously decided that the Referrer Policy would not influence CORS for that reason. However, Origin is also used outside of CORS and we have not effectively dealt with it there.

[yutakahirano]

Thank you.

[annevk]

Thanks for the update, this mostly looks good to me now except for some minor things. Test coverage is good and I think other browsers are on board as well. I can take care of filing bugs on them once we land this.

For the branching on referrer policy adding "Otherwise" and "Do nothing." as its steps might also be good, to be explicit about that scenario.

And we should add a Note explaining why we adhere to Referrer Policy only sometimes. I can also take that on before I merge this unless you want to give it a go.

[JuniorHsu]

I'm with you for the view of websocket. I addressed this with my new patch.

Thanks for the update, this mostly looks good to me now except for some minor things. Test coverage is good and I think other browsers are on board as well. I can take care of filing bugs on them once we land this.

Thanks for taking care of filing bug. FWIW, we also need a bug for navigation from about:blank

For the branching on referrer policy adding "Otherwise" and "Do nothing." as its steps might also be good, to be explicit about that scenario.

This issue is gone since I change the structure

And we should add a Note explaining why we adhere to Referrer Policy only sometimes. I can also take that on before I merge this unless you want to give it a go.

Thanks for taking care of the note.

[annevk]

Noticed a few issues unfortunately. I also added a note, please let me know if that looks good to you.

[annevk]

I think this is all good now. Anyone have any final thoughts? @JuniorHsu are you happy with the nits I pushed?

[JuniorHsu]

@annevk Yes, I'm happy :) Thanks for those tremendous help!

[annevk]

• https://un5h208565ak8emkwgjjkgb49yug.julianrbryant.com/show_bug.cgi?id=1424076
• https://un5h2085w35j89wkxbcf89h0br.julianrbryant.com/p/chromium/issues/detail?id=979142
• https://un5h2085w35jr3j0h78verhh.julianrbryant.com/show_bug.cgi?id=199261

[annevk]

And thank you for pushing this through @JuniorHsu!