Grant storage access to 3rd party, tracking resource on

RESOLVED FIXED in Firefox 63



10 months ago
9 months ago


(Reporter: baku, Assigned: baku)


(Depends on 1 bug, Blocks 2 bugs)

Firefox 63
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)



(2 attachments, 1 obsolete attachment)



10 months ago
When is executed by a 3rd party context, in the TP list, we want to add URL to the first party access storage list.

Comment 1

10 months ago
Posted patch window.patch (obsolete) — Splinter Review
Assignee: nobody → amarchesini
Attachment #8991061 - Flags: review?(ehsan)

Comment 2

10 months ago
Posted patch window.patchSplinter Review
Better approach.
Attachment #8991061 - Attachment is obsolete: true
Attachment #8991061 - Flags: review?(ehsan)
Attachment #8991078 - Flags: review?(ehsan)

Comment 3

10 months ago
Comment on attachment 8991078 [details] [diff] [review]

Review of attachment 8991078 [details] [diff] [review]:

r=me with the below addressed, especially the comment about tests!

::: dom/base/nsContentUtils.h
@@ +2976,5 @@
>                                            nsIChannel* aChannel,
>                                            nsIURI* aURI);
>    /*
> +   * Returns true if this window's channel has been marked as tracking resource.

Nit: s/as/as a/

::: dom/base/nsGlobalWindowOuter.cpp
@@ +7074,5 @@
> +  }
> +
> +  nsCOMPtr<nsIURI> url;
> +  nsresult rv = NS_NewURI(getter_AddRefs(url), aUrl.get(),
> +                          document->GetDocBaseURI());

You have a bug here in parsing this URL: you're not taking the document's charset encoding into account!  Compare to this other place where we parse the URL around the same place: <>

In order to avoid this kind of bug, and also to avoid paying the cost of URL parsing twice, please factor out the URL Parsing from SecurityCheckURL() into OpenInternal() and then pass the nsIURI* argument to SecurityCheckURL() and use the returned nsresult value to decide whether to call MaybeAllowStorageForOpenedWindow().  You can also pass the nsIURI* argument to this function instead of the aUrl string, and use it when calling GetUTFOrigin().

::: toolkit/components/antitracking/test/browser/3rdPartyOpen.html
@@ -5,5 @@
>  <body>
>  <h1>hi!</h1>
>  <script>
> -SpecialPowers.wrap(document).userInteractionForTesting();

Doing this will effectively remove the test that we have for bug 1469993, right?

How about having two tests, one that uses the document that gets directly like this one, and another that a document from a random domain (not a tracker) like which loads another document from the tracker domain and then we get user interaction on the tracker domain?  That way we would be testing both cases...

(It's fine if you want to add that test in a follow-up bug, up to you.)
Attachment #8991078 - Flags: review?(ehsan) → review+


10 months ago
Blocks: 1474812

Comment 4

10 months ago
Posted patch window2.patchSplinter Review
Here a test.

Comment 5

10 months ago
Pushed by
Grant storage access to 3rd party, tracking resource on, r=ehsan
Grant storage access to 3rd party, tracking resource on - tests, r=me
Backed out 2 changesets (bug 1474651) for lint failure in /builds/worker/checkouts/gecko/toolkit/components/antitracking/test/browser/popup.html:8:126 on a CLOSED TREE

Problematic push:

[task 2018-07-11T10:44:28.478Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2018-07-11T10:49:38.752Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/antitracking/test/browser/popup.html:8:126 | Missing semicolon. (semi)
[taskcluster 2018-07-11 10:49:39.414Z] === Task Finished ===
[taskcluster 2018-07-11 10:49:39.414Z] Unsuccessful task run with exit code: 1 completed in 528.88 seconds
Flags: needinfo?(amarchesini)


10 months ago
Flags: needinfo?(amarchesini)

Comment 7

10 months ago
Pushed by
Grant storage access to 3rd party, tracking resource on, r=ehsan
Grant storage access to 3rd party, tracking resource on - tests, r=me


10 months ago
Depends on: 1475189

Comment 8

10 months ago
Last Resolved: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63


10 months ago
Depends on: 1475708


10 months ago
No longer depends on: 1475708
Depends on: 1476324
Depends on: 1480901
Depends on: 1480899
You need to log in before you can comment on or make changes to this bug.