Re: [whatwg/fetch] Add support for compression dictionary transport (PR #1854)

@annevk commented on this pull request.



> @@ -1818,6 +1823,7 @@ device to assist defining CSP and Mixed Content. It is not exposed to JavaScript
 <p>A <dfn export>destination type</dfn> is one of:
 the empty string,
 "<code>audio</code>",
+"<code>compression-dictionary</code>",

This should be below audioworklet.

> @@ -3323,6 +3334,22 @@ or an <a>implementation-defined</a> value.
 </div>
 
 
+<h3 id=compression-dictionary-cache-partitions>Compression-dictionary cache partitions</h3>
+
+<div algorithm>
+<p>To <dfn>determine the compression-dictionary cache partition</dfn>, given a <a for=/>request</a> <var>request</var>:
+
+<ol>
+ <li><p>Let <var>key</var> be the result of <a for=request>determining the network partition key</a>
+ given <var>request</var>.
+
+ <li><p>If <var>key</var> is null, then return null.
+
+ <li><p>Return the unique compression-dictionary cache associated with <var>key</var>. [[!RFC9842]]

I think we should make compression-dictionary cache a link.

> +
+  <ol>
+   <li><p>Let <var>dictionaryValue</var> be the result of
+   <a for="header list">getting a structured field value</a> given `<code>Use-As-Dictionary</code>`,
+   "<code>dictionary</code>", and <var>response</var>'s <a for=response>header list</a>.
+
+   <li><p>If <var>dictionaryValue</var> is null or <var>dictionaryValue</var>["<code>match</code>"]
+   does not <a for=map>exist</a>, then return <var>response</var>.
+
+   <li><p>Let <var>compressionDictionaryCache</var> be the result of
+   <a>determining the compression-dictionary cache partition</a> given <var>request</var>.
+
+   <li><p>If <var>compressionDictionaryCache</var> is null, then return <var>response</var>.
+
+   <li><p>Let <var>pattern</var> be the result of
+   <a for=/>creating a URL pattern</a> given the bare item of <var>dictionaryValue</var>["<code>match</code>"],

What is bare item?

>   <li><p>Return <var>response</var>. <span class=note>Typically <var>response</var>'s
  <a for=response>body</a>'s <a for=body>stream</a> is still being enqueued to after
  returning.</span>
 </ol>
 </div>
 
+<h3 id=http-network-compression-dictionary-fetch>HTTP-network compression-dictionary fetch</h3>
+
+<div algorithm>
+<p>To <dfn id=concept-http-network-compression-dictionary-fetch>HTTP-network compression-dictionary fetch</dfn>,
+given a <a for=/>fetch params</a> <var>fetchParams</var>, an optional boolean
+<var>includeCredentials</var> (default false), and an optional boolean <var>forceNewConnection</var>
+(default false), run these steps:

```suggestion
(default false):
```

> + <li><p>If <var>request</var>'s <a for=request>mode</a> is "<code>no-cors</code>", then return the
+ result of running <a>HTTP-network fetch</a> given <var>fetchParams</var>,
+ <var>includeCredentials</var>, and <var>forceNewConnection</var>.
+
+ <li><p>If the user agent is configured to block cookies for <var>request</var>, then return the
+ result of running <a>HTTP-network fetch</a> given <var>fetchParams</var>,
+ <var>includeCredentials</var>, and <var>forceNewConnection</var>.
+
+ <li><p>Let <var>compressionDictionaryCache</var> be the result of
+ <a>determining the compression-dictionary cache partition</a> given <var>request</var>.
+
+ <li><p>If <var>compressionDictionaryCache</var> is null, then return the result of running
+ <a>HTTP-network fetch</a> given <var>fetchParams</var>, <var>includeCredentials</var>, and
+ <var>forceNewConnection</var>.
+
+ <li><p>Let <var>bestMatch</var> be the result of finding the best matching dictionary in

Can we add a link here as well? (In the way we refer to other RFC terms.)

> + <li><p>Let <var>compressionDictionaryCache</var> be the result of
+ <a>determining the compression-dictionary cache partition</a> given <var>request</var>.
+
+ <li><p>If <var>compressionDictionaryCache</var> is null, then return the result of running
+ <a>HTTP-network fetch</a> given <var>fetchParams</var>, <var>includeCredentials</var>, and
+ <var>forceNewConnection</var>.
+
+ <li><p>Let <var>bestMatch</var> be the result of finding the best matching dictionary in
+ <var>compressionDictionaryCache</var> for <var>request</var> as defined in
+ [[!RFC9842]].
+
+ <li><p>If <var>bestMatch</var> is null, then return the result of running <a>HTTP-network fetch</a>
+ given <var>fetchParams</var>, <var>includeCredentials</var>, and <var>forceNewConnection</var>.
+
+ <li><p>Add the `<code>Available-Dictionary</code>` and `<code>Dictionary-ID</code>`
+ (if applicable) headers to <var>request</var> using <var>bestMatch</var> as defined in

We have append/set algorithms for this. We should use them.

> + in <var>request</var>'s <a for=request>header list</a>.
+
+ <li><p>Let <var>response</var> be the result of running <a>HTTP-network fetch</a> given
+ <var>fetchParams</var>, <var>includeCredentials</var>, and <var>forceNewConnection</var>.
+
+ <li><p>Let <var>codings</var> be the result of <a>extracting header list values</a> given
+ `<code>Content-Encoding</code>` and <var>response</var>'s <a for=response>header list</a>.
+
+ <li><p>If <var>codings</var> is null or does not contain `<code>dcb</code>` or `<code>dcz</code>`,
+ then return <var>response</var>.
+
+ <li><p>If <var>request</var>'s <a for=request>response tainting</a> is "<code>opaque</code>",
+ then return a <a>network error</a>.
+
+ <li><p>Let <var>availableDictionaryItem</var> be the result of
+ <a>getting a structured field value</a> given `<code>Available-Dictionary</code>`,

add the for="header list" attribute to "getting a ..."

>   <li><p>Return <var>response</var>. <span class=note>Typically <var>response</var>'s
  <a for=response>body</a>'s <a for=body>stream</a> is still being enqueued to after
  returning.</span>
 </ol>
 </div>
 
+<h3 id=http-network-compression-dictionary-fetch>HTTP-network compression-dictionary fetch</h3>
+
+<div algorithm>
+<p>To <dfn id=concept-http-network-compression-dictionary-fetch>HTTP-network compression-dictionary fetch</dfn>,
+given a <a for=/>fetch params</a> <var>fetchParams</var>, an optional boolean
+<var>includeCredentials</var> (default false), and an optional boolean <var>forceNewConnection</var>
+(default false), run these steps:
+
+<ol>
+ <li><p>Let <var>request</var> be <var>fetchParams</var>'s <a for="fetch params">request</a>.
+
+ <li><p>If <var>request</var>'s <a for=request>mode</a> is "<code>no-cors</code>", then return the
+ result of running <a>HTTP-network fetch</a> given <var>fetchParams</var>,
+ <var>includeCredentials</var>, and <var>forceNewConnection</var>.

We have this return 5 times. I think we could restructure this with a lambda that initializes a variable. This would a) reduce repetition b) scope a number of variables to where they are needed c) hopefully add some clarity.

