DOM classes have no properties when accessed from a sandbox

RESOLVED FIXED

Status

()

RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: mano, Assigned: mrbkap)

Tracking

({regression})

unspecified
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(Whiteboard: [hardblocker])

Attachments

(2 attachments)

If a the prototype of a sandbox is set to a dom window, the window's "dom classes" properties (eg. window.NodeFilter) are accessible within the sandbox context.  However, the properties of these classes are inaccessible.

To reproduce, try this code in the js console:
alert('keys: ' + Object.keys(NodeFilter));
var sandbox = Components.utils.Sandbox(window, { sandboxPrototype: window });
var codeStr = "alert('keys in sandbox: ' + Object.keys(NodeFilter));";
Components.utils.evalInSandbox(codeStr, sandbox);

Updated

8 years ago
blocking2.0: --- → ?
Created attachment 495702 [details] [diff] [review]
Proposed fix

When an object passes across a compartment boundary (such as between the chrome compartment and the sandbox) we attempt to rewrap it. Constructors didn't have a precreate hook, so we'd recreate the constructor in the sandbox's compartment, but the newly created object didn't have the PostCreatePrototype-created properties. This patch gives constructors a PreCreate hook, so we end up creating a transparent wrapper for the sandbox instead of a whole new object.
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
Attachment #495702 - Flags: review?(Olli.Pettay)
Depends on: 617836
This is a compartments regression, need to fix this for Firefox 4.
blocking2.0: ? → betaN+
Keywords: regression

Updated

8 years ago
Attachment #495702 - Flags: review?(Olli.Pettay) → review?(peterv)
(In reply to comment #1)
> but the newly created object didn't have the
> PostCreatePrototype-created properties.

Trying to understand that, why not?

Updated

8 years ago
Duplicate of this bug: 613459

Updated

8 years ago
Summary: DOM classes have no properties when accessed from a sandbox → [needs review] DOM classes have no properties when accessed from a sandbox
mrbkap, can you answer peterv's question above please?

Updated

8 years ago
No longer blocks: 613459

Updated

8 years ago
Blocks: 613459
(In reply to comment #3)
> Trying to understand that, why not?

Because all we're wrapping is the constructor, we never actually create an object (or the real prototype for one). Furthermore, nsDOMConstructorSH::PostCreatePrototype purposely doesn't forward to nsDOMClassInfo::PostCreatePrototype.

Updated

8 years ago
Whiteboard: hardblocker

Updated

8 years ago
Whiteboard: hardblocker → [hardblocker]

Updated

8 years ago
Attachment #495702 - Flags: review+
Created attachment 502204 [details] [diff] [review]
Additional change to make the above get through mochitest.

With this additional change we get through all unit tests (didn't run talos) on Linux and Mac, but for some reason this (and the original patch as well) seems to cause an orange in xpcshell tests on windows only (?). The failure is in test_resource_async.js, here's the logs:

With my additional changes:

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1294455964.1294458187.853.gz&fulltext=1
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1294458189.1294459682.9181.gz&fulltext=1

W/o my additional changes:

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1294434465.1294437012.26657.gz&fulltext=1
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1294437831.1294439301.5596.gz&fulltext=1
I tried to reproduce the windows problem locally w/o luck, running all of xpcshell tests succeeds with the two patches in this bug applied, as does running the test that failed on try by itself. Therefore I pushed the two patches to try again and got no failures this time! My best guess is that my previous push was based on a bad changeset, or something.

Either way, I think this is ready to land now, once someone reviews my additional changes.

Updated

8 years ago
Attachment #502204 - Flags: review?(mrbkap)

Comment 9

8 years ago
Comment on attachment 502204 [details] [diff] [review]
Additional change to make the above get through mochitest.

Looks good to me.
Attachment #502204 - Flags: review+
Attachment #502204 - Flags: review?(mrbkap) → review+
Landed on m-c:

http://hg.mozilla.org/mozilla-central/rev/14752eafce65
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Updated

8 years ago
Summary: [needs review] DOM classes have no properties when accessed from a sandbox → DOM classes have no properties when accessed from a sandbox
Attachment #495702 - Flags: review?(peterv) → review+
You need to log in before you can comment on or make changes to this bug.