484180 - chrome cannot access dom storage object's properties, gets GetProperty call for a prototype function

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Honza Bambas (:mayhemer)]

This is a followup for bug found during making of test for localStorage object be accessible from chrome privileged code. The problem I found is following:

- in a chrome context I get the dom storage manager service and query it directly for a storage (localStorage) belonging to some origin (e.g. "http://example.com" as in the test referred bellow)
- while trying to store a value to it using storage.setItem("key", "value") I get correctly call to nsStorage2SH::NewResolve (the script helper class for localStorage) that does early return because "setItem" is found in the storage's prototype, leaving result values intact
- but then, what is the bug, I get call to nsStorage2SH::GetProperty for "setItem", looks like xpcom in this case doesn't recognize, after NewResolve, that "setItem" is a prototype function and not an object's dynamic (or whatever) property

The test as checked in is here: http://mxr.mozilla.org/mozilla-central/source/dom/tests/mochitest/localstorage/test_localStorageFromChrome.xhtml

Workaround to make the test work was usage of "storage.wrappedJSObject" instead of just "storage". So, to reproduce this bug, just remove ".wrappedJSObject" from the test.

The script helper methods are here: https://bugzilla.mozilla.org/attachment.cgi?id=368148&action=diff#a/dom/base/nsDOMClassInfo.cpp_sec3

This bugs is probably not in the storage implementation (nsStorage2SH class, please note the "2" in the name!) but more in xpcom itself, but it's just a guess.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

The problem here was that XPCNativeWrapper (and cross origin XOWs) call the scriptable helper's GetProperty. This patch fixes the case where we resolved a function, but leaves the attribute case as-is. I'll file a followup bug on that.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/63376e31726b

Comment 3

[cmtalbert]

Mr BKap,

This effectively prevents extensions from using localstorage on a page (calling into localstorage for a given domain x from chrome).  That seems like something we'd want to support, especially since we fixed it on trunk.  

So, should we fix this on 1.9.1?  

I've marked wanted-1.9.1-? to get drivers to notice.

Comment 4

[Johnny Stenback (:jst)]

Comment on attachment 368389 [details] [diff] [review]
Fix

Approving per popular demand.

And this has been on the trunk for some time, no regressions, low risk, and it makes chrome/extension developers happier.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e0820d781d66