You would do something like:

> Let x be the result of running these steps:
> 1. ...

>   <li><p>Return <var>response</var>. <span class=note>Typically <var>response</var>'s
  <a for=response>body</a>'s <a for=body>stream</a> is still being enqueued to after
  returning.</span>
 </ol>
 </div>
 
+<h3 id=http-network-compression-dictionary-fetch>HTTP-network compression-dictionary fetch</h3>

I wonder if here and below we also want a hyphen between network and compression. cc @sideshowbarker 

> + then return a <a>network error</a>.
+
+ <li><p>Let <var>availableDictionaryItem</var> be the result of
+ <a>getting a structured field value</a> given `<code>Available-Dictionary</code>`,
+ "<code>item</code>", and <var>request</var>'s <a for=request>header list</a>.
+
+ <li><p>If <var>availableDictionaryItem</var> is null, then return a <a>network error</a>.
+
+ <li><p>Let <var>availableDictionaryHash</var> be the bare item of <var>availableDictionaryItem</var>.
+
+ <li><p>Let <var>newBody</var> be a new <a for=/>body</a> whose <a for=body>stream</a> is the
+ result of transforming <var>response</var>'s <a for=response>body</a>'s <a for=body>stream</a>
+ with an algorithm that verifies that the dictionary hash in the stream matches
+ <var>availableDictionaryHash</var> and decodes the rest of the stream with the applicable
+ algorithm as defined in [[!RFC9842]]. If verification or decoding fails,
+ the transformed stream must error.

