# HG changeset patch # User Boris Zbarsky # Date 1515783680 18000 # Fri Jan 12 14:01:20 2018 -0500 # Node ID 41ee157189c8514624b5ff8be936b5f72b06ea34 # Parent 99f4647a241e0d20b5cefe34f8fed063f3852280 Bug 1430164. Stop doing weird sandbox-callable-wrapping for DOM constructors. r=bholley This fixes DOM constructor identity in web extension content scripts to work as expected. MozReview-Commit-ID: Ab4sFWiMU6c diff --git a/js/xpconnect/src/Sandbox.cpp b/js/xpconnect/src/Sandbox.cpp --- a/js/xpconnect/src/Sandbox.cpp +++ b/js/xpconnect/src/Sandbox.cpp @@ -751,16 +751,35 @@ bool WrapAccessorFunction(JSContext* cx, RootedObject func(cx, JS_FUNC_TO_DATA_PTR(JSObject*, op)); func = WrapCallable(cx, func, sandboxProtoProxy); if (!func) return false; op = JS_DATA_TO_FUNC_PTR(Op, func.get()); return true; } +static bool +IsDOMConstructor(JSObject* obj) +{ + // We really care about the underlying object here, which might be wrapped + // in cross-compartment wrappers. + obj = js::CheckedUnwrap(obj); + if (!obj) { + return false; + } + + if (JS_IsNativeFunction(obj, dom::Constructor)) { + return true; + } + + const js::Class* clasp = js::GetObjectClass(obj); + return dom::IsDOMIfaceAndProtoClass(clasp) && + dom::DOMIfaceAndProtoJSClass::FromJSClass(clasp)->mType == dom::eInterface; +} + bool xpc::SandboxProxyHandler::getPropertyDescriptor(JSContext* cx, JS::Handle proxy, JS::Handle id, JS::MutableHandle desc) const { JS::RootedObject obj(cx, wrappedObject(proxy)); @@ -775,17 +794,21 @@ xpc::SandboxProxyHandler::getPropertyDes if (!WrapAccessorFunction(cx, desc.getter(), desc.address(), JSPROP_GETTER, proxy)) return false; if (!WrapAccessorFunction(cx, desc.setter(), desc.address(), JSPROP_SETTER, proxy)) return false; if (desc.value().isObject()) { RootedObject val (cx, &desc.value().toObject()); - if (JS::IsCallable(val)) { + if (JS::IsCallable(val) && + // Don't wrap DOM constructors: they don't care about the "this" + // they're invoked with anyway, being constructors. And if we wrap + // them here we break invariants like Node == Node and whatnot. + !IsDOMConstructor(val)) { val = WrapCallable(cx, val, proxy); if (!val) return false; desc.value().setObject(*val); } } return true; diff --git a/js/xpconnect/tests/chrome/chrome.ini b/js/xpconnect/tests/chrome/chrome.ini --- a/js/xpconnect/tests/chrome/chrome.ini +++ b/js/xpconnect/tests/chrome/chrome.ini @@ -84,16 +84,17 @@ skip-if = os == 'win' || os == 'mac' || [test_bug1050049.html] [test_bug1065185.html] [test_bug1074863.html] [test_bug1092477.xul] [test_bug1124898.html] [test_bug1126911.html] [test_bug1281071.xul] [test_bug1390159.xul] +[test_bug1430164.html] [test_chrometoSource.xul] [test_cloneInto.xul] [test_cows.xul] [test_discardSystemSource.xul] [test_documentdomain.xul] [test_doublewrappedcompartments.xul] [test_evalInSandbox.xul] [test_evalInWindow.xul] diff --git a/js/xpconnect/tests/chrome/test_bug1430164.html b/js/xpconnect/tests/chrome/test_bug1430164.html new file mode 100644 --- /dev/null +++ b/js/xpconnect/tests/chrome/test_bug1430164.html @@ -0,0 +1,32 @@ + + + + + + Test for Bug 1430164 + + + + + +