Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Commit 1ffcf1a

Browse files
committed
Bug 861493. When passing arguments to an Xray for a WebIDL constructor, make sure to do the argument unwrapping before entering the content compartment. r=bholley,waldo
There are several changes here: 1) Adds some MutableThis methods to Optional, Nullable, and dictionaries to effectively allow doing a const_cast without knowing the actual type being templated over. I needed this because I do not in fact know that type in the relevant code. I'm open to suggestions for a better name for this method. 2) Adds some operator& to RootedJSValue to make it look more like a JS::Value, and in particular so I can JS_WrapValue the thing in it. 3) Adds a Slot() method to NonNullLazyRootedObject, just like NonNull has. 4) Adds an operator& to LazyRootedObject to make it look more like JSObject* so I can JS_WrapObject the thing in it. 5) Implements the actual rewrapping of the arguments into the content compartment. 6) Fixes a small preexisting bug in which we didn't look at named constructors in getTypesFromDescriptor (this was causing my tests to not compile). 7) Changes Xrays to not enter the content compartment when calling a WebIDL constructor. 8) Adds some friend API to report things as not being functions.
1 parent 38bbbc4 commit 1ffcf1a

13 files changed

Lines changed: 286 additions & 35 deletions

dom/bindings/BindingDeclarations.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,11 @@ class Optional
265265
return mImpl.ref();
266266
}
267267

268+
Optional& AsMutable() const
269+
{
270+
return *const_cast<Optional*>(this);
271+
}
272+
268273
// If we ever decide to add conversion operators for optional arrays
269274
// like the ones Nullable has, we'll need to ensure that Maybe<> has
270275
// the boolean before the actual data.
@@ -379,6 +384,16 @@ class RootedJSValue
379384
return mValue;
380385
}
381386

387+
JS::Value* operator&()
388+
{
389+
return &mValue;
390+
}
391+
392+
const JS::Value* operator&() const
393+
{
394+
return &mValue;
395+
}
396+
382397
private:
383398
// Don't allow copy-construction of these objects, because it'll do the wrong
384399
// thing with our flag mCx.

dom/bindings/BindingUtils.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1431,6 +1431,11 @@ class NonNullLazyRootedObject : public Maybe<JS::RootedObject>
14311431
MOZ_ASSERT(!empty() && ref(), "Can not alias null.");
14321432
return *ref();
14331433
}
1434+
1435+
JSObject** Slot() { // To make us look like a NonNull
1436+
// Assert if we're empty, on purpose
1437+
return ref().address();
1438+
}
14341439
};
14351440

14361441
class LazyRootedObject : public Maybe<JS::RootedObject>
@@ -1439,6 +1444,12 @@ class LazyRootedObject : public Maybe<JS::RootedObject>
14391444
operator JSObject*() const {
14401445
return empty() ? (JSObject*) nullptr : ref();
14411446
}
1447+
1448+
JSObject** operator&()
1449+
{
1450+
// Assert if we're empty, on purpose
1451+
return ref().address();
1452+
}
14421453
};
14431454

14441455
// A struct that has the same layout as an nsDependentString but much

dom/bindings/Codegen.py

Lines changed: 122 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,7 +1080,7 @@ def generate_code(self):
10801080
name = self._ctor.identifier.name
10811081
nativeName = MakeNativeName(self.descriptor.binaryNames.get(name, name))
10821082
callGenerator = CGMethodCall(nativeName, True, self.descriptor,
1083-
self._ctor)
1083+
self._ctor, isConstructor=True)
10841084
return preamble + callGenerator.define();
10851085

10861086
class CGClassConstructHookHolder(CGGeneric):
@@ -4031,6 +4031,93 @@ class MethodNotCreatorError(Exception):
40314031
def __init__(self, typename):
40324032
self.typename = typename
40334033

