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

@domenic commented on this pull request.

Several TODOs around HTML cross-linking, but I'm not sure what's the latest on the sequencing of the PRs...

>  </ol>
 
+<p>To <dfn>queue a cross-origin embedder policy CORP violation report</dfn>, given a
+<a for=/>response</a> <var>response</var>, an <a for=/>environment settings object</a>
+<var>settingsObject</var>, and a boolean <var>reportOnly</var>, run these steps:
+
+<ol>
+ <li><p>Let <var>endpoint</var> be <var>settingsObject</var>'s embedder policy's
+ report only reporting endpoint if the <var>reportOnly</var> is true and

These should cross-link to HTML, which won't work until the HTML PR is merged.

> +    <th>value
+   </thead>
+   <tbody>
+    <tr>
+     <td>"<code>type</code>"
+     <td>"<code>corp</code>"
+    </tr>
+    <tr>
+     <td>"<code>blocked-url</code>"
+     <td><var>serialized url</var>
+    </tr>
+   </tbody>
+  </table>
+
+ <li><p><a href="https://w3c.github.io/reporting/#queue-report">Queue</a> <var>body</var> as
+ the "<code>coep</code>"report type for <var>endpoint</var> on

```suggestion
 the "<code>coep</code>" report type for <var>endpoint</var> on
```

> +     <td>"<code>corp</code>"
+    </tr>
+    <tr>
+     <td>"<code>blocked-url</code>"
+     <td><var>serialized url</var>
+    </tr>
+   </tbody>
+  </table>
+
+ <li><p><a href="https://w3c.github.io/reporting/#queue-report">Queue</a> <var>body</var> as
+ the "<code>coep</code>"report type for <var>endpoint</var> on
+ <var>settingsObject</var>.
+ [[!REPORTING]]
+</ol>
+
+<p>To perform a <dfn export>cross-origin resource policy check</dfn>, given an <a for=url>origin</a>

This algorithm being separate from the internal version is a bit confusing. I might make it the first thing in this section, instead?

> +
+ <li><p><a href="https://w3c.github.io/reporting/#queue-report">Queue</a> <var>body</var> as
+ the "<code>coep</code>"report type for <var>endpoint</var> on
+ <var>settingsObject</var>.
+ [[!REPORTING]]
+</ol>
+
+<p>To perform a <dfn export>cross-origin resource policy check</dfn>, given an <a for=url>origin</a>
+<var>origin</var>, an <a for=/>environment settings object</a> <var>settingsObject</var>, a
+<a for=/>response</a> <var>response</var>, and an optional boolean <var>forNavigation</var>, run
+these steps:
+
+<ol>
+ <li><p>Set <var>forNavigation</var> to false if it is not given.
+
+ <li><p>Let <var>embedderPolicy</var> be <var>settingsObject</var>'s embedder policy.

TODO cross-link when HTML is merged

> +<p>To perform a <dfn export>cross-origin resource policy check</dfn>, given an <a for=url>origin</a>
+<var>origin</var>, an <a for=/>environment settings object</a> <var>settingsObject</var>, a
+<a for=/>response</a> <var>response</var>, and an optional boolean <var>forNavigation</var>, run
+these steps:
+
+<ol>
+ <li><p>Set <var>forNavigation</var> to false if it is not given.
+
+ <li><p>Let <var>embedderPolicy</var> be <var>settingsObject</var>'s embedder policy.
+
+ <li>
+  <p>If the <a>cross-origin resource policy internal check</a> with <var>origin</var>,
+  "<code>unsafe-none</code>", <var>response</var>, and <var>forNavigation</var> returns
+  <b>blocked</b>, then return <b>blocked</b>.
+
+  <p class="note">This is to queue only Cross-Origin Embedder Policy violation reports.

I don't understand this note

> + with <var>response</var>, <var>settingsObject</var>, and true.
+
+ <li><p>If the <a>cross-origin resource policy internal check</a> with <var>origin</var>,
+ <var>embedderPolicy</var>'s value, <var>response</var>, and <var>forNavigation</var> returns
+ <b>allowed</b>, then return <b>allowed</b>.
+
+ <li><p><a>Queue a cross-origin embedder policy CORP violation report</a> with <var>response</var>,
+ <var>settingsObject</var>, and false.
+
+ <li><p>Return <b>blocked</b>.
+</ol>
+
+<p class="note no-backref">Only HTML's navigate algorithm uses this check with
+<var>forNavigation</var> set to true, and it's always for nested navigations. Otherwise,
+<var>response</var> is either the <a for=internal>internal response</a> of an
+<a>opaque filtered response</a> or a <a for=/>response</a> which will be the

```suggestion
<a>opaque filtered response</a>, or a <a for=/>response</a> which will be the
```

> @@ -3586,9 +3678,8 @@ optionally with a <i>recursive flag</i>, run these steps:
   <p>If <var>internalResponse</var>'s <a for=response>URL list</a> <a for=list>is empty</a>, then
   set it to a <a for=list>clone</a> of <var>request</var>'s <a for=request>URL list</a>.
 
-  <p class=note>A <a for=/>response</a>'s <a for=response>URL list</a> will typically be empty at
-  this point, unless it came from a service worker, in which case it will only be empty if it was
-  created through <a lt="Response()" constructor><code>new Response()</code></a>.
+  <p class=note>A <a for=/>response</a>'s <a for=response>URL list</a> can be empty when it's for
+  "<code>about</code>" URLs for example.

This change seems a bit out of place? And the phrasing is a bit weird. If we keep it then I'd say

> A response's URL list can be empty (for example, when the response represents an `about:` URL).

-- 
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-429975280

Received on Friday, 12 June 2020 19:11:38 UTC