Bug 951948 - Remove Components wrappers. r=mrbkap
authorBobby Holley <bobbyholley@gmail.com>
Tue, 14 Jan 2014 18:49:30 -0800
changeset 173649 7cc30ae56811cae75aecc277251d1272f9ad2cc8
parent 173648 707abbb92a8ba920caae74fdaf9dc62e6364bc66
child 173650 2e691e6ce23a962bccd20347ab88ae22a6d9f07d
push idunknown
push userunknown
push dateunknown
reviewersmrbkap
bugs951948
milestone29.0a1
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;
 
 }