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

Status: RESOLVED FIXED

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

Description

[Dave Huseby [:huseby]]

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?

Comment 1

[Boris Zbarsky [:bzbarsky]]

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?

Comment 2

[Tim Huang[:timhuang]]

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.

Comment 3

[Tim Huang[:timhuang]]

Attachment: WIP - Fix devtools storage interface to use the correct user context id when opening indexdb connections.

The WIP patch based on the comment 2.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 6

[Tim Huang[:timhuang]]

Attachment: WIP - v2

This patch uses the principal object instead of OriginAttributes structs.

Comment 7

[Tim Huang[:timhuang]]

Attachment: Fix devtools storage interface to use the correct user context id when opening indexdb connections.

Comment 8

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

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.

Comment 9

[Tim Huang[:timhuang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f27de63c150

Comment 10

[Tim Huang[:timhuang]]

Treeherder shows that all devtools related tests are good. This is good to go.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fba631fb424c

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fba631fb424c

Comment 13

[Tanvi Vyas[:tanvi]]

Can we add a mochitest here?  Or file a followup to add one?

Comment 14

[Tim Huang[:timhuang]]

Bug 1263324 has been filed as a follow-up for the test case.