4034+
# A counter for making sure that when we're wrapping up things in
4035+
# nested sequences we don't use the same variable name to iterate over
4036+
# different sequences.
4037+
sequenceWrapLevel = 0
4038+
4039+
def wrapTypeIntoCurrentCompartment(type, value):
4040+
"""
4041+
Take the thing named by "value" and if it contains "any",
4042+
"object", or spidermonkey-interface types inside return a CGThing
4043+
that will wrap them into the current compartment.
4044+
"""
4045+
if type.isAny():
4046+
assert not type.nullable()
4047+
return CGGeneric("if (!JS_WrapValue(cx, &%s)) {\n"
4048+
" return false;\n"
4049+
"}" % value)
4050+
4051+
if type.isObject():
4052+
if not type.nullable():
4053+
value = "%s.Slot()" % value
4054+
else:
4055+
value = "&%s" % value
4056+
return CGGeneric("if (!JS_WrapObject(cx, %s)) {\n"
4057+
" return false;\n"
4058+
"}" % value)
4059+
4060+
if type.isSpiderMonkeyInterface():
4061+
raise TypeError("Can't handle wrapping of spidermonkey interfaces in "
4062+
"constructor arguments yet")
4063+
4064+
if type.isSequence():
4065+
if type.nullable():
4066+
type = type.inner
4067+
value = "%s.AsMutable().Value()" % value
4068+
global sequenceWrapLevel
4069+
index = "indexName%d" % sequenceWrapLevel
4070+
sequenceWrapLevel += 1
4071+
wrapElement = wrapTypeIntoCurrentCompartment(type.inner,
4072+
"%s[%s]" % (value, index))
4073+
sequenceWrapLevel -= 1
4074+
if not wrapElement:
4075+
return None
4076+
return CGWrapper(CGIndenter(wrapElement),
4077+
pre=("for (uint32_t %s = 0; %s < %s.Length(); ++%s) {\n" %
4078+
(index, index, value, index)),
4079+
post="\n}")
4080+
4081+
if type.isDictionary():
4082+
assert not type.nullable()
4083+
value = "%s.AsMutable()" % value
4084+
myDict = type.inner
4085+
memberWraps = []
4086+
while myDict:
4087+
for member in myDict.members:
4088+
memberWrap = wrapArgIntoCurrentCompartment(
4089+
member,
4090+
"%s.%s" % (value, CGDictionary.makeMemberName(member.identifier.name)))
4091+
if memberWrap:
4092+
memberWraps.append(memberWrap)
4093+
myDict = myDict.parent
4094+
return CGList(memberWraps, "\n") if len(memberWraps) != 0 else None
4095+
4096+
if type.isUnion():
4097+
raise TypeError("Can't handle wrapping of unions in constructor "
4098+
"arguments yet")
4099+
4100+
if (type.isString() or type.isPrimitive() or type.isEnum() or
4101+
type.isGeckoInterface() or type.isCallback()):
4102+
# All of these don't need wrapping
4103+
return None
4104+
4105+
raise TypeError("Unknown type; we don't know how to wrap it in constructor "
4106+
"arguments: %s" % type)
4107+
4108+
def wrapArgIntoCurrentCompartment(arg, value):
4109+
"""
4110+
As wrapTypeIntoCurrentCompartment but handles things being optional
4111+
"""
4112+
origValue = value
4113+
isOptional = arg.optional and not arg.defaultValue
4114+
if isOptional:
4115+
value = value + ".AsMutable().Value()"
4116+
wrap = wrapTypeIntoCurrentCompartment(arg.type, value)
4117+
if wrap and isOptional:
4118+
wrap = CGIfWrapper(wrap, "%s.WasPassed()" % origValue)
4119+
return wrap
4120+
40344121
class CGPerSignatureCall(CGThing):
40354122
"""
40364123
This class handles the guts of generating code for a particular
@@ -4056,7 +4143,7 @@ class CGPerSignatureCall(CGThing):
40564143

40574144
def __init__(self, returnType, arguments, nativeMethodName, static,
40584145
descriptor, idlNode, argConversionStartsAt=0, getter=False,
4059-
setter=False):
4146+
setter=False, isConstructor=False):
40604147
assert idlNode.isMethod() == (not getter and not setter)
40614148
assert idlNode.isAttr() == (getter or setter)
40624149

@@ -4117,6 +4204,30 @@ def __init__(self, returnType, arguments, nativeMethodName, static,
41174204
lenientFloatCode=lenientFloatCode) for
41184205
i in range(argConversionStartsAt, self.argCount)])
41194206

4207+
if isConstructor:
4208+
# If we're called via an xray, we need to enter the underlying
4209+
# object's compartment and then wrap up all of our arguments into
4210+
# that compartment as needed. This is all happening after we've
4211+
# already done the conversions from JS values to WebIDL (C++)
4212+
# values, so we only need to worry about cases where there are 'any'
4213+
# or 'object' types, or other things that we represent as actual
4214+
# JSAPI types, present. Effectively, we're emulating a
4215+
# CrossCompartmentWrapper, but working with the C++ types, not the
4216+
# original list of JS::Values.
4217+
cgThings.append(CGGeneric("Maybe<JSAutoCompartment> ac;"))
4218+
xraySteps = [
4219+
CGGeneric("obj = js::CheckedUnwrap(obj);\n"
4220+
"if (!obj) {\n"
4221+
" return false;\n"
4222+
"}\n"
4223+
"ac.construct(cx, obj);") ]
4224+
xraySteps.extend(
4225+
wrapArgIntoCurrentCompartment(arg, argname)
4226+
for (arg, argname) in self.getArguments())
4227+
cgThings.append(
4228+
CGIfWrapper(CGList(xraySteps, "\n"),
4229+
"xpc::WrapperFactory::IsXrayWrapper(obj)"))
4230+
41204231
cgThings.append(CGCallGenerator(
41214232
self.getErrorReport() if self.isFallible() else None,
41224233
self.getArguments(), argsPre, returnType,
@@ -4221,7 +4332,8 @@ class CGMethodCall(CGThing):
42214332
A class to generate selection of a method signature from a set of
42224333
signatures and generation of a call to that signature.
42234334
"""
4224-
def __init__(self, nativeMethodName, static, descriptor, method):
4335+
def __init__(self, nativeMethodName, static, descriptor, method,
4336+
isConstructor=False):
42254337
CGThing.__init__(self)
42264338