This must seems out-of-place.

> + "<code>item</code>", and <var>request</var>'s <a for=request>header list</a>.
+
+ <li><p>If <var>availableDictionaryItem</var> is null, then return a <a>network error</a>.
+
+ <li><p>Let <var>availableDictionaryHash</var> be the bare item of <var>availableDictionaryItem</var>.
+
+ <li><p>Let <var>newBody</var> be a new <a for=/>body</a> whose <a for=body>stream</a> is the
+ result of transforming <var>response</var>'s <a for=response>body</a>'s <a for=body>stream</a>
+ with an algorithm that verifies that the dictionary hash in the stream matches
+ <var>availableDictionaryHash</var> and decodes the rest of the stream with the applicable
+ algorithm as defined in [[!RFC9842]]. If verification or decoding fails,
+ the transformed stream must error.
+
+ <li><p>Set <var>response</var>'s <a for=response>body</a> to <var>newBody</var>.
+
+ <li><p><a>Delete</a> `<code>Content-Encoding</code>` from <var>response</var>'s

needs a for attribute

> @@ -1967,6 +1973,10 @@ not always relevant and might require different behavior.
    <td>"<code>video</code>"
    <td><code>media-src</code>
    <td>HTML's <code>&lt;video></code> element
+  <tr>
+   <td>"<code>compression-dictionary</code>"
+   <td><code>connect-src</code>
+   <td>HTML's <code>&lt;link rel=compression-dictionary&gt;</code>

```suggestion
   <td>HTML's <code>&lt;link rel=compression-dictionary></code>
```

> +
+   <li><p>Let <var>pattern</var> be the result of
+   <a for=/>creating a URL pattern</a> given the bare item of <var>dictionaryValue</var>["<code>match</code>"],
+   the <a lt="URL serializer">serialization</a> of <var>request</var>'s <a for=request>current URL</a>,
+   and an empty map.
+
+   <li><p>If <var>pattern</var> is failure or <var>pattern</var> <a for=/>has regexp groups</a>,
+   then return <var>response</var>.
+
+   <li><p>Let <var>expirationTime</var> be the time at which the <var>response</var> becomes
+   a <a>stale response</a>.
+
+   <li><p>If <var>expirationTime</var> is not in the future, then return <var>response</var>.
+
+   <li><p>Store <var>response</var> in <var>compressionDictionaryCache</var> with its associated
+   <var>pattern</var>, <var>dictionaryValue</var> and <var>expirationTime</var>.

Oxford comma

> + <li><p>Let <var>codings</var> be the result of <a>extracting header list values</a> given
+ `<code>Content-Encoding</code>` and <var>response</var>'s <a for=response>header list</a>.
+
+ <li><p>If <var>codings</var> is null or does not contain `<code>dcb</code>` or `<code>dcz</code>`,
+ then return <var>response</var>.
+
+ <li><p>If <var>request</var>'s <a for=request>response tainting</a> is "<code>opaque</code>",
+ then return a <a>network error</a>.
+
+ <li><p>Let <var>availableDictionaryItem</var> be the result of
+ <a>getting a structured field value</a> given `<code>Available-Dictionary</code>`,
+ "<code>item</code>", and <var>request</var>'s <a for=request>header list</a>.
+
+ <li><p>If <var>availableDictionaryItem</var> is null, then return a <a>network error</a>.
+
+ <li><p>Let <var>availableDictionaryHash</var> be the bare item of <var>availableDictionaryItem</var>.

This could maybe be inlined, though again we need more clarity on bare item.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/pull/1854#pullrequestreview-4345980370
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/fetch/pull/1854/review/4345980370@github.com>