923625 - DataStore should send the principal as argument in sendAsyncMessage

Status: RESOLVED FIXED

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

Comment 1

[Andrea Marchesini [:baku]]

Attachment: dataStore_principal.patch

Comment 2

[Andrea Marchesini [:baku]]

https://tbpl.mozilla.org/?tree=Try&rev=773d913e93db

Comment 3

[(no longer active)]

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.

Comment 4

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6da206d64b49

Comment 5

[Carsten Book [:Tomcat]]

(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

Comment 6

[Ed Morley [:emorley]]

(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 :-))

Comment 7

[Andrea Marchesini [:baku]]

Attachment: dataStore_principal.patch

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.

Comment 8

[Andrea Marchesini [:baku]]

Attachment: dataStore_principal.patch

Comment 9

[(no longer active)]

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!

Comment 10

[Andrea Marchesini [:baku]]

https://tbpl.mozilla.org/?tree=Try&rev=a7ea4affabd0

Comment 11

[Andrea Marchesini [:baku]]

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

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 13

[Paul Theriault [:pauljt] (no longer reading bugmail)]

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 );
},

Comment 14

[Andrea Marchesini [:baku]]

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