Open Bug 1618557 Opened 4 years ago Updated 1 year ago

It seems that LoadInfo's top-level document constructor sets up the wrong top-level anti-tracking principal

Categories

(Core :: Privacy: Anti-Tracking, defect, P1)

defect

Tracking

()

ASSIGNED

People

(Reporter: ehsan.akhgari, Assigned: baku, NeedInfo)

References

Details

Attachments

(1 file)

See this principal. Here innerWindow is the old inner window that the top-level navigation is replacing. It seems wrong to use its principal as the top-level anti-tracking principal.

baku, what do you think?

Flags: needinfo?(amarchesini)

This is related to https://hg.mozilla.org/mozilla-central/rev/d496a070135c. It's possible that stopAtOurLevel is mostly false so we always bail our early here, but it could be that the fix to bug 1577298 wasn't completely right...

Blocks: 1577298

baku said he'll take this.

Assignee: nobody → amarchesini

I tried dropping this write in this try push: https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=290729119&revision=9fd39f404f603d7cb668853a20013d7b661600d9

It seems to be fine, but it'd be nice to have a test.

Matt, if you want to work on this, it's great. I can review the code. Let me know.

Flags: needinfo?(amarchesini) → needinfo?(matt.woodrow)

I'm not really sure where to start for adding a test sorry.

I can upload a patch to just remove the offending call if that's useful!

Flags: needinfo?(matt.woodrow)
Priority: -- → P1
Status: NEW → ASSIGNED

Dimi, do we still need to fix this issue? Asking because the fission-porting has changed the code a lot lately.

Flags: needinfo?(dlee)

We still use topLevelPrincipal from LoadInfo to get partitionPrincipal at here. But it is only used when we can't partitionKey from CookieJarSetting in some special cases, so in most of the cases, the value of topLevelPrincipal doesn't matter.

I think the right solution now is to find the cases we don't set partitionKey, and then remove TopLevelPrincipal from LoadInfo entirely to prevent using it in the future. Because this value doesn't work in Fission anyway.

Flags: needinfo?(dlee)

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:baku, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(amarchesini)
See Also: → 1673889

The following patch is waiting for review from an inactive reviewer:

ID Title Author Reviewer Status
D79098 Bug 1618557 - Remove top-level principal from LoadInfo, r?dimi baku dimi: Resigned from review

:baku, could you please find another reviewer or abandon the patch if it is no longer relevant?

For more information, please visit auto_nag documentation.

Flags: needinfo?(amarchesini)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.