Re: [whatwg/fetch] Let Origin header honor referrer policy for non CORS request (#908)

annevk commented on this pull request.

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.

> @@ -4070,9 +4070,38 @@ Range Requests</cite>. [[HTTP-RANGE]] However, this is not widely supported by b
 
    <li><p>If the <i>CORS flag</i> is set, <var>httpRequest</var>'s <a for=request>method</a> is
    neither `<code>GET</code>` nor `<code>HEAD</code>`, or <var>httpRequest</var>'s
-   <a for=request>mode</a> is "<code>websocket</code>", then <a for="header list">append</a>
-   `<code>Origin</code>`/the result of <a>serializing a request origin</a> with
-   <var>httpRequest</var>, to <var>httpRequest</var>'s <a for=request>header list</a>.
+   <a for=request>mode</a> is "<code>websocket</code>", then:
+   <ol>
+    <li><p>Let <var>origin</var> be the result of <a>serializing a request origin</a> with
+    <var>httpRequest</var>.
+    <li><p>If <var>request</var>'s <i>CORS flag</i> is unset, then switch on

I suspect we also don't want to apply these steps when mode is "websocket" as that is effectively CORS too. And at that point I wonder if we can restructure this a bit. Perhaps something like:

1. Let serializedOrigin be the result of serializing a request origin with ...
2. If the CORS flag is set or httpRequest's mode is "websocket", then append Origin/serializedOrigin to httpRequest's header list.
3. Otherwise, if httpRequest's method is neither GET nor HEAD:
   1. The Referrer Policy sanitizing origin steps...

What do you think?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/pull/908#pullrequestreview-249879033