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.

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

Monkey patch for non-cookie storage access #47

Merged
merged 7 commits into from
Jun 8, 2023
Merged
Changes from all commits
Commits
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
58 changes: 52 additions & 6 deletions index.bs
Original file line number Diff line number Diff line change
Expand Up @@ -453,21 +453,21 @@ Insert the following steps in the <a spec="fetch">HTTP-network fetch</a> algorit
"... run the "set-cookie-string" parsing algorithm (see [section 5.2](https://httpwg.org/specs/rfc6265.html#set-cookie) of [[COOKIES]]) ...":

1. If cookies were stored in the cookie store in the previous step, then
run [=process a network cookie access for bounce tracking mitigations=]
run [=process a navigation storage access for bounce tracking mitigations=]
given <var ignore>request</var>.

<div algorithm>

To <dfn>process a network cookie access for bounce tracking mitigations</dfn>
To <dfn>process a navigation storage access for bounce tracking mitigations</dfn>
given a [=request=] |request|, perform the following steps:

1. Let |origin| be |request|'s [=request/origin=].
1. If |origin| is an [=opaque origin=], then abort these steps.
1. If |request|'s [=request/destination=] is "`document`", then:
1. If |request|'s [=request/client=] is null, or
|request|'s [=request/client=]'s [=environment/target browsing context=]
1. If |request|'s [=request/reserved client=] is null, or
|request|'s [=request/reserved client=]'s [=environment/target browsing context=]
is null, then abort these steps.
1. Let |topLevelTraversable| be |request|'s [=request/client=]'s
1. Let |topLevelTraversable| be |request|'s [=request/reserved client=]'s
[=environment/target browsing context=]'s
[=browsing context/top-level traversable=].
1. If |topLevelTraversable|'s [=top-level traversable/bounce tracking record=]
Expand All @@ -482,10 +482,56 @@ Issue: TODO: Handle subresource requests and iframes for client-side redirects.
</div>

Note: We currently don't treat cookie reads as stateful, but this would be a
reasonable future change. We could run [=process a network cookie access for bounce tracking mitigations=]
reasonable future change. We could run [=process a navigation storage access for bounce tracking mitigations=]
in the <a spec="fetch">HTTP-network-or-cache fetch</a> algorithm after step 8.21.1.2,
"... [=append=] (`Cookie`, cookies) to httpRequest’s [=header list=]. ..."

<h5 id="bounce-tracking-mitigations-service-worker-activation-monkey-patch">Service Worker Activation Monkey Patch</h5>

Each [=top-level traversable=] maintains a record of which sites have activated service workers in the current [=extended navigation=].

Insert the following steps in the [Handle Fetch](https://w3c.github.io/ServiceWorker/#on-fetch-request-algorithm) algorithm after step 23,
"If the result of running the [Run Service Worker](https://w3c.github.io/ServiceWorker/#run-service-worker) algorithm...":

1. Run [=process a navigation storage access for bounce tracking mitigations=] given <var ignore>request</var>.
Copy link
Collaborator

Choose a reason for hiding this comment

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

For some reason github is not showing me the steps for this algorithm anywhere, but I can see them in the preview.

The preview says that we are looking at the request's client. I think for this case we actually want reserved client instead:

https://fetch.spec.whatwg.org/#concept-request-reserved-client

The reserved client is the target container for the new document. The client is the container of the document that initiated the navigation. Sometimes they are the same (click a link in a tab to navigate the tab), but sometimes they are different (click a link that opens a new tab).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This algorithm was already written in the previous PR, it replaces "process a network cookie access for bounce tracking mitigations" since it looks like they're identical.

Makes sense, does that mean we want the same fix for cookie writes as well? It sounds like we should always use the request's "reserved client" when checking the destination navigable.

Copy link
Collaborator

Choose a reason for hiding this comment

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

"reserved client" is only used for document and iframe requests. Its confusing.

So maybe the cookie access thing needs to check one or the other depending on destination.


<h5 id="bounce-tracking-mitigations-storage-access-monkey-patch">Storage Access Monkey Patch</h5>

Each [=top-level traversable=] maintains a record of which sites have accessed storage in the current [=extended navigation=].

Insert the following steps in the <a spec="storage">obtain a storage bottle map</a> algorithm before step 10, "Return <var ignore>proxyMap</var>":

1. Run [=process a general storage access for bounce tracking mitigations=] given <var ignore>environment</var>.

Issue(whatwg/storage#165): This patch has to be run whenever a site accesses non-cookie storage.
<a spec="storage">Obtain a storage bottle map</a> is the intended hook for this, but it does not currently have full coverage across specs that use storage.
So this patch is not comprehensive.</p>

<div algorithm>

To <dfn>process a general storage access for bounce tracking mitigations</dfn>
given an [=environment=] |environment|, perform the following steps:
amaliev marked this conversation as resolved.
Show resolved Hide resolved

1. If |environment| is not an [=environment settings object=], then abort these steps.

Note: At time of writing, <a spec="storage">obtain a storage bottle map</a> can only accept an [=environment settings object=] |environment|,
but this will be refactored to support [=service workers=] which attempt to access storage on every navigation, and thus is not considered
when updating the [=bounce tracking record/storage access set=].

1. Let |origin| be |environment|'s [=environment/top-level origin=].
1. If |origin| is null or an [=opaque origin=], then abort these steps.
1. Let |browsingContext| be |environment|'s [=environment/target browsing context=].
1. If |browsingContext| is null, then abort these steps.
1. Let |topLevelTraversable| be |browsingContext|'s [=browsing context/top-level traversable=].
1. If |topLevelTraversable|'s [=top-level traversable/bounce tracking record=] is null,
then abort these steps.
1. Let |site| be the result of running [=obtain a site=] given |origin|.
1. [=set/Append=] |site|'s [=host=] to |topLevelTraversable|'s
[=top-level traversable/bounce tracking record=]'s
[=bounce tracking record/storage access set=].

</div>

<h5 id="bounce-tracking-mitigations-response-received-monkey-patch">Response Received Monkey
Patch</h5>

Expand Down