From 8ec9f238d924544da458e4d1ffee8754aa636414 Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Thu, 18 Sep 2014 12:30:38 -0500 Subject: [PATCH] Bug 1082672, part 4 - Change XrayWrapper code to be able to resolve symbol-keyed methods. r=bz, r=bholley. --HG-- extra : rebase_source : f78cbb83f63dfffd648c6d3c280273f4a61c9fe8 extra : amend_source : f006a096174eee166125430753e65e9a31bd930b --- dom/bindings/BindingUtils.cpp | 3 + js/src/jsapi.cpp | 39 +++++++- js/src/jsapi.h | 3 + js/src/vm/Runtime.h | 10 +- js/xpconnect/tests/chrome/test_xrayToJS.xul | 104 ++++++++++++++------ js/xpconnect/wrappers/XrayWrapper.cpp | 39 ++++---- 6 files changed, 144 insertions(+), 54 deletions(-) diff --git a/dom/bindings/BindingUtils.cpp b/dom/bindings/BindingUtils.cpp index 8dfbf4acf9ec..09e8e30d1e2d 100644 --- a/dom/bindings/BindingUtils.cpp +++ b/dom/bindings/BindingUtils.cpp @@ -1406,8 +1406,11 @@ XrayAttributeOrMethodKeys(JSContext* cx, JS::Handle wrapper, // looking at now. size_t i = list->specs - specList; for ( ; ids[i] != JSID_VOID; ++i) { + // Skip non-enumerable properties and symbol-keyed properties unless + // they are specially requested via flags. if (((flags & JSITER_HIDDEN) || (specList[i].flags & JSPROP_ENUMERATE)) && + ((flags & JSITER_SYMBOLS) || !JSID_IS_SYMBOL(ids[i])) && !props.append(ids[i])) { return false; } diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 30f8f13f4d65..39b6e4014277 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -3244,13 +3244,21 @@ JS_DefineConstIntegers(JSContext *cx, HandleObject obj, const JSConstIntegerSpec return DefineConstScalar(cx, obj, cis); } +static JS::SymbolCode +PropertySpecNameToSymbolCode(const char *name) +{ + MOZ_ASSERT(JS::PropertySpecNameIsSymbol(name)); + uintptr_t u = reinterpret_cast(name); + return JS::SymbolCode(u - 1); +} + static bool PropertySpecNameToId(JSContext *cx, const char *name, MutableHandleId id, js::InternBehavior ib = js::DoNotInternAtom) { if (JS::PropertySpecNameIsSymbol(name)) { - uintptr_t u = reinterpret_cast(name); - id.set(SYMBOL_TO_JSID(cx->wellKnownSymbols().get(u - 1))); + JS::SymbolCode which = PropertySpecNameToSymbolCode(name); + id.set(SYMBOL_TO_JSID(cx->wellKnownSymbols().get(which))); } else { JSAtom *atom = Atomize(cx, name, strlen(name), ib); if (!atom) @@ -5516,6 +5524,33 @@ JS::GetWellKnownSymbol(JSContext *cx, JS::SymbolCode which) return cx->runtime()->wellKnownSymbols->get(uint32_t(which)); } +static bool +PropertySpecNameIsDigits(const char *s) { + if (JS::PropertySpecNameIsSymbol(s)) + return false; + if (!*s) + return false; + for (; *s; s++) { + if (*s < '0' || *s > '9') + return false; + } + return true; +} + +JS_PUBLIC_API(bool) +JS::PropertySpecNameEqualsId(const char *name, HandleId id) +{ + if (JS::PropertySpecNameIsSymbol(name)) { + if (!JSID_IS_SYMBOL(id)) + return false; + Symbol *sym = JSID_TO_SYMBOL(id); + return sym->isWellKnownSymbol() && sym->code() == PropertySpecNameToSymbolCode(name); + } + + MOZ_ASSERT(!PropertySpecNameIsDigits(name)); + return JSID_IS_ATOM(id) && JS_FlatStringEqualsAscii(JSID_TO_ATOM(id), name); +} + JS_PUBLIC_API(bool) JS_Stringify(JSContext *cx, MutableHandleValue vp, HandleObject replacer, HandleValue space, JSONWriteCallback callback, void *data) diff --git a/js/src/jsapi.h b/js/src/jsapi.h index a49b10936f06..50676f8a6040 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -4517,6 +4517,9 @@ PropertySpecNameIsSymbol(const char *name) return u != 0 && u - 1 < WellKnownSymbolLimit; } +JS_PUBLIC_API(bool) +PropertySpecNameEqualsId(const char *name, HandleId id); + /* * Create a jsid that does not need to be marked for GC. * diff --git a/js/src/vm/Runtime.h b/js/src/vm/Runtime.h index 711bbd3ca1e2..e152394b8aee 100644 --- a/js/src/vm/Runtime.h +++ b/js/src/vm/Runtime.h @@ -423,10 +423,14 @@ struct WellKnownSymbols { js::ImmutableSymbolPtr iterator; - ImmutableSymbolPtr &get(size_t i) { - MOZ_ASSERT(i < JS::WellKnownSymbolLimit); + ImmutableSymbolPtr &get(size_t u) { + MOZ_ASSERT(u < JS::WellKnownSymbolLimit); ImmutableSymbolPtr *symbols = reinterpret_cast(this); - return symbols[i]; + return symbols[u]; + } + + ImmutableSymbolPtr &get(JS::SymbolCode code) { + return get(size_t(code)); } }; diff --git a/js/xpconnect/tests/chrome/test_xrayToJS.xul b/js/xpconnect/tests/chrome/test_xrayToJS.xul index 03ae0e5c8fae..31fe8c45b408 100644 --- a/js/xpconnect/tests/chrome/test_xrayToJS.xul +++ b/js/xpconnect/tests/chrome/test_xrayToJS.xul @@ -145,6 +145,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=933681 var version = Cc["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULAppInfo).version; var isNightlyBuild = version.endsWith("a1"); var isReleaseBuild = !version.contains("a"); + const jsHasSymbols = typeof Symbol === "function"; var gPrototypeProperties = {}; gPrototypeProperties['Date'] = ["getTime", "getTimezoneOffset", "getYear", "getFullYear", "getUTCFullYear", @@ -197,19 +198,23 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=933681 ["constructor", "toSource", "toString", "apply", "call", "bind", "isGenerator", "length", "name", "arguments", "caller"]; + // Sort an array that may contain symbols as well as strings. + function sortProperties(arr) { + function sortKey(prop) { + return typeof prop + ":" + prop.toString(); + } + arr.sort((a, b) => sortKey(a) < sortKey(b) ? -1 : +1); + } + // Sort all the lists so we don't need to mutate them later (or copy them // again to sort them). for (var c of Object.keys(gPrototypeProperties)) - gPrototypeProperties[c].sort(); + sortProperties(gPrototypeProperties[c]); function filterOut(array, props) { return array.filter(p => props.indexOf(p) == -1); } - function appendUnique(array, vals) { - filterOut(vals, array).forEach(v => array.push(v)); - } - function isTypedArrayClass(classname) { return typedArrayClasses.indexOf(classname) >= 0; } @@ -267,6 +272,12 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=933681 gPrototypeProperties[classname].filter(id => typeof id === "string").toSource(), "A property on the " + classname + " prototype has changed! You need a security audit from an XPConnect peer"); + if (jsHasSymbols) { + is(Object.getOwnPropertySymbols(localProto).map(uneval).sort().toSource(), + gPrototypeProperties[classname].filter(id => typeof id !== "string").map(uneval).sort().toSource(), + "A symbol-keyed property on the " + classname + + " prototype has been changed! You need a security audit from an XPConnect peer"); + } let protoProps = filterOut(desiredProtoProps, propsToSkip); let protoCallables = protoProps.filter(name => propertyIsGetter(localProto, name, classname) || @@ -280,6 +291,11 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=933681 testProtoCallables(protoCallables, xray, xrayProto, localProto); is(Object.getOwnPropertyNames(xrayProto).sort().toSource(), protoProps.toSource(), "getOwnPropertyNames works"); + if (jsHasSymbols) { + is(Object.getOwnPropertySymbols(xrayProto).map(uneval).sort().toSource(), + gPrototypeProperties[classname].filter(id => typeof id !== "string").map(uneval).sort().toSource(), + protoProps.toSource(), "getOwnPropertySymbols works"); + } is(xrayProto.constructor, iwin[classname], "constructor property works"); @@ -303,26 +319,35 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=933681 is(d.toLocaleString('de-DE'), d.wrappedJSObject.toLocaleString('de-DE'), "Results match"); } + var uniqueSymbol; + function testObject() { testXray('Object', Cu.unwaiveXrays(Cu.waiveXrays(iwin).Object.create(new iwin.Object())), new iwin.Object(), []); // Construct an object full of tricky things. + let symbolProps = ''; + if (jsHasSymbols) { + uniqueSymbol = iwin.eval('var uniqueSymbol = Symbol("uniqueSymbol"); uniqueSymbol'); + symbolProps = `, [uniqueSymbol]: 43, + [Symbol.for("registrySymbolProp")]: 44`; + } var trickyObject = - iwin.eval('new Object({ \ - primitiveProp: 42, objectProp: { foo: 2 }, \ - xoProp: top.location, hasOwnProperty: 10, \ - get getterProp() { return 2; }, \ - set setterProp(x) { }, \ - get getterSetterProp() { return 3; }, \ - set getterSetterProp(x) { }, \ - callableProp: function() { }, \ - nonXrayableProp: new WeakMap() })'); + iwin.eval(`new Object({ + primitiveProp: 42, objectProp: { foo: 2 }, + xoProp: top.location, hasOwnProperty: 10, + get getterProp() { return 2; }, + set setterProp(x) { }, + get getterSetterProp() { return 3; }, + set getterSetterProp(x) { }, + callableProp: function() { }, + nonXrayableProp: new WeakMap() + ${symbolProps} + })`); testTrickyObject(trickyObject); - } -function testArray() { + function testArray() { // The |length| property is generally very weird, especially with respect // to its behavior on the prototype. Array.prototype is actually an Array // instance, and therefore has a vestigial .length. But we don't want to @@ -332,18 +357,25 @@ function testArray() { let propsToSkip = ['length']; testXray('Array', new iwin.Array(20), new iwin.Array(), propsToSkip); + let symbolProps = ''; + if (jsHasSymbols) { + uniqueSymbol = iwin.eval('var uniqueSymbol = Symbol("uniqueSymbol"); uniqueSymbol'); + symbolProps = `trickyArray[uniqueSymbol] = 43; + trickyArray[Symbol.for("registrySymbolProp")] = 44;`; + } var trickyArray = - iwin.eval("var trickyArray = []; \ - trickyArray.primitiveProp = 42; \ - trickyArray.objectProp = { foo: 2 }; \ - trickyArray.xoProp = top.location; \ - trickyArray.hasOwnProperty = 10; \ - Object.defineProperty(trickyArray, 'getterProp', { get: function() { return 2; }}); \ - Object.defineProperty(trickyArray, 'setterProp', { set: function(x) {}}); \ - Object.defineProperty(trickyArray, 'getterSetterProp', { get: function() { return 3; }, set: function(x) {}}); \ - trickyArray.callableProp = function() {}; \ - trickyArray.nonXrayableProp = new WeakMap(); \ - trickyArray;"); + iwin.eval(`var trickyArray = []; + trickyArray.primitiveProp = 42; + trickyArray.objectProp = { foo: 2 }; + trickyArray.xoProp = top.location; + trickyArray.hasOwnProperty = 10; + Object.defineProperty(trickyArray, 'getterProp', { get: function() { return 2; }}); + Object.defineProperty(trickyArray, 'setterProp', { set: function(x) {}}); + Object.defineProperty(trickyArray, 'getterSetterProp', { get: function() { return 3; }, set: function(x) {}}); + trickyArray.callableProp = function() {}; + trickyArray.nonXrayableProp = new WeakMap(); + ${symbolProps} + trickyArray;`); // Test indexed access. trickyArray.wrappedJSObject[9] = "some indexed property"; @@ -366,11 +398,11 @@ function testArray() { is(trickyArray[1], undefined, "Frozen length forbids new properties"); testTrickyObject(trickyArray); -} + } -// Parts of this function are kind of specific to testing Object, but we factor -// it out so that we can re-use the trickyObject stuff on Arrays. -function testTrickyObject(trickyObject) { + // Parts of this function are kind of specific to testing Object, but we factor + // it out so that we can re-use the trickyObject stuff on Arrays. + function testTrickyObject(trickyObject) { // Make sure it looks right under the hood. is(trickyObject.wrappedJSObject.getterProp, 2, "Underlying object has getter"); @@ -382,11 +414,21 @@ function testTrickyObject(trickyObject) { expectedNames.push('length'); is(Object.getOwnPropertyNames(trickyObject).sort().toSource(), expectedNames.sort().toSource(), "getOwnPropertyNames should be filtered correctly"); + if (jsHasSymbols) { + var expectedSymbols = [Symbol.for("registrySymbolProp"), uniqueSymbol]; + is(Object.getOwnPropertySymbols(trickyObject).map(uneval).sort().toSource(), + expectedSymbols.map(uneval).sort().toSource(), + "getOwnPropertySymbols should be filtered correctly"); + } // Test that cloning uses the Xray view. var cloned = Cu.cloneInto(trickyObject, this); is(Object.getOwnPropertyNames(cloned).sort().toSource(), expectedNames.sort().toSource(), "structured clone should use the Xray view"); + if (jsHasSymbols) { + is(Object.getOwnPropertySymbols(cloned).map(uneval).sort().toSource(), + "[]", "structured cloning doesn't clone symbol-keyed properties yet"); + } // Test iteration and in-place modification. Beware of 'expando', which is the property // we placed on the xray proto. diff --git a/js/xpconnect/wrappers/XrayWrapper.cpp b/js/xpconnect/wrappers/XrayWrapper.cpp index fac44a384685..f5e67d3bd867 100644 --- a/js/xpconnect/wrappers/XrayWrapper.cpp +++ b/js/xpconnect/wrappers/XrayWrapper.cpp @@ -458,21 +458,14 @@ JSXrayTraits::resolveOwnProperty(JSContext *cx, const Wrapper &jsWrapper, return JS_IdToValue(cx, className, desc.value()); } - // Compute the property name we're looking for. Indexed array properties - // are handled above. We'll handle well-known symbols when we start - // supporting Symbol.iterator in bug 918828. - if (!JSID_IS_STRING(id)) - return true; - Rooted str(cx, JSID_TO_FLAT_STRING(id)); - // Grab the JSClass. We require all Xrayable classes to have a ClassSpec. const js::Class *clasp = js::GetObjectClass(target); MOZ_ASSERT(clasp->spec.defined()); - // Scan through the functions. + // Scan through the functions. Indexed array properties are handled above. const JSFunctionSpec *fsMatch = nullptr; for (const JSFunctionSpec *fs = clasp->spec.prototypeFunctions; fs && fs->name; ++fs) { - if (JS_FlatStringEqualsAscii(str, fs->name)) { + if (PropertySpecNameEqualsId(fs->name, id)) { fsMatch = fs; break; } @@ -501,7 +494,7 @@ JSXrayTraits::resolveOwnProperty(JSContext *cx, const Wrapper &jsWrapper, // Scan through the properties. const JSPropertySpec *psMatch = nullptr; for (const JSPropertySpec *ps = clasp->spec.prototypeProperties; ps && ps->name; ++ps) { - if (JS_FlatStringEqualsAscii(str, ps->name)) { + if (PropertySpecNameEqualsId(ps->name, id)) { psMatch = ps; break; } @@ -632,6 +625,15 @@ JSXrayTraits::defineProperty(JSContext *cx, HandleObject wrapper, HandleId id, return true; } +static bool +MaybeAppend(jsid id, unsigned flags, AutoIdVector &props) +{ + MOZ_ASSERT(!(flags & JSITER_SYMBOLSONLY)); + if (!(flags & JSITER_SYMBOLS) && JSID_IS_SYMBOL(id)) + return true; + return props.append(id); +} + bool JSXrayTraits::enumerateNames(JSContext *cx, HandleObject wrapper, unsigned flags, AutoIdVector &props) @@ -711,12 +713,12 @@ JSXrayTraits::enumerateNames(JSContext *cx, HandleObject wrapper, unsigned flags const js::Class *clasp = js::GetObjectClass(target); MOZ_ASSERT(clasp->spec.defined()); - // Intern all the strings, and pass theme to the caller. + // Convert the method and property names to jsids and pass them to the caller. for (const JSFunctionSpec *fs = clasp->spec.prototypeFunctions; fs && fs->name; ++fs) { - RootedString str(cx, JS_InternString(cx, fs->name)); - if (!str) + jsid id; + if (!PropertySpecNameToPermanentId(cx, fs->name, &id)) return false; - if (!props.append(INTERNED_STRING_TO_JSID(cx, str))) + if (!MaybeAppend(id, flags, props)) return false; } for (const JSPropertySpec *ps = clasp->spec.prototypeProperties; ps && ps->name; ++ps) { @@ -727,10 +729,11 @@ JSXrayTraits::enumerateNames(JSContext *cx, HandleObject wrapper, unsigned flags MOZ_ASSERT(ps->flags & JSPROP_NATIVE_ACCESSORS, "Self-hosted accessor added to Xrayable class - ping the XPConnect " "module owner about adding test coverage"); - RootedString str(cx, JS_InternString(cx, ps->name)); - if (!str) + + jsid id; + if (!PropertySpecNameToPermanentId(cx, ps->name, &id)) return false; - if (!props.append(INTERNED_STRING_TO_JSID(cx, str))) + if (!MaybeAppend(id, flags, props)) return false; } @@ -1978,7 +1981,7 @@ XrayWrapper::ownPropertyKeys(JSContext *cx, HandleObject wrapper, AutoIdVector &props) const { assertEnteredPolicy(cx, wrapper, JSID_VOID, BaseProxyHandler::ENUMERATE); - return enumerate(cx, wrapper, JSITER_OWNONLY | JSITER_HIDDEN, props); + return enumerate(cx, wrapper, JSITER_OWNONLY | JSITER_HIDDEN | JSITER_SYMBOLS, props); } template