Re: [whatwg/fetch] Integrate CORP and COEP (#1030)

@annevk commented on this pull request.

This looks so much better, thank you! I left a number of inline nits.

Not needed for this PR, but what do you think about moving the URL list copying and CORP check into the scheme fetch algorithm? We might not even need as much URL list copying for the other schemes as redirecting to them is forbidden.

> @@ -3145,58 +3148,145 @@ response <a for=/>header</a> can be used to require checking a <a for=/>request<
 Cross-Origin-Resource-Policy     = %s"same-origin" / %s"same-site" / %s"cross-origin" ; case-sensitive
 </code></pre>
 
-<p>To perform a <dfn>cross-origin resource policy check</dfn>, given a <var>request</var> and
-<var>response</var>, run these steps:</p>
+<p>To perform a <dfn>cross-origin resource policy internal check</dfn>, given a
+<a for=/>response</a> <var>response</var>, an <a for=url>origin</a>, <var>origin</var>, an
+embedder policy value <var>embedder policy value</var> and a boolean
+<var>for navigation</var>, run these steps:

Since we're checking if origin, using policy X, can read a response I'd reorder the arguments a bit.

>    `<a http-header><code>Cross-Origin-Resource-Policy</code></a>` headers will have the same effect.
 
- <li><p>If <var>policy</var> is `<code>same-origin</code>`, then return <b>blocked</b>.
+ <li><p>If <var>policy</var> is neither "<code>same-origin</code>", "<code>same-site</code>" nor

Oxford comma before nor.

> @@ -3145,58 +3148,145 @@ response <a for=/>header</a> can be used to require checking a <a for=/>request<
 Cross-Origin-Resource-Policy     = %s"same-origin" / %s"same-site" / %s"cross-origin" ; case-sensitive
 </code></pre>
 
-<p>To perform a <dfn>cross-origin resource policy check</dfn>, given a <var>request</var> and
-<var>response</var>, run these steps:</p>
+<p>To perform a <dfn>cross-origin resource policy internal check</dfn>, given a
+<a for=/>response</a> <var>response</var>, an <a for=url>origin</a>, <var>origin</var>, an
+embedder policy value <var>embedder policy value</var> and a boolean
+<var>for navigation</var>, run these steps:

Please camelCase the variable names.

>    `<a http-header><code>Cross-Origin-Resource-Policy</code></a>` headers will have the same effect.
 
- <li><p>If <var>policy</var> is `<code>same-origin</code>`, then return <b>blocked</b>.
+ <li><p>If <var>policy</var> is neither "<code>same-origin</code>", "<code>same-site</code>" nor

You need to use backticks here as the original text did since the result of getting a header value is a byte sequence, not a string.

-- 
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/1030#pullrequestreview-424562638

Received on Thursday, 4 June 2020 15:54:08 UTC