Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add spec for No-Vary-Search hint in speculation rules #278

Merged
merged 5 commits into from
Jul 4, 2023
Merged

Add spec for No-Vary-Search hint in speculation rules #278

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e34e92f
Add spec for No-Vary-Search hint in speculation rules
kjmcnee Jun 9, 2023
a6c3649
WIP
kjmcnee Jun 13, 2023
68ed42e
Update matching
kjmcnee Jun 13, 2023
b2d4ddc
Address comments
kjmcnee Jun 27, 2023
83879dd
Better notes
kjmcnee Jul 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 32 additions & 10 deletions no-vary-search.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ spec: RFC8941; urlPrefix: https://www.rfc-editor.org/rfc/rfc8941.html
text: dictionary; url: name-dictionaries
text: boolean; url: name-boolean
text: inner list; url: name-inner-lists
text: parsing structured fields; url: #text-parse
</pre>
<style>
#example-equivalence-canonicalization table {
Expand Down Expand Up @@ -82,15 +83,14 @@ The [=obtain a URL search variance=] algorithm ensures that all [=URL search var
<h2 id="parsing">Parsing</h2>

<div algorithm>
To <dfn>obtain a URL search variance</dfn> given a [=response=] |response|:
To <dfn>parse a URL search variance</dfn> given a [=map=] |value|:

1. Let |value| be the result of [=header list/getting a structured field value=] given [:No-Vary-Search:] and "`dictionary`" from |response|'s [=response/header list=].
1. If |value| is null, then return the [=default URL search variance=].
1. If |value|'s [=map/keys=] [=list/contains=] anything other than "`key-order`", "`params`", or "`except`", then return the [=default URL search variance=].
1. If |value| is null, then return null.
1. If |value|'s [=map/keys=] [=list/contains=] anything other than "`key-order`", "`params`", or "`except`", then return null.
1. Let |result| be a new [=URL search variance=].
1. Set |result|'s [=URL search variance/vary on key order=] to true.
1. If |value|["`key-order`"] [=map/exists=]:
1. If |value|["`key-order`"] is not a [=boolean=], then return the [=default URL search variance=].
1. If |value|["`key-order`"] is not a [=boolean=], then return null.
1. Set |result|'s [=URL search variance/vary on key order=] to the boolean negation of |value|["`key-order`"].
1. If |value|["`params`"] [=map/exists=]:
1. If |value|["`params`"] is a [=boolean=]:
Expand All @@ -101,17 +101,27 @@ The [=obtain a URL search variance=] algorithm ensures that all [=URL search var
1. Set |result|'s [=URL search variance/no-vary params=] to the empty list.
1. Set |result|'s [=URL search variance/vary params=] to [=URL search variance/no-vary params/wildcard=].
1. Otherwise, if |value|["`params`"] is a [=list=]:
1. If any [=list/item=] in |value|["`params`"] is not a [=string=], then return the [=default URL search variance=].
1. If any [=list/item=] in |value|["`params`"] is not a [=string=], then return null.
1. Set |result|'s [=URL search variance/no-vary params=] to the result of applying [=parse a key=] to each [=list/item=] in |value|["`params`"].
1. Set |result|'s [=URL search variance/vary params=] to [=URL search variance/no-vary params/wildcard=].
1. Otherwise, return the [=default URL search variance=].
1. Otherwise, return null.
1. If |value|["`except`"] [=map/exists=]:
1. If |value|["`params`"] is not true, then return the [=default URL search variance=].
1. If |value|["`except`"] is not a [=list=], then return the [=default URL search variance=].
1. If any [=list/item=] in |value|["`except`"] is not a [=string=], then return the [=default URL search variance=].
1. If |value|["`params`"] is not true, then return null.
1. If |value|["`except`"] is not a [=list=], then return null.
1. If any [=list/item=] in |value|["`except`"] is not a [=string=], then return null.
1. Set |result|'s [=URL search variance/vary params=] to the result of applying [=parse a key=] to each [=list/item=] in |value|["`except`"].
1. Return |result|.

</div>

<div algorithm>
To <dfn>obtain a URL search variance</dfn> given a [=response=] |response|:

1. Let |fieldValue| be the result of [=header list/getting a structured field value=] given [:No-Vary-Search:] and "`dictionary`" from |response|'s [=response/header list=].
1. Let |result| be the result of [=parsing a URL search variance=] given |fieldValue|.
1. If |result| is null, then return the [=default URL search variance=].
1. Otherwise return |result|.

<p class="note">In general, this algorithm is strict and tends to return the [=default URL search variance=] whenever it sees something it doesn't recognize. This is because the [=default URL search variance=] behavior will just cause fewer cache hits, which is an acceptable fallback behavior.
</div>

Expand Down Expand Up @@ -189,6 +199,18 @@ The [=obtain a URL search variance=] algorithm ensures that all [=URL search var
</table>
</div>

<div algorithm>
To <dfn>obtain a URL search variance hint</dfn> given a [=string=] |hintValue|:

1. Let |fieldValue| be the result of [=parsing structured fields=] given |hintValue| and "`dictionary`".
1. If parsing failed, then return null.
1. Let |result| be the result of [=parsing a URL search variance=] given |fieldValue|.
1. If |result| is null, then return null.
1. Otherwise return |result|.
kjmcnee marked this conversation as resolved.
Show resolved Hide resolved

<p class="note">Speculation rule parsing is strict, so the return value of this algorithm surfaces parsing errors instead of silently returning the [=default URL search variance=].
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that logic applies here. Here's my reasoning:

Whatever the meaning of the NVS header, recognized or unrecognized is, what we want to know is "would the prefetch cache, when it arrives, consider it to match?". In that light, I think it makes sense to match the behavior of the prefetch cache (and therefore, to treat this no more strictly than it does).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I think I see your point. Also the motivation for the strictness described in "parse speculation rules" talks about misunderstood rules leading to more speculation than was intended. But here, we know the consequence of an invalid hint and it would not cause more speculation.

For debuggability, we should presumably still have the implementation warn about this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tagging @liviutinta in case of any concerns.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the parsing of the hint to fallback to the default variance. Note that in "parse a speculation rule" this still treats non-string values for expects_no_vary_search as invalid.

</div>

<div algorithm>
To <dfn>parse a key</dfn> given an [=ASCII string=] |keyString|:

Expand Down
76 changes: 67 additions & 9 deletions prefetch.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ spec: nav-speculation; urlPrefix: prerendering.html
type: dfn
text: getting the supported loading modes; url: get-the-supported-loading-modes
text: uncredentialed-prefetch; for: Supports-Loading-Mode; url: supports-loading-mode-uncredentialed-prefetch
spec: nav-speculation; urlPrefix: no-vary-search.html
type: dfn
text: URL search variance; url: url-search-variance
text: obtain a URL search variance; url: obtain-a-url-search-variance
text: equivalent modulo search variance; url: equivalent-modulo-search-variance
spec: resource-timing; urlPrefix: https://w3c.github.io/resource-timing/
type: dfn; for: PerformanceResourceTiming; text: delivery type; url: dfn-delivery-type
</pre>
Expand Down Expand Up @@ -185,6 +190,7 @@ A <dfn export>prefetch record</dfn> is a [=struct=] with the following [=struct/
* <dfn export for="prefetch record">URL</dfn>, a [=URL=]
* <dfn export for="prefetch record">anonymization policy</dfn>, a [=prefetch IP anonymization policy=]
* <dfn export for="prefetch record">referrer policy</dfn>, a [=referrer policy=]
* <dfn export for="prefetch record">No-Vary-Search hint</dfn>, a [=URL search variance=]
* <dfn export for="prefetch record">label</dfn>, a [=string=]

<div class="note">This is intended for use by a specification or [=implementation-defined=] feature to identify which prefetches it created. It might also associate other data with this struct.</div>
Expand All @@ -194,6 +200,7 @@ A <dfn export>prefetch record</dfn> is a [=struct=] with the following [=struct/
* <dfn export for="prefetch record">fetch controller</dfn>, a [=fetch controller=] (a new [=fetch controller=] by default)
* <dfn export for="prefetch record">sandboxing flag set</dfn>, a [=sandboxing flag set=]
* <dfn export for="prefetch record">redirect chain</dfn>, a [=redirect chain=] (empty by default)
* <dfn export for="prefetch record">creation time</dfn>, a {{DOMHighResTimeStamp}} (0.0 by default)
kjmcnee marked this conversation as resolved.
Show resolved Hide resolved
* <dfn export for="prefetch record">expiry time</dfn>, a {{DOMHighResTimeStamp}} (0.0 by default)
* <dfn export for="prefetch record">source partition key</dfn>, a [=network partition key=] or null (the default)
* <dfn export for="prefetch record">isolated partition key</dfn>, a [=network partition key=] whose first item is an [=opaque origin=] and which represents a separate partition in which cross-partition state can be temporarily stored, or null (the default)
Expand All @@ -207,6 +214,24 @@ A [=prefetch record=]'s <dfn export for="prefetch record">response</dfn> is the

The user agent may [=prefetch record/cancel and discard=] records from the [=Document/prefetch records=] even if they are not expired, e.g., due to resource constraints. Since completed records with expiry times in the past will never be [=find a matching prefetch record|matching prefetch records=], they can be removed with no observable consequences.

<div algorithm>
A [=prefetch record=] |prefetchRecord| <dfn export for="prefetch record">matches a URL</dfn> given a [=URL=] |url| if the following algorithm returns true:
1. If |prefetchRecord|'s [=prefetch record/URL=] is equal to |url|, return true.
1. If |prefetchRecord|'s [=prefetch record/response=] is not null:
1. Let |searchVariance| be the result of [=obtaining a URL search variance=] given |prefetchRecord|'s [=prefetch record/response=].
1. If |prefetchRecord|'s [=prefetch record/URL=] and |url| are [=equivalent modulo search variance=] given |searchVariance|, return true.
1. Otherwise, return false.
</div>

<div algorithm>
A [=prefetch record=] |prefetchRecord| <dfn export for="prefetch record">possibly matches a URL</dfn> given a [=URL=] |url| if the following algorithm returns true:
kjmcnee marked this conversation as resolved.
Show resolved Hide resolved
1. If |prefetchRecord| [=prefetch record/matches a URL=] given |url|, return true.
1. If |prefetchRecord|'s [=prefetch record/response=] is null:
1. Let |searchVariance| be |prefetchRecord|'s [=prefetch record/No-Vary-Search hint=].
1. If |prefetchRecord|'s [=prefetch record/URL=] and |url| are [=equivalent modulo search variance=] given |searchVariance|, return true.
1. Otherwise, return false.
</div>

<div algorithm>
To <dfn export for="prefetch record">cancel and discard</dfn> a [=prefetch record=] |prefetchRecord| given a {{Document}} |document|, perform the following steps.

Expand All @@ -225,34 +250,66 @@ The user agent may [=prefetch record/cancel and discard=] records from the [=Doc
1. [=Assert=]: |document| is [=Document/fully active=].
1. Let |currentTime| be the [=current high resolution time=] for the [=relevant global object=] of |document|.
1. Let |expiryTime| be |currentTime| + 300000 (i.e., five minutes).
1. [=list/Remove=] all elements of |document|'s [=Document/prefetch records=] which have the same [=prefetch record/URL=] as |prefetchRecord| and whose [=prefetch record/state=] equals "`completed`".
1. [=list/Remove=] all elements of |document|'s [=Document/prefetch records=] which [=prefetch record/match a URL=] given |prefetchRecord|'s [=prefetch record/URL=] and whose [=prefetch record/state=] equals "`completed`".
kjmcnee marked this conversation as resolved.
Show resolved Hide resolved
1. Set |prefetchRecord|'s [=prefetch record/state=] to "`completed`" and [=prefetch record/expiry time=] to |expiryTime|.
</div>

<div algorithm="find a matching prefetch record">
To <dfn export>find a matching prefetch record</dfn> given a {{Document}} |document|, [=URL=] |url|, and [=sandboxing flag set=] |sandboxFlags|, perform the following steps.
<div algorithm="find a matching complete prefetch record">
To <dfn export>find a matching complete prefetch record</dfn> given a {{Document}} |document|, [=URL=] |url|, and [=sandboxing flag set=] |sandboxFlags|, perform the following steps.

1. [=Assert=]: |document| is [=Document/fully active=].
1. Let |currentTime| be the [=current high resolution time=] for the [=relevant global object=] of |document|.
1. Let |exactRecord| be null.
1. Let |inexactRecord| be null.
1. [=list/For each=] |record| of |document|'s [=Document/prefetch records=]:
1. If |record|'s [=prefetch record/URL=] is not equal to |url|, then [=iteration/continue=].
1. If |record|'s [=prefetch record/state=] is not "`completed`", then [=iteration/continue=].
1. If |record|'s [=prefetch record/sandboxing flag set=] is empty and |sandboxFlags| is not empty, then [=iteration/continue=].

<div class="note">
Strictly speaking, it would still be possible for this to be valid if sandbox flags have been added to the container since prefetch but those flags would not cause an error due to cross origin opener policy. This is expected to be rare and so isn't handled.
</div>
1. [=list/Remove=] |record| from |document|'s [=Document/prefetch records=].
1. If |record|'s [=prefetch record/expiry time=] is less than |currentTime|, return null.
1. If |record|'s [=prefetch record/redirect chain=] [=redirect chain/has updated credentials=], return null.
1. Return |record|.
1. If |record|'s [=prefetch record/URL=] is equal to |url|:
1. Set |exactRecord| to |record|.
1. [=iteration/Break=].
1. If |inexactRecord| is null and |record| [=prefetch record/matches a URL=] given |url|:
1. Set |inexactRecord| to |record|.
1. Let |recordToUse| be |exactRecord| if |exactRecord| is not null, otherwise |inexactRecord|.
1. If |recordToUse| is not null:
1. [=list/Remove=] |recordToUse| from |document|'s [=Document/prefetch records=].
1. Let |currentTime| be the [=current high resolution time=] for the [=relevant global object=] of |document|.
1. If |recordToUse|'s [=prefetch record/expiry time=] is less than |currentTime|, return null.
1. If |recordToUse|'s [=prefetch record/redirect chain=] [=redirect chain/has updated credentials=], return null.
1. Return |recordToUse|.
1. Return null.

<p class="note">It's not obvious, but this doesn't actually require that the prefetch have received the complete body, just the response headers. In particular, a navigation to a prefetched response might nonetheless not load instantaneously.</p>

<p class="issue">It might be possible to use cache response headers to determine when a response can be used multiple times, but given the short lifetime of the prefetch buffer it's unclear whether this is worthwhile.</p>
</div>

<div algorithm="find a matching prefetch record">
kjmcnee marked this conversation as resolved.
Show resolved Hide resolved
To <dfn export>find a matching prefetch record</dfn> given a {{Document}} |document|, [=URL=] |url|, and [=sandboxing flag set=] |sandboxFlags|, perform the following steps.
kjmcnee marked this conversation as resolved.
Show resolved Hide resolved

1. [=Assert=]: this is running [=in parallel=].
1. Let |cutoffTime| be null.
1. While true:
1. Let |completeRecord| be the result of [=finding a matching complete prefetch record=] given |document|, |url|, and |sandboxFlags|.
1. If |completeRecord| is not null, return |completeRecord|.
1. Let |potentialRecords| be an empty [=list=].
1. [=list/For each=] |record| of |document|'s [=Document/prefetch records=]:
1. If all of the following are true, then [=list/append=] |record| to |potentialRecords|:
* |record|'s [=prefetch record/state=] is "`ongoing`".
* |record| [=prefetch record/possibly matches a URL=] given |url|.
* |record|'s [=prefetch record/sandboxing flag set=] is not empty or |sandboxFlags| is empty.
* |record|'s [=prefetch record/expiry time=] is greater than the [=current high resolution time=] for the [=relevant global object=] of |document|.
* |record|'s [=prefetch record/redirect chain=] does not [=redirect chain/has updated credentials|have updated credentials=].
kjmcnee marked this conversation as resolved.
Show resolved Hide resolved
* |cutoffTime| is null or |record|'s [=prefetch record/creation time=] is less than |cutoffTime|.
1. If |potentialRecords| [=list/is empty=], return null.
1. Wait until the [=prefetch record/state=] of any element of |document|'s [=Document/prefetch records=] changes.
kjmcnee marked this conversation as resolved.
Show resolved Hide resolved
1. If |cutoffTime| is null and any element of |potentialRecords| has a [=prefetch record/state=] that is not "`ongoing`", set |cutoffTime| to the [=current high resolution time=] for the [=relevant global object=] of |document|.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be set before the wait, possibly outside the loop? I assume the goal here is "don't block on prefetches which started after we first started looking"? The current semantics are, at least, confusing to me.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The note below this explains the motivation. The goal is to allow prefetches started after we started blocking, which is why the cutoff isn't set until this point. Would it help to move the note next to this line?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed a little aloud. I see the informative note below, but I'd appreciate it if it could be expanded to be explicit about the intuitive understanding of this heuristic (blocking only on prefetch requests which started before a prefetch completed), why we're using it (we want to allow a slightly later prefetch which is a better match, but don't want a fixed timeout that might cause issues with slow servers), and what its properties are (e.g., in the worst case, we block on two nearly-consecutive prefetches and then the actual navigation if we still failed to match).

Parentheticals are merely inspiration from my understanding and you can write a more cohesive note. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


<p class="note">We have a cutoff for waiting for potential matches such that an additional prefetch that started while we were blocking a navigation could still be used to serve the navigation. In the case of prefetch failure while blocking, we stop considering subsequent prefetches, so we establish a limit after which we fall back to a non-prefetch request.</p>
</div>

<div algorithm>
To <dfn export>create navigation params from a prefetch record</dfn> given a [=navigable=] |navigable|, a [=document state=] |documentState|, a [=navigation id=] |navigationId|, a {{NavigationTimingType}} |navTimingType|, a [=request=] |request|, a [=prefetch record=] |record|, a [=target snapshot params=] |targetSnapshotParams|, and a [=source snapshot params=] |sourceSnapshotParams|, perform the following steps.

Expand Down Expand Up @@ -613,6 +670,7 @@ The <dfn>list of sufficiently strict speculative navigation referrer policies</d
1. Set |prefetchRecord|'s [=prefetch record/source partition key=] to the result of [=determining the network partition key=] given |document|'s [=relevant settings object=].
1. [=Assert=]: |prefetchRecord|'s [=prefetch record/URL=]'s [=url/scheme=] is an [=HTTP(S) scheme=].
1. [=list/Append=] |prefetchRecord| to |document|'s [=Document/prefetch records=]
1. Set |prefetchRecord|'s [=prefetch record/creation time=] to the [=current high resolution time=] for the [=relevant global object=] of |document|.
1. Set |prefetchRecord|'s [=prefetch record/sandboxing flag set=] to the result of [=determining the creation sandboxing flags=] for |document|'s [=Document/browsing context=] given |document|'s [=node navigable=]'s [=navigable/container=].
1. Let |referrerPolicy| be |prefetchRecord|'s [=prefetch record/referrer policy=] if |prefetchRecord|'s [=prefetch record/referrer policy=] is not the empty string, and |document|'s [=Document/policy container=]'s [=policy container/referrer policy=] otherwise.
1. Let |documentState| be a new [=document state=] with
Expand Down
Loading