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

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: mayhemer, Assigned: mrbkap)

Tracking

({fixed1.9.1})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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.
Assignee: jhpedemonte → nobody
Component: Java to XPCOM Bridge → XPConnect
QA Contact: xpcom-bridge → xpconnect
Posted patch FixSplinter Review
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.
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
Attachment #368389 - Flags: superreview?(jst)
Attachment #368389 - Flags: review?(jst)
Attachment #368389 - Flags: superreview?(jst)
Attachment #368389 - Flags: superreview+
Attachment #368389 - Flags: review?(jst)
Attachment #368389 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/63376e31726b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
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.
Flags: wanted1.9.1?
Attachment #368389 - Flags: approval1.9.1+
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.
Flags: wanted1.9.1?
You need to log in before you can comment on or make changes to this bug.