Bug 951948 - Remove Components wrappers. r=mrbkap
authorBobby Holley <bobbyholley@gmail.com>
Tue, 14 Jan 2014 18:49:30 -0800
changeset 163438 7cc30ae56811cae75aecc277251d1272f9ad2cc8
parent 163437 707abbb92a8ba920caae74fdaf9dc62e6364bc66
child 163439 2e691e6ce23a962bccd20347ab88ae22a6d9f07d
push id25996
push useremorley@mozilla.com
push dateWed, 15 Jan 2014 15:54:39 +0000
treeherdermozilla-central@dd2cf81c56b7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs951948
milestone29.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 951948 - Remove Components wrappers. r=mrbkap We fix up the tests here to test the new behavior, and fix some bugs in the test while we're at it.
js/xpconnect/src/XPCWrappedNative.cpp
js/xpconnect/tests/unit/test_components.js
js/xpconnect/wrappers/AccessCheck.cpp
js/xpconnect/wrappers/AccessCheck.h
js/xpconnect/wrappers/FilteringWrapper.cpp
js/xpconnect/wrappers/WrapperFactory.cpp
js/xpconnect/wrappers/WrapperFactory.h
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -1704,20 +1704,16 @@ XPCWrappedNative::GetSameCompartmentSecu
     // 'wrapping failed' cases.
     //
     // NB: We don't make SOWs for remote XUL domains where XBL scopes are
     // disallowed.
     if (NeedsSOW() && xpc::AllowXBLScope(js::GetContextCompartment(cx))) {
         wrapper = xpc::WrapperFactory::WrapSOWObject(cx, flat);
         if (!wrapper)
             return nullptr;
-    } else if (xpc::WrapperFactory::IsComponentsObject(flat)) {
-        wrapper = xpc::WrapperFactory::WrapComponentsObject(cx, flat);
-        if (!wrapper)
-            return nullptr;
     }
 
     // If we made a wrapper, cache it and return it.
     if (wrapper) {
         SetWrapper(wrapper);
         return wrapper;
     }
 
--- a/js/xpconnect/tests/unit/test_components.js
+++ b/js/xpconnect/tests/unit/test_components.js
@@ -13,33 +13,42 @@ function run_test() {
   [sb1, sb2, sb4].forEach(function(x) { x.Components = Cu.getComponentsForScope(x); });
 
   // non-chrome accessing chrome Components
   sb1.C = Components;
   checkThrows("C.utils", sb1);
   checkThrows("C.classes", sb1);
 
   // non-chrome accessing own Components
-  checkThrows("Components.utils", sb1);
-  checkThrows("Components.classes", sb1);
+  do_check_eq(Cu.evalInSandbox("typeof Components.interfaces", sb1), 'object');
+  do_check_eq(Cu.evalInSandbox("typeof Components.utils", sb1), 'undefined');
+  do_check_eq(Cu.evalInSandbox("typeof Components.classes", sb1), 'undefined');
+
+  // Make sure an unprivileged Components is benign.
+  var C2 = Cu.evalInSandbox("Components", sb2);
+  var whitelist = ['interfaces', 'interfacesByID', 'results', 'isSuccessCode', 'QueryInterface'];
+  for (var prop in Components) {
+    do_print("Checking " + prop);
+    do_check_eq((prop in C2), whitelist.indexOf(prop) != -1);
+  }
 
   // non-chrome same origin
-  var C2 = Cu.evalInSandbox("Components", sb2);
-  do_check_neq(rv, C2.utils);
   sb1.C2 = C2;
-  checkThrows("C2.utils", sb1);
-  checkThrows("C2.classes", sb1);
+  do_check_eq(Cu.evalInSandbox("typeof C2.interfaces", sb1), 'object');
+  do_check_eq(Cu.evalInSandbox("typeof C2.utils", sb1), 'undefined');
+  do_check_eq(Cu.evalInSandbox("typeof C2.classes", sb1), 'undefined');
 
   // chrome accessing chrome
   sb3.C = Components;
   rv = Cu.evalInSandbox("C.utils", sb3);
   do_check_eq(rv, Cu);
 
   // non-chrome cross origin
   sb4.C2 = C2;
-  checkThrows("C2.utils", sb1);
-  checkThrows("C2.classes", sb1);
+  checkThrows("C2.interfaces", sb4);
+  checkThrows("C2.utils", sb4);
+  checkThrows("C2.classes", sb4);
 }
 
 function checkThrows(expression, sb) {
   var result = Cu.evalInSandbox('(function() { try { ' + expression + '; return "allowed"; } catch (e) { return e.toString(); }})();', sb);
   do_check_true(!!/denied/.exec(result));
 }
