open a new tab in a default tab shouldn't have userContextId set

RESOLVED INVALID

Status

()

Core
DOM
RESOLVED INVALID
2 years ago
a year ago

People

(Reporter: huseby, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [userContextId][OA-testing])

(Reporter)

Description

2 years ago
in the file docshell/base/nsDocShell.cpp there's the CreatePrincipalReferrer function that needs to be fixed to propagate the correct user context id:

> 9526 nsresult
> 9527 nsDocShell::CreatePrincipalFromReferrer(nsIURI* aReferrer,
> 9528                                         nsIPrincipal** aResult)
> 9529 {
> 9530   PrincipalOriginAttributes attrs;
> 9531   attrs.InheritFromDocShellToDoc(GetOriginAttributes(), aReferrer);
> 9532   nsCOMPtr<nsIPrincipal> prin =
> 9533     BasePrincipal::CreateCodebasePrincipal(aReferrer, attrs);
> 9534   prin.forget(aResult);
> 9535
> 9536   return *aResult ? NS_OK : NS_ERROR_FAILURE;
> 9537 }

this needs to be changed to inherit the origin attributes from the current DocShell, or whatever is necessary, so that the userContextId propagates properly. file bug and assign it to tanvi.
(Reporter)

Comment 1

2 years ago
This works properly.
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WONTFIX

Updated

2 years ago
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Hi Tanvi
Would you mind sharing more details why you reopened this?
If there are still problems on this I'd like to help on this. :)

Thanks
Flags: needinfo?(tanvi)

Comment 3

2 years ago
(In reply to Yoshi Huang[:allstars.chh] from comment #2)
> Hi Tanvi
> Would you mind sharing more details why you reopened this?
> If there are still problems on this I'd like to help on this. :)
> 
> Thanks

Hi Yoshi,

When calling CreatePrincipalFromReferrer, we should be using the origin attributes from the current docshell.  Not the referrer (the referrer could have come from another docshell in another tab that has a different set of origin attributes).  I reopened this bug because I wanted to confirm that we use the right OriginAttributes.  Here is an example:

Tab open to a.com in the Work Container.  User opens a link from a.com into a new default container tab for b.com.  The b.com load should have the origin attributes of the new docshell with usercontextID = 0.

Looking at the code, this probably happens.  Since CreatePrincipalFromReferrer calls InheritFromDocShellToDoc:
https://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#9543

But why is aReferrer even passed into InheritFromDocShellToDoc?  It doesn't look like the aURI parameter is used:
http://mxr.mozilla.org/mozilla-central/source/caps/BasePrincipal.cpp#34
Flags: needinfo?(tanvi) → needinfo?(allstars.chh)
(In reply to Tanvi Vyas - behind on needinfos [:tanvi] from comment #3)
> But why is aReferrer even passed into InheritFromDocShellToDoc?  It doesn't
> look like the aURI parameter is used:
> http://mxr.mozilla.org/mozilla-central/source/caps/BasePrincipal.cpp#34

See the TODO a few lines below
http://mxr.mozilla.org/mozilla-central/source/caps/BasePrincipal.cpp#44

The nsIURI argument is primarily for NSec, if we found the URI provided is not inside the same package, we shouldn't inherit mSignedPkg attribute, but it should be not in priority now.

From Jonas' comment
https://bugzilla.mozilla.org/show_bug.cgi?id=1209162#c39
"However when we then load a document into that docshell, the inherited properties depend on the uri of that document. But PrincipalOriginAttributes::InheritFromDocShellToDoc will take care of that."

If you find that confusing I'll file another bug to remove it.
Flags: needinfo?(allstars.chh)
(Reporter)

Updated

2 years ago
Whiteboard: [userContextId] → [userContextId][OA]
Assignee: tanvi → allstars.chh
Depends on: 1264156
(In reply to Tanvi Vyas [:tanvi] from comment #3)
> Tab open to a.com in the Work Container.  User opens a link from a.com into
> a new default container tab for b.com.  The b.com load should have the
> origin attributes of the new docshell with usercontextID = 0.
> 

I'll write a test for testing the opened tab should have the defaul user context id.

(In reply to Yoshi Huang[:allstars.chh] from comment #4)
> From Jonas' comment
> https://bugzilla.mozilla.org/show_bug.cgi?id=1209162#c39
> "However when we then load a document into that docshell, the inherited
> properties depend on the uri of that document. But
> PrincipalOriginAttributes::InheritFromDocShellToDoc will take care of that."
> 
> If you find that confusing I'll file another bug to remove it.
bug 1264156

Updated

2 years ago
Whiteboard: [userContextId][OA] → [userContextId][OA-testing]
It looks Bug 1248251 already has fixed this.
Summary: docshell needs to set the correct user context id when creating a principal from referrer → test for referrer should not be sent in a container tab.
Sorry, update the subject according to comment 5.
Summary: test for referrer should not be sent in a container tab. → open a new tab in a default tab shouldn't have userContextId set
not working on this
Assignee: allstars.chh → nobody
close this as invalid as the subject is no longer correct.
Status: REOPENED → RESOLVED
Last Resolved: 2 years agoa year ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.