Re: [whatwg/fetch] CORB: blocking of nosniff and 206 responses (#686)

jakearchibald requested changes on this pull request.

Just minor stuff. This is looking good.

> @@ -2354,6 +2354,55 @@ X-Content-Type-Options           = "nosniff" ; case-insensitive</pre>
 pertain to them. Also, considering "<code>image</code>" was not compatible with deployed content.
 
 
+<h3 id=corb>CORB</h3>
+
+<p class="note">Cross-origin read blocking, better known as CORB, is an algorithm by which dubious
+cross-origin resource fetches are identified and blocked  before they reach a web page. CORB reduces
+the risk of leaking sensitive data by keeping it further from cross-origin web pages.

`blocked  before` remove double space.

> @@ -2354,6 +2354,55 @@ X-Content-Type-Options           = "nosniff" ; case-insensitive</pre>
 pertain to them. Also, considering "<code>image</code>" was not compatible with deployed content.
 
 
+<h3 id=corb>CORB</h3>
+
+<p class="note">Cross-origin read blocking, better known as CORB, is an algorithm by which dubious
+cross-origin resource fetches are identified and blocked  before they reach a web page. CORB reduces
+the risk of leaking sensitive data by keeping it further from cross-origin web pages.

I'm not keen on "dubious" here, but I don't have a better idea.

Is it fair to say we're "blocking fetches that would fail anyway, but blocking them earlier to reduce the risk of leaking sensitive data…"

> @@ -2354,6 +2354,55 @@ X-Content-Type-Options           = "nosniff" ; case-insensitive</pre>
 pertain to them. Also, considering "<code>image</code>" was not compatible with deployed content.
 
 
+<h3 id=corb>CORB</h3>
+
+<p class="note">Cross-origin read blocking, better known as CORB, is an algorithm by which dubious
+cross-origin resource fetches are identified and blocked  before they reach a web page. CORB reduces
+the risk of leaking sensitive data by keeping it further from cross-origin web pages.
+
+<p>A <dfn>CORB-protected MIME type</dfn> is an <a>HTML MIME type</a>, a <a>JSON MIME type</a>, or an
+<a>XML MIME type</a> excluding <code>image/svg+xml</code>.
+
+<p class="note no-backref">Accessing cross-origin resources with <a>CORB-protected MIME types</a> is
+managed by the <a>CORS protocol</a> (e.g., in case of <a><code>fetch()</code></a> or
+{{XMLHttpRequest}}), not observable (e.g., in case of pings or CSP reports which ignore the
+response), or would result in an error (e.g., when failing to decode an HTML document embedded in an
+<code>img</code> tag as an image). This means that CORB can block <a>CORB-protected MIME types</a>
+resources without being disruptive to web pages.

It wasn't clear to me that this note was talking about what happens aside from CORB.

Maybe start "Even without CORB…"?

> Accessing cross-origin resources

Maybe "Accessing the content of cross-origin resources"? Since we allow cross origin resources for imgs, script CSS.

`fetch()` can fetch `no-cors` so it might not be a good example here.

It isn't clear that this is presenting a list of things, so maybe: "…is managed *either* by the CORS protocol".

> +<a>XML MIME type</a> excluding <code>image/svg+xml</code>.
+
+<p class="note no-backref">Accessing cross-origin resources with <a>CORB-protected MIME types</a> is
+managed by the <a>CORS protocol</a> (e.g., in case of <a><code>fetch()</code></a> or
+{{XMLHttpRequest}}), not observable (e.g., in case of pings or CSP reports which ignore the
+response), or would result in an error (e.g., when failing to decode an HTML document embedded in an
+<code>img</code> tag as an image). This means that CORB can block <a>CORB-protected MIME types</a>
+resources without being disruptive to web pages.
+
+<p>To perform a <dfn noexport>CORB check</dfn>, given a <var>request</var> and <var>response</var>,
+run these steps:</p>
+
+<ol>
+ <li><p>If <var>request</var>'s <a for=request>initiator</a> is "<code>download</code>", then return
+ <b>allowed</b>.
+ <!-- XXX If we recast downloading as navigation this step can be removed. -->

@annevk what's your feeling on making this page-visible? I've been frustrated in the past digging into a spec detail only to find there was useful information in an html comment.

> + <a>HTTP(S) scheme</a>, then return <b>allowed</b>.
+
+ <li><p>Let <var>mimeType</var> be the result of <a for="header list">extracting a MIME type</a>
+ from <var>response</var>'s <a for=response>header list</a>.
+
+ <li><p>If <var>response</var>'s <a for=response>status</a> is <code>206</code> and
+ <var>mimeType</var> (ignoring parameters) is a <a>CORB-protected MIME type</a>, then return
+ <b>blocked</b>.
+
+ <li><p>Let <var>nosniff</var> be the result of <a>extracting header values</a> from the
+ <em>first</em> <a for=/>header</a> whose <a for=header>name</a> is a <a>byte-case-insensitive</a>
+ match for `<a http-header><code>X-Content-Type-Options</code></a>` in <var>response</var>'s
+ <a for=response>header list</a>.
+
+ <li><p>If <var>nosniff</var> is not failure and <var>mimeType</var> (ignoring parameters) is a
+ <a>CORB-protected MIME type</a> or <code>text/plain</code>, then return <b>blocked</b>.

We should have a note or something explaining why we block `text/plain` here but not for 206.

>       <!-- file URLs end up here as they are not same-origin typically. -->
+
+     <li>
+      <p>If <var>noCorsResponse</var> is not a <a>filtered response</a> and the <a>CORB check</a>
+      with <var>request</var> and <var>noCorsResponse</var> returns <b>blocked</b>, then:

I'd flip this around, and immediately return `noCorsResponse` if it isn't filtered or passes the CORB check. 

>       <!-- file URLs end up here as they are not same-origin typically. -->
+
+     <li>
+      <p>If <var>noCorsResponse</var> is not a <a>filtered response</a> and the <a>CORB check</a>
+      with <var>request</var> and <var>noCorsResponse</var> returns <b>blocked</b>, then:
+
+      <ol>
+       <li><p>Let <var>oldNoCorsResponse</var> be <var>noCorsResponse</var>.
+
+       <li>
+        <p>Set <var>noCorsResponse</var> to a new <a for=/>response</a> whose

If you do the immediate return as suggested above, you also don't need to do the variable juggling here.

Instead, you can call this something more meaningful, like `sanitizedResponse`.

-- 
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/686#pullrequestreview-118026647

Received on Monday, 7 May 2018 15:26:54 UTC