Closed Bug 923625 Opened 6 years ago Closed 6 years ago

DataStore should send the principal as argument in sendAsyncMessage

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: baku, Assigned: baku)

References

Details

Attachments

(1 file, 2 obsolete files)

No description provided.
Depends on: 904298
Attached patch dataStore_principal.patch (obsolete) — Splinter Review
Attachment #828025 - Flags: review?(ehsan)
Comment on attachment 828025 [details] [diff] [review]
dataStore_principal.patch

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

::: dom/datastore/DataStoreService.js
@@ +240,4 @@
>            self.getDataStoreCreate(aWindow, resolve, aStores);
> +        }, function() {
> +          debug("DataStoreServiceChild error callback!");
> +          reject(new aWindow.DOMError("SecurityError", "Access deny"));

Nit: denied.
Attachment #828025 - Flags: review?(ehsan) → review+
(In reply to Andrea Marchesini (:baku) from comment #4)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/6da206d64b49

sorry had to back this out since we had perma orange failures in mochitest 3 on b2g ics after this push. The failure was https://tbpl.mozilla.org/php/getParsedLog.php?id=30366672&tree=Mozilla-Inbound
(Un-CCing sheriffs@ since this was just a backout - Tomcat please re CC if there was something else that we needed to be aware of :-))
Attached patch dataStore_principal.patch (obsolete) — Splinter Review
The principal cannot be checked on B2g Desktop because it always fails.
The reason is that the mochitests run in a test app, but DataStore mochitests install a new app. The principal is checked on top of the 'main' app.
DataStore sends the principal of the nested app and the principal check kills it.
Attachment #828025 - Attachment is obsolete: true
Attachment #8338392 - Flags: review?(ehsan)
Attachment #8338392 - Attachment is obsolete: true
Attachment #8338392 - Flags: review?(ehsan)
Attachment #8338453 - Flags: review?(ehsan)
OS: Linux → All
Hardware: x86_64 → All
Comment on attachment 8338453 [details] [diff] [review]
dataStore_principal.patch

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

Please file a follow-up bug to remove the "geo." prefix from this pref.  Thanks!
Attachment #8338453 - Flags: review?(ehsan) → review+
Blocks: 943430
https://hg.mozilla.org/mozilla-central/rev/ccb70d9cf925
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Ok this is  a naive question, but why does the child still send the principal at all? I was assuming that the code in 916091 make it so the child doesn't send it at all, but it still seems to get sent, but maybe its just validated somehow?

I mean in reference to this code in DataStoreService.js:

cpmm.sendAsyncMessage("DataStore:Get",
   { name: aName }, null, aWindow.document.nodePrincipal );
},
Flags: needinfo?(amarchesini)
Bug 916091 makes possible to send the principal with a message.
The reason why we send the principal is for 3 reasons:

1. validation - using AssertAppPrincipal
https://mxr.mozilla.org/mozilla-central/source/dom/ipc/ContentParent.cpp#2745

2. and for retrieving the appId and do an additional validation in DataStoreService.
bug 942639

3. for checking which DataStore has to be sent in readonly and what in read/write.
https://mxr.mozilla.org/mozilla-central/source/dom/datastore/DataStoreService.js#249
Flags: needinfo?(amarchesini)
Flags: in-testsuite?
Depends on: 1024986
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.