Re: [whatwg/fetch] Define the Content-Type header parser (#831)

domenic commented on this pull request.



> +      </ol>
+    </ol>
+
+   <li><p>Remove all <a>HTTP tab or space</a> from the start and end of <var>value</var>.
+
+   <li><p><a for=list>Append</a> <var>value</var> to <var>values</var>.
+
+   <li><p>Set <var>value</var> to the empty string.
+  </ol>
+
+ <li><p>Return <var>values</var>.
+</ol>
+
+<div class=example id=example-header-list-get-decode-split>
+ <p>This is how <a>get, decode, and split</a> functions in practice with `<code>A</code>` as
+ <var>name</var> argument:

"as" -> "as the"

> +</ol>
+
+<div class=example id=example-header-list-get-decode-split>
+ <p>This is how <a>get, decode, and split</a> functions in practice with `<code>A</code>` as
+ <var>name</var> argument:
+
+ <table>
+  <tr>
+   <th>Headers (as on the network)
+   <th>Output
+  <tr>
+   <td>
+    <pre><code>
+A: nosniff,
+</code></pre>
+   <td rowspan=2>« "<code>nosniff</code>", "" »

So you could do the following formatting, not sure if it'd be better:

```html
«<br>
&nbsp;&nbsp;"<code>nosniff</code>",<br>
&nbsp;&nbsp;""<br>
»
```

(or maybe some variant with white-space: pre on the rows). Example:

![image](https://user-images.githubusercontent.com/617481/48811563-624e1c00-ecfc-11e8-860b-2f3ec8757458.png)


> @@ -471,6 +685,9 @@ each other by 0x2C 0x20, in order.
      "<code>text/plain</code>", then return false.
     </ol>
 
+    <p class=warning>This intentionally does not use <a>extract a MIME type</a> as that algorithm is
+    rather forgiving and servers are not expected to implement it.

I find this warning confusing. Why are we talking about what servers should implement here? Isn't this part of the processing model for the browser?

> +     <li><p>Set <var>essence</var> to <var>mimeType</var>'s <a for="MIME type">essence</a>.
+    </ol>
+
+   <li><p>Otherwise, if <var>mimeType</var>'s <a for="MIME type">essence</a> is <var>essence</var>,
+   <var>mimeType</var>'s <a for="MIME type">parameters</a>["<code>charset</code>"] does not
+   <a for=map>exist</a>, and <var>charset</var> is non-null, set <var>mimeType</var>'s
+   <a for="MIME type">parameters</a>["<code>charset</code>"] to <var>charset</var>.
+  </ol>
+
+ <li><p>Return <var>mimeType</var>.
+</ol>
+
+<p class=warning>When <a>extract a MIME type</a> returns failure or a <a for=/>MIME type</a> whose
+<a for="MIME type">essence</a> is incorrect for a given format, treat this as a fatal error.
+Existing web platform features not always following this pattern has been a major source of security
+vulnerabilities over the years.

Maybe "Existing web platform features have not always followed this pattern, which has been a major source of security vulnerabilities in those features over the years".

> +   <li><p>Otherwise, if <var>mimeType</var>'s <a for="MIME type">essence</a> is <var>essence</var>,
+   <var>mimeType</var>'s <a for="MIME type">parameters</a>["<code>charset</code>"] does not
+   <a for=map>exist</a>, and <var>charset</var> is non-null, set <var>mimeType</var>'s
+   <a for="MIME type">parameters</a>["<code>charset</code>"] to <var>charset</var>.
+  </ol>
+
+ <li><p>Return <var>mimeType</var>.
+</ol>
+
+<p class=warning>When <a>extract a MIME type</a> returns failure or a <a for=/>MIME type</a> whose
+<a for="MIME type">essence</a> is incorrect for a given format, treat this as a fatal error.
+Existing web platform features not always following this pattern has been a major source of security
+vulnerabilities over the years.
+
+<p class=note>A <a for=/>MIME type</a>'s <a for="MIME type">parameters</a> are typically ignored and
+this has not been a problem in practice.

This note seems out of place right after an algorithm that preserves the parameters. Maybe it is supposed to relate to the previous warning? If so I'd put it inside the warning, instead of as a separate note.

> +
+<p class=warning>When <a>extract a MIME type</a> returns failure or a <a for=/>MIME type</a> whose
+<a for="MIME type">essence</a> is incorrect for a given format, treat this as a fatal error.
+Existing web platform features not always following this pattern has been a major source of security
+vulnerabilities over the years.
+
+<p class=note>A <a for=/>MIME type</a>'s <a for="MIME type">parameters</a> are typically ignored and
+this has not been a problem in practice.
+
+<div class=example id=example-extract-a-mime-type>
+ <p>This is how <a>extract a MIME type</a> functions in practice:
+
+ <table>
+  <tr>
+   <th>Headers (as on the network)
+   <th>Output

Maybe "Output (serialized)", optionally linking "serialized" to the MIME type serializer.

> + <p>This is how <a>extract a MIME type</a> functions in practice:
+
+ <table>
+  <tr>
+   <th>Headers (as on the network)
+   <th>Output
+  <tr>
+   <td>
+    <pre><code>
+Content-Type: text/plain;charset=gbk, text/html
+</code></pre>
+   <td><code>text/html</code>
+  <tr>
+   <td>
+    <pre><code>
+Content-Type: text/html;charset=gbk;a=b, text/html;x=y

Can you do a rowspan with this example having two Content-Type lines? It's an especially weird feature that "later" Content-Type lines can get a charset "added to them" by earlier ones, so showing that would be good. (I know a comma version is equivalent, but making it explicit is nice too.)

> +
+ <li><p>Return <var>mimeType</var>.
+</ol>
+
+<p class=warning>When <a>extract a MIME type</a> returns failure or a <a for=/>MIME type</a> whose
+<a for="MIME type">essence</a> is incorrect for a given format, treat this as a fatal error.
+Existing web platform features not always following this pattern has been a major source of security
+vulnerabilities over the years.
+
+<p class=note>A <a for=/>MIME type</a>'s <a for="MIME type">parameters</a> are typically ignored and
+this has not been a problem in practice.
+
+<div class=example id=example-extract-a-mime-type>
+ <p>This is how <a>extract a MIME type</a> functions in practice:
+
+ <table>

Maybe add a `*/*` example?

-- 
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/831#pullrequestreview-177035946