Bug 1083060 - Refactor the XOW access control code to use an enum rather than a char*. r=bz
authorBobby Holley <bobbyholley@gmail.com>
Fri, 17 Oct 2014 16:17:02 +0200
changeset 211044 78f725c6441db73e22b886040a0782f11478bbe0
parent 211043 4714ac6ccbfe6d715edaf82ac1e6f71cecafd13e
child 211045 ba0373a2af17c17e65456136dd8d2f03aea2fabd
push id27667
push usercbook@mozilla.com
push dateMon, 20 Oct 2014 12:40:56 +0000
treeherdermozilla-central@cc2d8bdbccb8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1083060
milestone36.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 1083060 - Refactor the XOW access control code to use an enum rather than a char*. r=bz Srsly.
js/xpconnect/wrappers/AccessCheck.cpp
js/xpconnect/wrappers/AccessCheck.h
--- a/js/xpconnect/wrappers/AccessCheck.cpp
+++ b/js/xpconnect/wrappers/AccessCheck.cpp
@@ -88,26 +88,26 @@ AccessCheck::isChrome(JSObject *obj)
 nsIPrincipal *
 AccessCheck::getPrincipal(JSCompartment *compartment)
 {
     return GetCompartmentPrincipal(compartment);
 }
 
 // Hardcoded policy for cross origin property access. See the HTML5 Spec.
 static bool
-IsPermitted(const char *name, JSFlatString *prop, bool set)
+IsPermitted(CrossOriginObjectType type, JSFlatString *prop, bool set)
 {
     size_t propLength = JS_GetStringLength(JS_FORGET_STRING_FLATNESS(prop));
     if (!propLength)
         return false;
 
     char16_t propChar0 = JS_GetFlatStringCharAt(prop, 0);
-    if (name[0] == 'L' && !strcmp(name, "Location"))
+    if (type == CrossOriginLocation)
         return dom::LocationBinding::IsPermitted(prop, propChar0, set);
-    if (name[0] == 'W' && !strcmp(name, "Window"))
+    if (type == CrossOriginWindow)
         return dom::WindowBinding::IsPermitted(prop, propChar0, set);
 
     return false;
 }
 
 static bool
 IsFrameId(JSContext *cx, JSObject *objArg, jsid idArg)
 {
@@ -136,20 +136,29 @@ IsFrameId(JSContext *cx, JSObject *objAr
             return false;
         }
         col->NamedItem(idAsString, getter_AddRefs(domwin));
     }
 
     return domwin != nullptr;
 }
 
-static bool
-IsWindow(const char *name)
+CrossOriginObjectType
+IdentifyCrossOriginObject(JSObject *obj)
 {
-    return name[0] == 'W' && !strcmp(name, "Window");
+    obj = js::UncheckedUnwrap(obj, /* stopAtOuter = */ false);
+    const js::Class *clasp = js::GetObjectClass(obj);
+    MOZ_ASSERT(!XrayUtils::IsXPCWNHolderClass(Jsvalify(clasp)), "shouldn't have a holder here");
+
+    if (clasp->name[0] == 'L' && !strcmp(clasp->name, "Location"))
+        return CrossOriginLocation;
+    if (clasp->name[0] == 'W' && !strcmp(clasp->name, "Window"))
+        return CrossOriginWindow;
+
+    return CrossOriginOpaque;
 }
 
 bool
 AccessCheck::isCrossOriginAccessPermitted(JSContext *cx, HandleObject wrapper, HandleId id,
                                           Wrapper::Action act)
 {
     if (act == Wrapper::CALL)
         return false;
@@ -161,35 +170,28 @@ AccessCheck::isCrossOriginAccessPermitte
     // is allowed, and rely on FilteringWrapper to filter out any disallowed accessors.
     if (act == Wrapper::GET_PROPERTY_DESCRIPTOR) {
         return isCrossOriginAccessPermitted(cx, wrapper, id, Wrapper::GET) ||
                isCrossOriginAccessPermitted(cx, wrapper, id, Wrapper::SET);
     }
 
     RootedObject obj(cx, Wrapper::wrappedObject(wrapper));
 
-    const char *name;
-    const js::Class *clasp = js::GetObjectClass(obj);
-    MOZ_ASSERT(!XrayUtils::IsXPCWNHolderClass(Jsvalify(clasp)), "shouldn't have a holder here");
-    if (clasp->ext.innerObject)
-        name = "Window";
-    else
-        name = clasp->name;
-
+    CrossOriginObjectType type = IdentifyCrossOriginObject(obj);
     if (JSID_IS_STRING(id)) {
-        if (IsPermitted(name, JSID_TO_FLAT_STRING(id), act == Wrapper::SET))
+        if (IsPermitted(type, JSID_TO_FLAT_STRING(id), act == Wrapper::SET))
             return true;
     }
 
     if (act != Wrapper::GET)
         return false;
 
     // Check for frame IDs. If we're resolving named frames, make sure to only
     // resolve ones that don't shadow native properties. See bug 860494.
-    if (IsWindow(name)) {
+    if (type == CrossOriginWindow) {
         if (JSID_IS_STRING(id)) {
             bool wouldShadow = false;
             if (!XrayUtils::HasNativeProperty(cx, wrapper, id, &wouldShadow) ||
                 wouldShadow)
             {
                 // If the named subframe matches the name of a DOM constructor,
                 // the global resolve triggered by the HasNativeProperty call
                 // above will try to perform a CheckedUnwrap on |wrapper|, and
--- a/js/xpconnect/wrappers/AccessCheck.h
+++ b/js/xpconnect/wrappers/AccessCheck.h
@@ -22,16 +22,23 @@ class AccessCheck {
     static bool subsumesConsideringDomain(JSCompartment *a, JSCompartment *b);
     static bool isChrome(JSCompartment *compartment);
     static bool isChrome(JSObject *obj);
     static nsIPrincipal *getPrincipal(JSCompartment *compartment);
     static bool isCrossOriginAccessPermitted(JSContext *cx, JS::HandleObject obj,
                                              JS::HandleId id, js::Wrapper::Action act);
 };
 
+enum CrossOriginObjectType {
+    CrossOriginWindow,
+    CrossOriginLocation,
+    CrossOriginOpaque
+};
+CrossOriginObjectType IdentifyCrossOriginObject(JSObject *obj);
+
 struct Policy {
 };
 
 // This policy allows no interaction with the underlying callable. Everything throws.
 struct Opaque : public Policy {
     static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) {
         return false;
     }