From 7041704c76a523953fa2c7237a4f94f92d47f051 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 25 Apr 2013 19:03:05 -0400 Subject: [PATCH] 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. --- dom/bindings/BindingDeclarations.h | 15 +++ dom/bindings/BindingUtils.h | 11 ++ dom/bindings/Codegen.py | 126 +++++++++++++++++- dom/bindings/Configuration.py | 1 + dom/bindings/Nullable.h | 4 + dom/bindings/test/TestBindingHeader.h | 12 ++ dom/bindings/test/TestCodeGen.webidl | 18 ++- dom/tests/mochitest/chrome/Makefile.in | 1 + .../chrome/test_sandbox_bindings.xul | 8 ++ .../chrome/test_xray_event_constructor.xul | 35 +++++ js/src/jsfriendapi.cpp | 6 + js/src/jsfriendapi.h | 3 + js/xpconnect/wrappers/XrayWrapper.cpp | 81 ++++++----- 13 files changed, 286 insertions(+), 35 deletions(-) create mode 100644 dom/tests/mochitest/chrome/test_xray_event_constructor.xul diff --git a/dom/bindings/BindingDeclarations.h b/dom/bindings/BindingDeclarations.h index 3f373b0737c..f222de7d384 100644 --- a/dom/bindings/BindingDeclarations.h +++ b/dom/bindings/BindingDeclarations.h @@ -265,6 +265,11 @@ public: return mImpl.ref(); } + Optional& AsMutable() const + { + return *const_cast(this); + } + // If we ever decide to add conversion operators for optional arrays // like the ones Nullable has, we'll need to ensure that Maybe<> has // the boolean before the actual data. @@ -379,6 +384,16 @@ public: return mValue; } + JS::Value* operator&() + { + return &mValue; + } + + const JS::Value* operator&() const + { + return &mValue; + } + private: // Don't allow copy-construction of these objects, because it'll do the wrong // thing with our flag mCx. diff --git a/dom/bindings/BindingUtils.h b/dom/bindings/BindingUtils.h index 23e4e0ce77a..38dc0fe4a24 100644 --- a/dom/bindings/BindingUtils.h +++ b/dom/bindings/BindingUtils.h @@ -1431,6 +1431,11 @@ public: MOZ_ASSERT(!empty() && ref(), "Can not alias null."); return *ref(); } + + JSObject** Slot() { // To make us look like a NonNull + // Assert if we're empty, on purpose + return ref().address(); + } }; class LazyRootedObject : public Maybe @@ -1439,6 +1444,12 @@ public: operator JSObject*() const { return empty() ? (JSObject*) nullptr : ref(); } + + JSObject** operator&() + { + // Assert if we're empty, on purpose + return ref().address(); + } }; // A struct that has the same layout as an nsDependentString but much diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index eaedb1a1700..41e74555573 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -1080,7 +1080,7 @@ class CGClassConstructor(CGAbstractStaticMethod): name = self._ctor.identifier.name nativeName = MakeNativeName(self.descriptor.binaryNames.get(name, name)) callGenerator = CGMethodCall(nativeName, True, self.descriptor, - self._ctor) + self._ctor, isConstructor=True) return preamble + callGenerator.define(); class CGClassConstructHookHolder(CGGeneric): @@ -4031,6 +4031,93 @@ class MethodNotCreatorError(Exception): def __init__(self, typename): self.typename = typename +# A counter for making sure that when we're wrapping up things in +# nested sequences we don't use the same variable name to iterate over +# different sequences. +sequenceWrapLevel = 0 + +def wrapTypeIntoCurrentCompartment(type, value): + """ + Take the thing named by "value" and if it contains "any", + "object", or spidermonkey-interface types inside return a CGThing + that will wrap them into the current compartment. + """ + if type.isAny(): + assert not type.nullable() + return CGGeneric("if (!JS_WrapValue(cx, &%s)) {\n" + " return false;\n" + "}" % value) + + if type.isObject(): + if not type.nullable(): + value = "%s.Slot()" % value + else: + value = "&%s" % value + return CGGeneric("if (!JS_WrapObject(cx, %s)) {\n" + " return false;\n" + "}" % value) + + if type.isSpiderMonkeyInterface(): + raise TypeError("Can't handle wrapping of spidermonkey interfaces in " + "constructor arguments yet") + + if type.isSequence(): + if type.nullable(): + type = type.inner + value = "%s.AsMutable().Value()" % value + global sequenceWrapLevel + index = "indexName%d" % sequenceWrapLevel + sequenceWrapLevel += 1 + wrapElement = wrapTypeIntoCurrentCompartment(type.inner, + "%s[%s]" % (value, index)) + sequenceWrapLevel -= 1 + if not wrapElement: + return None + return CGWrapper(CGIndenter(wrapElement), + pre=("for (uint32_t %s = 0; %s < %s.Length(); ++%s) {\n" % + (index, index, value, index)), + post="\n}") + + if type.isDictionary(): + assert not type.nullable() + value = "%s.AsMutable()" % value + myDict = type.inner + memberWraps = [] + while myDict: + for member in myDict.members: + memberWrap = wrapArgIntoCurrentCompartment( + member, + "%s.%s" % (value, CGDictionary.makeMemberName(member.identifier.name))) + if memberWrap: + memberWraps.append(memberWrap) + myDict = myDict.parent + return CGList(memberWraps, "\n") if len(memberWraps) != 0 else None + + if type.isUnion(): + raise TypeError("Can't handle wrapping of unions in constructor " + "arguments yet") + + if (type.isString() or type.isPrimitive() or type.isEnum() or + type.isGeckoInterface() or type.isCallback()): + # All of these don't need wrapping + return None + + raise TypeError("Unknown type; we don't know how to wrap it in constructor " + "arguments: %s" % type) + +def wrapArgIntoCurrentCompartment(arg, value): + """ + As wrapTypeIntoCurrentCompartment but handles things being optional + """ + origValue = value + isOptional = arg.optional and not arg.defaultValue + if isOptional: + value = value + ".AsMutable().Value()" + wrap = wrapTypeIntoCurrentCompartment(arg.type, value) + if wrap and isOptional: + wrap = CGIfWrapper(wrap, "%s.WasPassed()" % origValue) + return wrap + class CGPerSignatureCall(CGThing): """ This class handles the guts of generating code for a particular @@ -4056,7 +4143,7 @@ class CGPerSignatureCall(CGThing): def __init__(self, returnType, arguments, nativeMethodName, static, descriptor, idlNode, argConversionStartsAt=0, getter=False, - setter=False): + setter=False, isConstructor=False): assert idlNode.isMethod() == (not getter and not setter) assert idlNode.isAttr() == (getter or setter) @@ -4117,6 +4204,30 @@ if (global.Failed()) { lenientFloatCode=lenientFloatCode) for i in range(argConversionStartsAt, self.argCount)]) + if isConstructor: + # If we're called via an xray, we need to enter the underlying + # object's compartment and then wrap up all of our arguments into + # that compartment as needed. This is all happening after we've + # already done the conversions from JS values to WebIDL (C++) + # values, so we only need to worry about cases where there are 'any' + # or 'object' types, or other things that we represent as actual + # JSAPI types, present. Effectively, we're emulating a + # CrossCompartmentWrapper, but working with the C++ types, not the + # original list of JS::Values. + cgThings.append(CGGeneric("Maybe ac;")) + xraySteps = [ + CGGeneric("obj = js::CheckedUnwrap(obj);\n" + "if (!obj) {\n" + " return false;\n" + "}\n" + "ac.construct(cx, obj);") ] + xraySteps.extend( + wrapArgIntoCurrentCompartment(arg, argname) + for (arg, argname) in self.getArguments()) + cgThings.append( + CGIfWrapper(CGList(xraySteps, "\n"), + "xpc::WrapperFactory::IsXrayWrapper(obj)")) + cgThings.append(CGCallGenerator( self.getErrorReport() if self.isFallible() else None, self.getArguments(), argsPre, returnType, @@ -4221,7 +4332,8 @@ class CGMethodCall(CGThing): A class to generate selection of a method signature from a set of signatures and generation of a call to that signature. """ - def __init__(self, nativeMethodName, static, descriptor, method): + def __init__(self, nativeMethodName, static, descriptor, method, + isConstructor=False): CGThing.__init__(self) methodName = '"%s.%s"' % (descriptor.interface.identifier.name, method.identifier.name) @@ -4238,7 +4350,9 @@ class CGMethodCall(CGThing): def getPerSignatureCall(signature, argConversionStartsAt=0): return CGPerSignatureCall(signature[0], signature[1], nativeMethodName, static, descriptor, - method, argConversionStartsAt) + method, + argConversionStartsAt=argConversionStartsAt, + isConstructor=isConstructor) signatures = method.signatures() @@ -7086,6 +7200,10 @@ class CGDictionary(CGThing): " NS_ENSURE_TRUE(cx, false);\n" " return Init(cx, json.ref());\n" " }\n" if not self.workers else "") + + " ${selfName}& AsMutable() const\n" + " {\n" + " return *const_cast<${selfName}*>(this);\n" + " }\n" "\n" + "\n".join(memberDecls) + "\n" "private:\n" diff --git a/dom/bindings/Configuration.py b/dom/bindings/Configuration.py index 4ecd958dc98..f2bae117c11 100644 --- a/dom/bindings/Configuration.py +++ b/dom/bindings/Configuration.py @@ -475,6 +475,7 @@ def getTypesFromDescriptor(descriptor): members = [m for m in descriptor.interface.members] if descriptor.interface.ctor(): members.append(descriptor.interface.ctor()) + members.extend(descriptor.interface.namedConstructors) signatures = [s for m in members if m.isMethod() for s in m.signatures()] types = [] for s in signatures: diff --git a/dom/bindings/Nullable.h b/dom/bindings/Nullable.h index 5fb57392941..e265cbf1d78 100644 --- a/dom/bindings/Nullable.h +++ b/dom/bindings/Nullable.h @@ -64,6 +64,10 @@ public: return mIsNull; } + Nullable& AsMutable() const { + return *const_cast(this); + } + // Make it possible to use a const Nullable of an array type with other // array types. template diff --git a/dom/bindings/test/TestBindingHeader.h b/dom/bindings/test/TestBindingHeader.h index 2bd2c5658ae..f936e49e2fe 100644 --- a/dom/bindings/test/TestBindingHeader.h +++ b/dom/bindings/test/TestBindingHeader.h @@ -132,6 +132,18 @@ public: static already_AddRefed Test(const GlobalObject&, const nsAString&, ErrorResult&); + static + already_AddRefed Test2(const GlobalObject&, + JSContext*, + const DictForConstructor&, + JS::Value, + JSObject&, + JSObject*, + const Sequence&, + const Optional&, + const Optional >&, + const Optional&, + ErrorResult&); // Integer types int8_t ReadonlyByte(); diff --git a/dom/bindings/test/TestCodeGen.webidl b/dom/bindings/test/TestCodeGen.webidl index da4191b9480..e894d70188b 100644 --- a/dom/bindings/test/TestCodeGen.webidl +++ b/dom/bindings/test/TestCodeGen.webidl @@ -103,7 +103,10 @@ interface OnlyForUseInConstructor { Constructor(long arg1, IndirectlyImplementedInterface iface), // Constructor(long arg1, long arg2, (TestInterface or OnlyForUseInConstructor) arg3), NamedConstructor=Test, - NamedConstructor=Test(DOMString str) + NamedConstructor=Test(DOMString str), + NamedConstructor=Test2(DictForConstructor dict, any any1, object obj1, + object? obj2, sequence seq, optional any any2, + optional object obj3, optional object? obj4) ] interface TestInterface { // Integer types @@ -631,6 +634,7 @@ dictionary ParentDict : GrandparentDict { long c = 5; TestInterface someInterface; TestExternalInterface someExternalInterface; + any parentAny; }; dictionary DictContainingDict { @@ -642,6 +646,18 @@ dictionary DictContainingSequence { sequence ourSequence2; }; +dictionary DictForConstructor { + Dict dict; + DictContainingDict dict2; + sequence seq1; + sequence>? seq2; + sequence?> seq3; + // No support for sequences of "any" or "object" as return values yet. + object obj1; + object? obj2; + any any1 = null; +}; + interface TestIndexedGetterInterface { getter long item(unsigned long idx); readonly attribute unsigned long length; diff --git a/dom/tests/mochitest/chrome/Makefile.in b/dom/tests/mochitest/chrome/Makefile.in index 4bdb0f44b43..7e4bf785ba1 100644 --- a/dom/tests/mochitest/chrome/Makefile.in +++ b/dom/tests/mochitest/chrome/Makefile.in @@ -62,6 +62,7 @@ MOCHITEST_CHROME_FILES = \ test_queryCaretRect.html \ queryCaretRectWin.html \ queryCaretRectUnix.html \ + test_xray_event_constructor.xul \ $(NULL) ifeq (WINNT,$(OS_ARCH)) diff --git a/dom/tests/mochitest/chrome/test_sandbox_bindings.xul b/dom/tests/mochitest/chrome/test_sandbox_bindings.xul index 09ef60d2a33..fdbbb0edf60 100644 --- a/dom/tests/mochitest/chrome/test_sandbox_bindings.xul +++ b/dom/tests/mochitest/chrome/test_sandbox_bindings.xul @@ -233,6 +233,14 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=741267 ok(false, "Should be able to return NodeFilter from a sandbox " + e); } + try { + var eventCtor = Components.utils.evalInSandbox("Event", sandbox); + var e = new eventCtor("test", { bubbles: true }); + is(e.bubbles, true, "Dictionary argument should work"); + } catch (e) { + ok(false, "Should be able to construct my event " + e); + } + SimpleTest.finish(); } diff --git a/dom/tests/mochitest/chrome/test_xray_event_constructor.xul b/dom/tests/mochitest/chrome/test_xray_event_constructor.xul new file mode 100644 index 00000000000..f60dc3be95a --- /dev/null +++ b/dom/tests/mochitest/chrome/test_xray_event_constructor.xul @@ -0,0 +1,35 @@ + + + + + + + diff --git a/js/src/jsfriendapi.cpp b/js/src/jsfriendapi.cpp index 990ffe8cb3d..f31fe5fe8c8 100644 --- a/js/src/jsfriendapi.cpp +++ b/js/src/jsfriendapi.cpp @@ -1057,3 +1057,9 @@ js_DefineOwnProperty(JSContext *cx, JSObject *objArg, jsid idArg, return js_DefineOwnProperty(cx, HandleObject(obj), id, descriptor, bp); } + +JS_FRIEND_API(JSBool) +js_ReportIsNotFunction(JSContext *cx, const JS::Value& v) +{ + return ReportIsNotFunction(cx, v); +} diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index 2f9b5381ed0..71c5f9fcc56 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -1454,4 +1454,7 @@ extern JS_FRIEND_API(JSBool) js_DefineOwnProperty(JSContext *cx, JSObject *objArg, jsid idArg, const js::PropertyDescriptor& descriptor, JSBool *bp); +extern JS_FRIEND_API(JSBool) +js_ReportIsNotFunction(JSContext *cx, const JS::Value& v); + #endif /* jsfriendapi_h___ */ diff --git a/js/xpconnect/wrappers/XrayWrapper.cpp b/js/xpconnect/wrappers/XrayWrapper.cpp index 0203b57a474..5f809b2d28c 100644 --- a/js/xpconnect/wrappers/XrayWrapper.cpp +++ b/js/xpconnect/wrappers/XrayWrapper.cpp @@ -146,11 +146,13 @@ public: HandleObject wrapper, HandleObject holder, HandleId id, JSPropertyDescriptor *desc, unsigned flags); - static bool call(JSContext *cx, HandleObject wrapper, const JS::CallArgs &args) + static bool call(JSContext *cx, HandleObject wrapper, + const JS::CallArgs &args, js::Wrapper& baseInstance) { MOZ_NOT_REACHED("Call trap currently implemented only for XPCWNs"); } - static bool construct(JSContext *cx, HandleObject wrapper, const JS::CallArgs &args) + static bool construct(JSContext *cx, HandleObject wrapper, + const JS::CallArgs &args, js::Wrapper& baseInstance) { MOZ_NOT_REACHED("Call trap currently implemented only for XPCWNs"); } @@ -198,8 +200,10 @@ public: Handle existingDesc, bool *defined); static bool enumerateNames(JSContext *cx, HandleObject wrapper, unsigned flags, AutoIdVector &props); - static bool call(JSContext *cx, HandleObject wrapper, const JS::CallArgs &args); - static bool construct(JSContext *cx, HandleObject wrapper, const JS::CallArgs &args); + static bool call(JSContext *cx, HandleObject wrapper, + const JS::CallArgs &args, js::Wrapper& baseInstance); + static bool construct(JSContext *cx, HandleObject wrapper, + const JS::CallArgs &args, js::Wrapper& baseInstance); static bool isResolving(JSContext *cx, JSObject *holder, jsid id); @@ -242,8 +246,10 @@ public: Handle existingDesc, bool *defined); static bool enumerateNames(JSContext *cx, HandleObject wrapper, unsigned flags, AutoIdVector &props); - static bool call(JSContext *cx, HandleObject wrapper, const JS::CallArgs &args); - static bool construct(JSContext *cx, HandleObject wrapper, const JS::CallArgs &args); + static bool call(JSContext *cx, HandleObject wrapper, + const JS::CallArgs &args, js::Wrapper& baseInstance); + static bool construct(JSContext *cx, HandleObject wrapper, + const JS::CallArgs &args, js::Wrapper& baseInstance); static bool isResolving(JSContext *cx, JSObject *holder, jsid id) { @@ -1128,7 +1134,8 @@ XPCWrappedNativeXrayTraits::createHolder(JSContext *cx, JSObject *wrapper) bool XPCWrappedNativeXrayTraits::call(JSContext *cx, HandleObject wrapper, - const JS::CallArgs &args) + const JS::CallArgs &args, + js::Wrapper& baseInstance) { // Run the resolve hook of the wrapped native. XPCWrappedNative *wn = getWN(wrapper); @@ -1153,7 +1160,8 @@ XPCWrappedNativeXrayTraits::call(JSContext *cx, HandleObject wrapper, bool XPCWrappedNativeXrayTraits::construct(JSContext *cx, HandleObject wrapper, - const JS::CallArgs &args) + const JS::CallArgs &args, + js::Wrapper& baseInstance) { // Run the resolve hook of the wrapped native. XPCWrappedNative *wn = getWN(wrapper); @@ -1237,42 +1245,55 @@ DOMXrayTraits::enumerateNames(JSContext *cx, HandleObject wrapper, unsigned flag } bool -DOMXrayTraits::call(JSContext *cx, HandleObject wrapper, const JS::CallArgs &args) +DOMXrayTraits::call(JSContext *cx, HandleObject wrapper, + const JS::CallArgs &args, js::Wrapper& baseInstance) { RootedObject obj(cx, getTargetObject(wrapper)); - { - JSAutoCompartment ac(cx, obj); - RootedValue thisv(cx, args.thisv()); - if (!JS_WrapValue(cx, thisv.address())) + js::Class* clasp = js::GetObjectClass(obj); + // What we have is either a WebIDL interface object, a WebIDL prototype + // object, or a WebIDL instance object. WebIDL prototype objects never have + // a clasp->call. WebIDL interface objects we want to invoke on the xray + // compartment. WebIDL instance objects either don't have a clasp->call or + // are using "legacycaller", which basically means plug-ins. We want to + // call those on the content compartment. + if (clasp->flags & JSCLASS_IS_DOMIFACEANDPROTOJSCLASS) { + if (!clasp->call) { + js_ReportIsNotFunction(cx, JS::ObjectValue(*wrapper)); return false; - args.setThis(thisv); - for (size_t i = 0; i < args.length(); ++i) { - if (!JS_WrapValue(cx, &args[i])) - return false; } - if (!Call(cx, thisv, obj, args.length(), args.array(), args.rval().address())) + // call it on the Xray compartment + if (!clasp->call(cx, args.length(), args.base())) + return false; + } else { + // This is only reached for WebIDL instance objects, and in practice + // only for plugins. Just call them on the content compartment. + if (!baseInstance.call(cx, wrapper, args)) return false; } return JS_WrapValue(cx, args.rval().address()); } bool -DOMXrayTraits::construct(JSContext *cx, HandleObject wrapper, const JS::CallArgs &args) +DOMXrayTraits::construct(JSContext *cx, HandleObject wrapper, + const JS::CallArgs &args, js::Wrapper& baseInstance) { RootedObject obj(cx, getTargetObject(wrapper)); MOZ_ASSERT(mozilla::dom::HasConstructor(obj)); - RootedObject newObj(cx); - { - JSAutoCompartment ac(cx, obj); - for (size_t i = 0; i < args.length(); ++i) { - if (!JS_WrapValue(cx, &args[i])) - return false; + js::Class* clasp = js::GetObjectClass(obj); + // See comments in DOMXrayTraits::call() explaining what's going on here. + if (clasp->flags & JSCLASS_IS_DOMIFACEANDPROTOJSCLASS) { + if (!clasp->construct) { + js_ReportIsNotFunction(cx, JS::ObjectValue(*wrapper)); + return false; } - newObj = JS_New(cx, obj, args.length(), args.array()); + if (!clasp->construct(cx, args.length(), args.base())) + return false; + } else { + if (!baseInstance.construct(cx, wrapper, args)) + return false; } - if (!newObj || !JS_WrapObject(cx, newObj.address())) + if (!args.rval().isObject() || !JS_WrapValue(cx, args.rval().address())) return false; - args.rval().setObject(*newObj); return true; } @@ -1854,7 +1875,7 @@ bool XrayWrapper::call(JSContext *cx, HandleObject wrapper, const JS::CallArgs &args) { assertEnteredPolicy(cx, wrapper, JSID_VOID); - return Traits::call(cx, wrapper, args); + return Traits::call(cx, wrapper, args, Base::singleton); } template @@ -1862,7 +1883,7 @@ bool XrayWrapper::construct(JSContext *cx, HandleObject wrapper, const JS::CallArgs &args) { assertEnteredPolicy(cx, wrapper, JSID_VOID); - return Traits::construct(cx, wrapper, args); + return Traits::construct(cx, wrapper, args, Base::singleton); } /*