--- a/js/xpconnect/wrappers/AccessCheck.cpp
+++ b/js/xpconnect/wrappers/AccessCheck.cpp
@@ -396,38 +396,9 @@ ExposedPropertiesOnly::check(JSContext *
 
 bool
 ExposedPropertiesOnly::allowNativeCall(JSContext *cx, JS::IsAcceptableThis test,
                                        JS::NativeImpl impl)
 {
     return js::IsReadOnlyDateMethod(test, impl) || js::IsTypedArrayThisCheck(test);
 }
 
-bool
-ComponentsObjectPolicy::check(JSContext *cx, JSObject *wrapperArg, jsid idArg, Wrapper::Action act)
-{
-    RootedObject wrapper(cx, wrapperArg);
-    RootedId id(cx, idArg);
-    JSAutoCompartment ac(cx, wrapper);
-
-    if (JSID_IS_STRING(id) && act == Wrapper::GET) {
-        JSFlatString *flatId = JSID_TO_FLAT_STRING(id);
-        if (JS_FlatStringEqualsAscii(flatId, "isSuccessCode") ||
-            JS_FlatStringEqualsAscii(flatId, "lookupMethod") ||
-            JS_FlatStringEqualsAscii(flatId, "interfaces") ||
-            JS_FlatStringEqualsAscii(flatId, "interfacesByID") ||
-            JS_FlatStringEqualsAscii(flatId, "results"))
-        {
-            return true;
-        }
-    }
-
-    // We don't have any way to recompute same-compartment Components wrappers,
-    // so we need this dynamic check. This can go away when we expose Components
-    // as SpecialPowers.wrap(Components) during automation.
-    if (xpc::IsUniversalXPConnectEnabled(cx)) {
-        return true;
-    }
-
-    return false;
 }
-
-}
--- a/js/xpconnect/wrappers/AccessCheck.h
+++ b/js/xpconnect/wrappers/AccessCheck.h
@@ -93,23 +93,11 @@ struct ExposedPropertiesOnly : public Po
 
     static bool deny(js::Wrapper::Action act, JS::HandleId id) {
         // Fail silently for GETs.
         return act == js::Wrapper::GET;
     }
     static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl);
 };
 
-// Components specific policy
-struct ComponentsObjectPolicy : public Policy {
-    static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act);
-
-    static bool deny(js::Wrapper::Action act, JS::HandleId id) {
-        return false;
-    }
-    static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl) {
-        return false;
-    }
-};
-
 }
 
 #endif /* __AccessCheck_h__ */
--- a/js/xpconnect/wrappers/FilteringWrapper.cpp
+++ b/js/xpconnect/wrappers/FilteringWrapper.cpp
@@ -189,27 +189,22 @@ FilteringWrapper<Base, Policy>::enter(JS
 }
 
 // NB: don't need SOW here because the resulting wrapper would be identical to
 // NNXOW.
 #define SCSOW FilteringWrapper<SameCompartmentSecurityWrapper, Opaque>
 #define XOW FilteringWrapper<SecurityXrayXPCWN, CrossOriginAccessiblePropertiesOnly>
 #define DXOW   FilteringWrapper<SecurityXrayDOM, CrossOriginAccessiblePropertiesOnly>
 #define NNXOW FilteringWrapper<CrossCompartmentSecurityWrapper, Opaque>
-#define CW FilteringWrapper<SameCompartmentSecurityWrapper, ComponentsObjectPolicy>
-#define XCW FilteringWrapper<CrossCompartmentSecurityWrapper, ComponentsObjectPolicy>
 #define GO FilteringWrapper<CrossCompartmentSecurityWrapper, GentlyOpaque>
 template<> SCSOW SCSOW::singleton(0);
 template<> XOW XOW::singleton(0);
 template<> DXOW DXOW::singleton(0);
 template<> NNXOW NNXOW::singleton(0);
 
