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

@annevk commented on this pull request.



> +    "HTTP-COMPRESSION-DICTIONARIES": {
+        "aliasOf": "RFC9842"
+    },

We can just reference the RFC number directly. No need for an alias.

> @@ -1798,6 +1801,7 @@ is "<code>all</code>" or "<code>none</code>". Unless stated otherwise it is "<co
 <p>A <a for=/>request</a> has an associated
 <dfn export for=request id=concept-request-initiator>initiator</dfn>, which is
 the empty string,
+"<code>compression-dictionary</code>",

Why do we need a distinct initiator if we also have a distinct destination?

> @@ -3267,6 +3277,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>. [[!HTTP-COMPRESSION-DICTIONARIES]]

Are there PRs against Storage and Clear-Site-Data about clearing this cache? It would be good if those were linked from the commit message.

> @@ -6385,12 +6412,131 @@ run these steps:
  <li><p>If <var>isAuthenticationFetch</var> is true, then create an <a>authentication entry</a> for
  <var>request</var> and the given realm.
 
+ <li>
+  <p>If <var>response</var>'s <a for=response>header list</a>
+  <a for="header list">contains</a> `<code>Use-As-Dictionary</code>`, then:

```suggestion
  <a for="header list">contains</a> `<code>Use-As-Dictionary</code>`:
```

> @@ -6385,12 +6412,131 @@ run these steps:
  <li><p>If <var>isAuthenticationFetch</var> is true, then create an <a>authentication entry</a> for
  <var>request</var> and the given realm.
 
+ <li>
+  <p>If <var>response</var>'s <a for=response>header list</a>
+  <a for="header list">contains</a> `<code>Use-As-Dictionary</code>`, then:
+  <!-- This is defined in [[!HTTP-COMPRESSION-DICTIONARIES]] -->
+
+  <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>pattern</var> be the result of creating a URL pattern from

"creating a URL pattern" needs some kind of link to a definition.

> +  <a for="header list">contains</a> `<code>Use-As-Dictionary</code>`, then:
+  <!-- This is defined in [[!HTTP-COMPRESSION-DICTIONARIES]] -->
+
+  <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>pattern</var> be the result of creating a URL pattern from
+   <var>dictionaryValue</var>["<code>match</code>"] and <var>request</var>'s
+   <a for=request>current URL</a>.
+
+   <li><p>If <var>pattern</var> is failure or <var>pattern</var> has regexp groups, then return

"has regexp groups" needs a definition.

> +   <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>.

Maybe we should do this before creating a URL pattern. This seems less expensive.

> +   <li><p>Let <var>corpPolicy</var> be the result of <a for="header list">getting</a>
+   `<a http-header><code>Cross-Origin-Resource-Policy</code></a>` from <var>response</var>'s
+   <a for=response>header list</a>.
+
+   <li><p>If <var>request</var>'s <a for=request>response tainting</a> is "<code>opaque</code>" and
+   <var>corpPolicy</var> is not `<code>cross-origin</code>`, then return <var>response</var>.

This isn't good enough. We want _corpPolicy_ to be a good value across redirects. We should have tests for that as well.

> +   <li><p>If <var>pattern</var> is failure or <var>pattern</var> has regexp groups, 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>corpPolicy</var> be the result of <a for="header list">getting</a>
+   `<a http-header><code>Cross-Origin-Resource-Policy</code></a>` from <var>response</var>'s
+   <a for=response>header list</a>.
+
+   <li><p>If <var>request</var>'s <a for=request>response tainting</a> is "<code>opaque</code>" and
+   <var>corpPolicy</var> is not `<code>cross-origin</code>`, then return <var>response</var>.
+
+   <li><p>Let <var>expirationTime</var> be the time at which the <var>response</var> becomes stale.

This needs more links to definitions.

> +
+   <li><p>If <var>compressionDictionaryCache</var> is null, then return <var>response</var>.
+
+   <li><p>Let <var>corpPolicy</var> be the result of <a for="header list">getting</a>
+   `<a http-header><code>Cross-Origin-Resource-Policy</code></a>` from <var>response</var>'s
+   <a for=response>header list</a>.
+
+   <li><p>If <var>request</var>'s <a for=request>response tainting</a> is "<code>opaque</code>" and
+   <var>corpPolicy</var> is not `<code>cross-origin</code>`, then return <var>response</var>.
+
+   <li><p>Let <var>expirationTime</var> be the time at which the <var>response</var> becomes stale.
+
+   <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>dictionaryValue</var> and <var>expirationTime</var>.

Shouldn't we also store _pattern_ somewhere?

> + <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>.

I don't understand this. The dictionary is partitioned so how is it different from the HTTP cache?

> + <li><p><a for="header list">Combine</a> (`<code>Accept-Encoding</code>`, `<code>dcb</code>`)
+ in <var>request</var>'s <a for=request>header list</a>.
+
+ <li><p><a for="header list">Combine</a> (`<code>Accept-Encoding</code>`, `<code>dcz</code>`)
+ in <var>request</var>'s <a for=request>header list</a>.

Are we doing this even if web developers set these headers? That seems rather sketchy.

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

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