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

@domenic commented on this pull request.

I'm not feeling super-qualified to review here; hopefully @annevk can help more. But I did a first pass on some basic stuff.

> @@ -3143,58 +3143,144 @@ 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 string
+<var>embedder policy value</var>, a <a for=/>request</a> <var>request</var> and

Probably this can become `an <a>embedder policy value</a> <var>embedder policy</var>` or similar since that term is now defined.

>  
-  <p class="note no-backref">While redirects that carry  a
-  `<a http-header><code>Cross-Origin-Resource-Policy</code></a>` header are checked, redirects
-  without such a header resulting in <var>response</var> do not affect the outcome as the default is
-  <b>allowed</b>.
-  <!-- This changes with COEP's cross-origin value. -->
+  <p class="note no-backref">Only HTML's navigate algorithm uses this check for the
+  "<code>navigate</code>" <a for=request>mode</a>, for nested navigations. [[!HTML]]

```suggestion
  "<code>navigate</code>" <a for=request>mode</a>, for nested navigations. [[HTML]]
```

(doesn't really matter, but since this is a non-normative note, the reference should be non-normative

> -  <p class="note no-backref">While redirects that carry  a
-  `<a http-header><code>Cross-Origin-Resource-Policy</code></a>` header are checked, redirects
-  without such a header resulting in <var>response</var> do not affect the outcome as the default is
-  <b>allowed</b>.
-  <!-- This changes with COEP's cross-origin value. -->
+  <p class="note no-backref">Only HTML's navigate algorithm uses this check for the
+  "<code>navigate</code>" <a for=request>mode</a>, for nested navigations. [[!HTML]]
+
+  <ol>
+   <li>
+    <p>Assert: <var>request</var> is for a nested navigation.
+
+    <p class=XXX>Fix this assertion when
+    <a href="https://github.com/whatwg/fetch/pull/948">#948</a> is merged.
+
+   <li><p>If <var>embedder policy value</var> is "<code>unsafe-none</code>", then return

This would become `<code><a for="embedder policy value">unsafe-none</a></code>` after the HTML PR is ready. Same for other references to unsafe-none and require-corp.

>  
- <li><p>Return <b>allowed</b>.
+    <ul class=brief>
+     <li><p><var>request</var>'s <a for=request>origin</a> is <a>schemelessly same site</a> with

Why change this to schemelessly same site?

>  
-  <ul class=brief>
-   <li><var>request</var>'s <a for=request>origin</a> is <a>schemelessly same site</a> with
-   <var>request</var>'s <a for=request>current URL</a>'s <a for=url>origin</a>
-   <li><var>request</var>'s <a for=request>origin</a>'s <a for=url>scheme</a> is
-   "<code>https</code>" or <var>response</var>'s <a for=response>HTTPS state</a> is
-   "<code>none</code>"
-  </ul>
+  <dl class=switch>

This lost the otherwise case, meaning it will not return anything in that case. I think we still need the Otherwise case, right?

> +    <th>key
+    <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

This should use Bikeshed linking syntax. Unfortunately it looks like the spec is not set up to allow that easily. I sent https://github.com/w3c/reporting/pull/211

>  </ol>
 
 
+<p>To perform a <dfn>cross-origin resource policy check</dfn>, given a <a for=/>request</a>
+<var>request</var> and a <a for=/>response</a> <var>response</var>, run these steps:</p>
+<ol>
+ <li><p>Let <var>embedder policy</var> be <var>request</var>'s <a for=request>client</a>'s
+ embedder policy.

This will need to cross-link

>  </ol>
 
 
+<p>To perform a <dfn>cross-origin resource policy check</dfn>, given a <a for=/>request</a>
+<var>request</var> and a <a for=/>response</a> <var>response</var>, run these steps:</p>
+<ol>
+ <li><p>Let <var>embedder policy</var> be <var>request</var>'s <a for=request>client</a>'s
+ embedder policy.
+
+ <li>
+  <p>If the <a>cross-origin resource policy internal check</a> with "<code>unsafe-none</code>",
+  <var>request</var> and <var>response</var> returns <b>blocked</b>, then return <b>blocked</b>.
+
+  <p class="note no-backref">This is to queue only COEP related violation reports.

```suggestion
  <p class="note">This is to queue only COEP-related violation reports.
```

>  </ol>
 
 
+<p>To perform a <dfn>cross-origin resource policy check</dfn>, given a <a for=/>request</a>
+<var>request</var> and a <a for=/>response</a> <var>response</var>, run these steps:</p>
+<ol>
+ <li><p>Let <var>embedder policy</var> be <var>request</var>'s <a for=request>client</a>'s
+ embedder policy.
+
+ <li>
+  <p>If the <a>cross-origin resource policy internal check</a> with "<code>unsafe-none</code>",
+  <var>request</var> and <var>response</var> returns <b>blocked</b>, then return <b>blocked</b>.
+
+  <p class="note no-backref">This is to queue only COEP related violation reports.
+
+ <li><p>If the <a>cross-origin resource policy internal check</a> with <var>embedder policy</var>'s
+ report only value, <var>request</var> and <var>response</var> returns <b>blocked</b>, then

"report only value" should cross-link. I guess we should export that from HTML too.

> +<p>To perform a <dfn>cross-origin resource policy check</dfn>, given a <a for=/>request</a>
+<var>request</var> and a <a for=/>response</a> <var>response</var>, run these steps:</p>
+<ol>
+ <li><p>Let <var>embedder policy</var> be <var>request</var>'s <a for=request>client</a>'s
+ embedder policy.
+
+ <li>
+  <p>If the <a>cross-origin resource policy internal check</a> with "<code>unsafe-none</code>",
+  <var>request</var> and <var>response</var> returns <b>blocked</b>, then return <b>blocked</b>.
+
+  <p class="note no-backref">This is to queue only COEP related violation reports.
+
+ <li><p>If the <a>cross-origin resource policy internal check</a> with <var>embedder policy</var>'s
+ report only value, <var>request</var> and <var>response</var> returns <b>blocked</b>, then
+ <a>queue a cross-origin embedder policy CORP violation report</a> with
+ <var>request</var> and <var>embedder policy</var>'s report only reporting endpoint.

Same for "report only reporting endpoint".

Hmm, could we make this algorithm that an "embedder policy" and a mode or something, which is "normal" or "report only"? So it doesn't have to look into the internals of the "embedder policy" struct as much? I dunno, maybe it's fine to have these algorithms so tightly coupled. But it is strange to me that they are split across different specs if so.

>  
 <ol>
- <li><p>If <var>request</var>'s <a for=request>mode</a> is not "<code>no-cors</code>", then return
- <b>allowed</b>.
+ <li><p>Assert: <var>request</var>'s <a for=request>mode</a> is "<code>navigate</code>" or
+ "<code>no-cors</code>".

This is very different than https://wicg.github.io/cross-origin-embedder-policy/#integration-fetch. I guess that is intentional?

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