-template<> CW CW::singleton(0);
-template<> XCW XCW::singleton(0);
-
 template<> GO GO::singleton(0);
 
 template class XOW;
 template class DXOW;
 template class NNXOW;
 template class ChromeObjectWrapperBase;
 template class GO;
 }
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -311,19 +311,16 @@ WrapperFactory::PrepareForWrapping(JSCon
 #ifdef DEBUG
 static void
 DEBUG_CheckUnwrapSafety(HandleObject obj, js::Wrapper *handler,
                         JSCompartment *origin, JSCompartment *target)
 {
     if (AccessCheck::isChrome(target) || xpc::IsUniversalXPConnectEnabled(target)) {
         // If the caller is chrome (or effectively so), unwrap should always be allowed.
         MOZ_ASSERT(handler->isSafeToUnwrap());
-    } else if (WrapperFactory::IsComponentsObject(obj)) {
-        // The Components object that is restricted regardless of origin.
-        MOZ_ASSERT(!handler->isSafeToUnwrap());
     } else if (AccessCheck::needsSystemOnlyWrapper(obj)) {
         // The rules for SOWs are complicated enough. Just skip double-checking them here.
     } else if (handler == &FilteringWrapper<CrossCompartmentSecurityWrapper, GentlyOpaque>::singleton) {
         // We explicitly use a SecurityWrapper to protect privileged callers from
         // less-privileged objects that they should never see. Skip the check in
         // this case.
     } else {
         // Otherwise, it should depend on whether the target subsumes the origin.
@@ -407,22 +404,19 @@ WrapperFactory::Rewrap(JSContext *cx, Ha
     if (xpc::IsUniversalXPConnectEnabled(target)) {
         wrapper = &CrossCompartmentWrapper::singleton;
 
     // If this is a chrome object being exposed to content without Xrays, use
     // a COW.
     } else if (originIsChrome && !targetIsChrome && xrayType == NotXray) {
         wrapper = &ChromeObjectWrapper::singleton;
 
-    // If content is accessing a Components object or NAC, we need a special filter,
-    // even if the object is same origin. Note that we allow access to NAC for
-    // remote-XUL whitelisted domains, since they don't have XBL scopes.
-    } else if (IsComponentsObject(obj) && !AccessCheck::isChrome(target)) {
-        wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper,
-                                    ComponentsObjectPolicy>::singleton;
+    // If content is accessing NAC, we need a special filter, even if the
+    // object is same origin. Note that we allow access to NAC for remote-XUL
+    // whitelisted domains, since they don't have XBL scopes.
     } else if (AccessCheck::needsSystemOnlyWrapper(obj) &&
                xpc::AllowXBLScope(target) &&
                !(targetIsChrome || (targetSubsumesOrigin && nsContentUtils::IsCallerXBL())))
     {
         wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper, Opaque>::singleton;
     }
 
     // Normally, a non-xrayable non-waived content object that finds itself in
@@ -511,18 +505,16 @@ WrapperFactory::WrapForSameCompartment(J
         return obj;
 
     // Extract the WN. It should exist.
     XPCWrappedNative *wn = XPCWrappedNative::Get(obj);
     MOZ_ASSERT(wn, "Trying to wrap a dead WN!");
 
     // The WN knows what to do.
     RootedObject wrapper(cx, wn->GetSameCompartmentSecurityWrapper(cx));
-    MOZ_ASSERT_IF(wrapper != obj && IsComponentsObject(js::UncheckedUnwrap(obj)),
-                  !Wrapper::wrapperHandler(wrapper)->isSafeToUnwrap());
     return wrapper;
 }
 
 // Call WaiveXrayAndWrap when you have a JS object that you don't want to be
 // wrapped in an Xray wrapper. cx->compartment is the compartment that will be
 // using the returned object. If the object to be wrapped is already in the
 // correct compartment, then this returns the unwrapped object.
 bool
@@ -582,33 +574,16 @@ WrapperFactory::WrapSOWObject(JSContext 
     JSObject *wrapperObj =
         Wrapper::New(cx, obj, JS_GetGlobalForObject(cx, obj),
                      &FilteringWrapper<SameCompartmentSecurityWrapper,
                      Opaque>::singleton);
     return wrapperObj;
 }
 
 bool
-WrapperFactory::IsComponentsObject(JSObject *obj)
-{
-    const char *name = js::GetObjectClass(obj)->name;
-    return name[0] == 'n' && !strcmp(name, "nsXPCComponents");
-}
-
-JSObject *
-WrapperFactory::WrapComponentsObject(JSContext *cx, HandleObject obj)
-{
-    JSObject *wrapperObj =
-        Wrapper::New(cx, obj, JS_GetGlobalForObject(cx, obj),
-                     &FilteringWrapper<SameCompartmentSecurityWrapper, ComponentsObjectPolicy>::singleton);
-
-    return wrapperObj;
-}
-
-bool
 WrapperFactory::XrayWrapperNotShadowing(JSObject *wrapper, jsid id)
 {
     ResolvingId *rid = ResolvingId::getResolvingIdFromWrapper(wrapper);
     return rid->isXrayShadowing(id);
 }
 
 /*
  * Calls to JS_TransplantObject* should go through these helpers here so that
--- a/js/xpconnect/wrappers/WrapperFactory.h
+++ b/js/xpconnect/wrappers/WrapperFactory.h
@@ -64,22 +64,16 @@ class WrapperFactory {
 
     // Wrap wrapped object into a waiver wrapper and then re-wrap it.
     static bool WaiveXrayAndWrap(JSContext *cx, JS::MutableHandleValue vp);
     static bool WaiveXrayAndWrap(JSContext *cx, JS::MutableHandleObject object);
 
     // Wrap a (same compartment) object in a SOW.
     static JSObject *WrapSOWObject(JSContext *cx, JSObject *obj);
 
-    // Return true if this is a Components object.
-    static bool IsComponentsObject(JSObject *obj);
-
-    // Wrap a (same compartment) Components object.
-    static JSObject *WrapComponentsObject(JSContext *cx, JS::HandleObject obj);
-
     // Returns true if the wrapper is in not shadowing mode for the id.
     static bool XrayWrapperNotShadowing(JSObject *wrapper, jsid id);
 };
 
 extern js::Wrapper XrayWaiver;
 
 }