Closed Bug 1237915 Opened 5 years ago Closed 5 years ago

devtools storage interface needs to use the correct user context id when opening indexdb connections

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: huseby, Assigned: timhuang)

References

(Blocks 1 open bug)

Details

(Whiteboard: [userContextId][OA])

Attachments

(1 file, 2 obsolete files)

in the file devtools/server/actors/storage.js there's this code:

> 1323     if (/^(about:|chrome:)/.test(host)) {
> 1324       principal = Services.scriptSecurityManager.getSystemPrincipal();
> 1325     } else {
> 1326       let uri = Services.io.newURI(host, null, null);
> 1327       principal = Services.scriptSecurityManager.createCodebasePrincipal(uri, {});
> 1328     }

two things about this code i'm concerned about.  the first thing is the test for about:.  isn't the new tab page an "about:" page?  if it is, then we don't want to use the system principal for the new tab page principal, it needs to have a normal codebase principal with the correct user context id.  the second thing is that for all of the uri's, we need to use origin attributes with the correct user context id.  i'm not sure the best way to get the user context id in this scope.  maybe there's a way to access the docshell making the request?
Does this code have access to the document making the request?  If not, can we change it to pass around serialized principals or something instead of hostnames?
Whiteboard: [userContextId]
Assignee: huseby → tihuang
According to the following code in the storage.js, the docshell can be acquired from the parentActor of the storage actor.

> 1990    // Fetch all the inner iframe windows in this tab.
> 1991    this.fetchChildWindows(this.parentActor.docShell);

However, the scope of the code in the comment 0 does not belong to the same scope of the storage actor. But the origin attributes can still be passed as an argument because the storage actor calls this function. Therefore, I think we could add an argument to pass the origin attributes. Although, there are many functions involve this function to open requests to indexed DB in the storage.js. So these functions should be modified accordingly.
I would recommend if we pass around nsIPrincipal objects rather than explicitly pass OriginAttributes structs.

So grab document.nodePrincipal and pass that to the indexedDB code.
If that for some reason is impossible, you should grab the originAttributes from the document. Not from the docshell. But we really should try to use the principal instead.
Attached patch WIP - v2 (obsolete) — Splinter Review
This patch uses the principal object instead of OriginAttributes structs.
Attachment #8735817 - Flags: feedback?(jonas)
Attachment #8735438 - Attachment is obsolete: true
Attachment #8735438 - Flags: feedback?(huseby)
Attachment #8735817 - Attachment is obsolete: true
Attachment #8735817 - Flags: feedback?(jonas)
Comment on attachment 8737450 [details] [diff] [review]
Fix devtools storage interface to use the correct user context id when opening indexdb connections.

Review of attachment 8737450 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me... I never liked the fact that we were grabbing the system principle here.
Attachment #8737450 - Flags: review?(mratcliffe) → review+
Treeherder shows that all devtools related tests are good. This is good to go.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/fba631fb424c
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
See Also: → 1262813
Can we add a mochitest here?  Or file a followup to add one?
Blocks: 1263324
Bug 1263324 has been filed as a follow-up for the test case.
Whiteboard: [userContextId] → [userContextId][OA]
No longer blocks: 1263324
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.