42274339
methodName = '"%s.%s"' % (descriptor.interface.identifier.name, method.identifier.name)
@@ -4238,7 +4350,9 @@ def requiredArgCount(signature):
42384350
def getPerSignatureCall(signature, argConversionStartsAt=0):
42394351
return CGPerSignatureCall(signature[0], signature[1],
42404352
nativeMethodName, static, descriptor,
4241-
method, argConversionStartsAt)
4353+
method,
4354+
argConversionStartsAt=argConversionStartsAt,
4355+
isConstructor=isConstructor)
42424356

42434357

42444358
signatures = method.signatures()
@@ -7086,6 +7200,10 @@ def declare(self):
70867200
" NS_ENSURE_TRUE(cx, false);\n"
70877201
" return Init(cx, json.ref());\n"
70887202
" }\n" if not self.workers else "") +
7203+
" ${selfName}& AsMutable() const\n"
7204+
" {\n"
7205+
" return *const_cast<${selfName}*>(this);\n"
7206+
" }\n"
70897207
"\n" +
70907208
"\n".join(memberDecls) + "\n"
70917209
"private:\n"

dom/bindings/Configuration.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,7 @@ def getTypesFromDescriptor(descriptor):
475475
members = [m for m in descriptor.interface.members]
476476
if descriptor.interface.ctor():
477477
members.append(descriptor.interface.ctor())
478+
members.extend(descriptor.interface.namedConstructors)
478479
signatures = [s for m in members if m.isMethod() for s in m.signatures()]
479480
types = []
480481
for s in signatures:

dom/bindings/Nullable.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ struct Nullable
6464
return mIsNull;
6565
}
6666

67+
Nullable& AsMutable() const {
68+
return *const_cast<Nullable*>(this);
69+
}
70+
6771
// Make it possible to use a const Nullable of an array type with other
6872
// array types.
6973
template<typename U>

dom/bindings/test/TestBindingHeader.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,18 @@ class TestInterface : public nsISupports,
132132
static
133133
already_AddRefed<TestInterface> Test(const GlobalObject&, const nsAString&,
134134
ErrorResult&);
135+
static
136+
already_AddRefed<TestInterface> Test2(const GlobalObject&,
137+
JSContext*,
138+
const DictForConstructor&,
139+
JS::Value,
140+
JSObject&,
141+
JSObject*,
142+
const Sequence<Dict>&,
143+
const Optional<JS::Value>&,
144+
const Optional<NonNull<JSObject> >&,
145+
const Optional<JSObject*>&,
146+
ErrorResult&);
135147

136148
// Integer types
137149
int8_t ReadonlyByte();

dom/bindings/test/TestCodeGen.webidl

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,10 @@ interface OnlyForUseInConstructor {
103103
Constructor(long arg1, IndirectlyImplementedInterface iface),
104104
// Constructor(long arg1, long arg2, (TestInterface or OnlyForUseInConstructor) arg3),
105105
NamedConstructor=Test,
106-
NamedConstructor=Test(DOMString str)
106+
NamedConstructor=Test(DOMString str),
107+
NamedConstructor=Test2(DictForConstructor dict, any any1, object obj1,
108+
object? obj2, sequence<Dict> seq, optional any any2,
109+
optional object obj3, optional object? obj4)
107110
]
108111
interface TestInterface {
109112
// Integer types
@@ -631,6 +634,7 @@ dictionary ParentDict : GrandparentDict {
631634
long c = 5;
632635
TestInterface someInterface;
633636
TestExternalInterface someExternalInterface;
637+
any parentAny;
634638
};
635639

636640
dictionary DictContainingDict {
@@ -642,6 +646,18 @@ dictionary DictContainingSequence {
642646
sequence<TestInterface> ourSequence2;
643647
};
644648

649+
dictionary DictForConstructor {
650+
Dict dict;
651+
DictContainingDict dict2;
652+
sequence<Dict> seq1;
653+
sequence<sequence<Dict>>? seq2;
654+
sequence<sequence<Dict>?> seq3;
655+
// No support for sequences of "any" or "object" as return values yet.
656+
object obj1;
657+
object? obj2;
658+
any any1 = null;
659+
};
660+
645661
interface TestIndexedGetterInterface {
646662
getter long item(unsigned long idx);
647663
readonly attribute unsigned long length;

dom/tests/mochitest/chrome/Makefile.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ MOCHITEST_CHROME_FILES = \
6262
test_queryCaretRect.html \
6363
queryCaretRectWin.html \
6464
queryCaretRectUnix.html \
65+
test_xray_event_constructor.xul \
6566
$(NULL)
6667

6768
ifeq (WINNT,$(OS_ARCH))

dom/tests/mochitest/chrome/test_sandbox_bindings.xul

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,14 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=741267
233233
ok(false, "Should be able to return NodeFilter from a sandbox " + e);
234234
}
235235
236+
try {
237+
var eventCtor = Components.utils.evalInSandbox("Event", sandbox);
238+
var e = new eventCtor("test", { bubbles: true });
239+
is(e.bubbles, true, "Dictionary argument should work");
240+
} catch (e) {
241+
ok(false, "Should be able to construct my event " + e);
242+
}
243+
236244
SimpleTest.finish();
237245
}
238246
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?xml version="1.0"?>
2+
<?xml-stylesheet type="text/css" href="chrome://global/skin"?>
3+
<?xml-stylesheet type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"?>
4+
<!--
5+
https://bugzilla.mozilla.org/show_bug.cgi?id=861493
6+
-->
7+
<window title="Mozilla Bug 861493"
8+
xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
9+
<script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
10+
11+
<iframe id="t" type="content"></iframe>
12+
13+
<!-- test results are displayed in the html:body -->
14+
<body xmlns="http://www.w3.org/1999/xhtml">
15+
<a href="https://bugzilla.mozilla.org/show_bug.cgi?id=861493"
16+
target="_blank">Mozilla Bug 861493</a>
17+
</body>
18+
19+
<!-- test code goes here -->
20+
<script type="application/javascript">
21+
<![CDATA[
22+
/** Test for Bug 861493 **/
23+
SimpleTest.waitForExplicitFinish();
24+
25+
addLoadEvent(function() {
26+
ok(Components.utils.isXrayWrapper($("t").contentWindow),
27+
"Should have xray");
28+
var e = new $("t").contentWindow.Event("test", { bubbles: true });
29+
is(e.bubbles, true, "Dictionary should have worked");
30+
SimpleTest.finish();
31+
})
32+
33+
]]>
34+
</script>
35+
</window>

0 commit comments

Comments
 (0)