Backout 23c9f61a243b & 6ca11f4b470c (bug 805807) for mochitest-1 orange in test_contextmenu.html
authorEd Morley <emorley@mozilla.com>
Fri, 02 Nov 2012 14:12:51 +0000
changeset 112164 57eeca4191dd1d38f7d2351406565105d6fc8e9c
parent 112163 5c88f84c75d6d26253d2d9132a3228cb66857a3c
child 112165 b095fe02009e02b1798ee68da1ffe42c2c857056
push id23798
push userryanvm@gmail.com
push dateSat, 03 Nov 2012 00:06:35 +0000
treeherdermozilla-central@6134edeea902 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs805807
milestone19.0a1
backs out23c9f61a243b4bbb833b9ac139d425c1805a6660
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
Backout 23c9f61a243b & 6ca11f4b470c (bug 805807) for mochitest-1 orange in test_contextmenu.html
editor/libeditor/html/tests/test_bug468353.html
js/src/jswrapper.h
js/xpconnect/tests/unit/test_components.js
js/xpconnect/wrappers/AccessCheck.cpp
js/xpconnect/wrappers/AccessCheck.h
js/xpconnect/wrappers/FilteringWrapper.cpp
toolkit/identity/tests/chrome/sandbox_content_perms.html
--- a/editor/libeditor/html/tests/test_bug468353.html
+++ b/editor/libeditor/html/tests/test_bug468353.html
@@ -37,16 +37,17 @@ function checkStylesheets() {
   }
   catch (ex) { }
 
   is(removed, 0, "Should have thrown if stylesheet was not there");
 }
 
 function runTest() {
   const Ci = SpecialPowers.Ci;
+  const Cc = SpecialPowers.Components.classes;
 
   /** Found while fixing bug 440614 **/
   var editframe = window.frames[0];
   var editdoc = editframe.document;
   var editor = null;
   editdoc.write('');
   editdoc.close();
 
--- a/js/src/jswrapper.h
+++ b/js/src/jswrapper.h
@@ -38,16 +38,22 @@ class JS_FRIEND_API(Wrapper) : public Di
         PUNCTURE
     };
 
     enum Flags {
         CROSS_COMPARTMENT = 1 << 0,
         LAST_USED_FLAG = CROSS_COMPARTMENT
     };
 
+    typedef enum {
+        PermitObjectAccess,
+        PermitPropertyAccess,
+        DenyAccess
+    } Permission;
+
     static JSObject *New(JSContext *cx, JSObject *obj, JSObject *proto,
                          JSObject *parent, Wrapper *handler);
 
     static Wrapper *wrapperHandler(RawObject wrapper);
 
     static JSObject *wrappedObject(RawObject wrapper);
 
     unsigned flags() const {
--- a/js/xpconnect/tests/unit/test_components.js
+++ b/js/xpconnect/tests/unit/test_components.js
@@ -1,45 +1,48 @@
-const Cu = Components.utils;
-
 function run_test() {
+  var Cu = Components.utils;
   var sb1 = Cu.Sandbox("http://www.blah.com");
   var sb2 = Cu.Sandbox("http://www.blah.com");
   var sb3 = Cu.Sandbox(this);
   var sb4 = Cu.Sandbox("http://www.other.com");
   var rv;
 
   // Components is normally hidden from content on the XBL scope chain, but we
   // expose it to content here to make sure that the security wrappers work
   // regardless.
   [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);
+  rv = Cu.evalInSandbox("C.utils", sb1);
+  do_check_eq(rv, undefined);  
+  rv = Cu.evalInSandbox("C.interfaces", sb1);
+  do_check_neq(rv, undefined);
 
   // non-chrome accessing own Components
-  checkThrows("Components.utils", sb1);
-  checkThrows("Components.classes", sb1);
+  rv = Cu.evalInSandbox("Components.utils", sb1);
+  do_check_eq(rv, undefined);
+  rv = Cu.evalInSandbox("Components.interfaces", sb1);
+  do_check_neq(rv, undefined); 
 
   // non-chrome same origin
   var C2 = Cu.evalInSandbox("Components", sb2);
-  do_check_neq(rv, C2.utils);
+  do_check_neq(rv, C2.utils); 
   sb1.C2 = C2;
-  checkThrows("C2.utils", sb1);
-  checkThrows("C2.classes", sb1);
+  rv = Cu.evalInSandbox("C2.utils", sb1);
+  do_check_eq(rv, undefined);
+  rv = Cu.evalInSandbox("C2.interfaces", sb1);
+  do_check_neq(rv, 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);
-}
+  rv = Cu.evalInSandbox("C2.interfaces", sb1);
+  do_check_neq(rv, undefined);
+  rv = Cu.evalInSandbox("C2.utils", sb1);
+  do_check_eq(rv, undefined);
 
-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
@@ -325,33 +325,48 @@ AccessCheck::deny(JSContext *cx, jsid id
         if (chars)
             JS_ReportError(cx, "Permission denied to access property '%hs'", chars);
     }
 }
 
 enum Access { READ = (1<<0), WRITE = (1<<1), NO_ACCESS = 0 };
 
 static bool
+Deny(JSContext *cx, jsid id, Wrapper::Action act)
+{
+    // Refuse to perform the action and just return the default value.
+    if (act == Wrapper::GET)
+        return true;
+    // If its a set, deny it and throw an exception.
+    AccessCheck::deny(cx, id);
+    return false;
+}
+
+static bool
 IsInSandbox(JSContext *cx, JSObject *obj)
 {
     JSAutoCompartment ac(cx, obj);
     JSObject *global = JS_GetGlobalForObject(cx, obj);
     return !strcmp(js::GetObjectJSClass(global)->name, "Sandbox");
 }
 
 bool
-ExposedPropertiesOnly::check(JSContext *cx, JSObject *wrapper, jsid id, Wrapper::Action act)
+ExposedPropertiesOnly::check(JSContext *cx, JSObject *wrapper, jsid id, Wrapper::Action act,
+                             Permission &perm)
 {
     JSObject *wrappedObject = Wrapper::wrappedObject(wrapper);
 
-    if (act == Wrapper::CALL)
+    if (act == Wrapper::CALL) {
+        perm = PermitObjectAccess;
         return true;
+    }
 
+    perm = DenyAccess;
     if (act == Wrapper::PUNCTURE)
-        return false;
+        return Deny(cx, id, act);
 
     jsid exposedPropsId = GetRTIdByIndex(cx, XPCJSRuntime::IDX_EXPOSEDPROPS);
 
     // We need to enter the wrappee's compartment to look at __exposedProps__,
     // but we want to be in the wrapper's compartment if we call Deny().
     //
     // Unfortunately, |cx| can be in either compartment when we call ::check. :-(
     JSAutoCompartment ac(cx, wrappedObject);
@@ -360,16 +375,17 @@ ExposedPropertiesOnly::check(JSContext *
     if (!JS_HasPropertyById(cx, wrappedObject, exposedPropsId, &found))
         return false;
 
     // Always permit access to "length" and indexed properties of arrays.
     if ((JS_IsArrayObject(cx, wrappedObject) ||
          JS_IsTypedArrayObject(wrappedObject, cx)) &&
         ((JSID_IS_INT(id) && JSID_TO_INT(id) >= 0) ||
          (JSID_IS_STRING(id) && JS_FlatStringEqualsAscii(JSID_TO_FLAT_STRING(id), "length")))) {
+        perm = PermitPropertyAccess;
         return true; // Allow
     }
 
     // If no __exposedProps__ existed, deny access.
     if (!found) {
         // Everything below here needs to be done in the wrapper's compartment.
         JSAutoCompartment wrapperAC(cx, wrapper);
         // Make a temporary exception for objects in a chrome sandbox to help
@@ -384,47 +400,55 @@ ExposedPropertiesOnly::check(JSContext *
                 nsCOMPtr<nsIDocument> doc =
                     do_QueryInterface(win->GetExtantDocument());
                 if (doc) {
                     doc->WarnOnceAbout(nsIDocument::eNoExposedProps,
                                        /* asError = */ true);
                 }
             }
 
+            perm = PermitPropertyAccess;
             return true;
         }
-        return false;
+        return Deny(cx, id, act);
     }
 
-    if (id == JSID_VOID)
+    if (id == JSID_VOID) {
+        // This will force the caller to call us back for individual property accesses.
+        perm = PermitPropertyAccess;
         return true;
+    }
 
     JS::Value exposedProps;
     if (!JS_LookupPropertyById(cx, wrappedObject, exposedPropsId, &exposedProps))
         return false;
 
-    if (exposedProps.isNullOrUndefined())
-        return false;
+    if (exposedProps.isNullOrUndefined()) {
+        JSAutoCompartment wrapperAC(cx, wrapper);
+        return Deny(cx, id, act);
+    }
 
     if (!exposedProps.isObject()) {
         JS_ReportError(cx, "__exposedProps__ must be undefined, null, or an Object");
         return false;
     }
 
     JSObject *hallpass = &exposedProps.toObject();
 
     Access access = NO_ACCESS;
 
     JSPropertyDescriptor desc;
     memset(&desc, 0, sizeof(desc));
     if (!JS_GetPropertyDescriptorById(cx, hallpass, id, JSRESOLVE_QUALIFIED, &desc)) {
         return false; // Error
     }
-    if (!desc.obj || !(desc.attrs & JSPROP_ENUMERATE))
-        return false;
+    if (desc.obj == NULL || !(desc.attrs & JSPROP_ENUMERATE)) {
+        JSAutoCompartment wrapperAC(cx, wrapper);
+        return Deny(cx, id, act);
+    }
 
     if (!JSVAL_IS_STRING(desc.value)) {
         JS_ReportError(cx, "property must be a string");
         return false;
     }
 
     JSString *str = JSVAL_TO_STRING(desc.value);
     size_t length;
@@ -458,42 +482,48 @@ ExposedPropertiesOnly::check(JSContext *
 
     if (access == NO_ACCESS) {
         JS_ReportError(cx, "specified properties must have a permission bit set");
         return false;
     }
 
     if ((act == Wrapper::SET && !(access & WRITE)) ||
         (act != Wrapper::SET && !(access & READ))) {
-        return false;
+        JSAutoCompartment wrapperAC(cx, wrapper);
+        return Deny(cx, id, act);
     }
 
-    return true;
+    perm = PermitPropertyAccess;
+    return true; // Allow
 }
 
 bool
-ComponentsObjectPolicy::check(JSContext *cx, JSObject *wrapper, jsid id, Wrapper::Action act)
+ComponentsObjectPolicy::check(JSContext *cx, JSObject *wrapper, jsid id, Wrapper::Action act,
+                              Permission &perm) 
 {
+    perm = DenyAccess;
     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"))
         {
+            perm = PermitPropertyAccess;
             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)) {
+        perm = PermitPropertyAccess;
         return true;
     }
 
-    return false;
+    return Deny(cx, id, act);
 }
 
 }
--- a/js/xpconnect/wrappers/AccessCheck.h
+++ b/js/xpconnect/wrappers/AccessCheck.h
@@ -34,40 +34,62 @@ class AccessCheck {
     static bool needsSystemOnlyWrapper(JSObject *obj);
 
     static bool isScriptAccessOnly(JSContext *cx, JSObject *wrapper);
 
     static void deny(JSContext *cx, jsid id);
 };
 
 struct Policy {
+    typedef js::Wrapper::Permission Permission;
+
+    static const Permission PermitObjectAccess = js::Wrapper::PermitObjectAccess;
+    static const Permission PermitPropertyAccess = js::Wrapper::PermitPropertyAccess;
+    static const Permission DenyAccess = js::Wrapper::DenyAccess;
+};
+
+// This policy permits access to all properties.
+struct Permissive : public Policy {
+    static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act,
+                      Permission &perm) {
+        perm = PermitObjectAccess;
+        return true;
+    }
 };
 
 // This policy only permits access to the object if the subject can touch
 // system objects.
 struct OnlyIfSubjectIsSystem : public Policy {
-    static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) {
-        return AccessCheck::isSystemOnlyAccessPermitted(cx);
-    }
-
-    static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) {
+    static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act,
+                      Permission &perm) {
+        if (AccessCheck::isSystemOnlyAccessPermitted(cx)) {
+            perm = PermitObjectAccess;
+            return true;
+        }
+        perm = DenyAccess;
+        JSAutoCompartment ac(cx, wrapper);
         AccessCheck::deny(cx, id);
         return false;
     }
 };
 
 // This policy only permits access to properties that are safe to be used
 // across origins.
 struct CrossOriginAccessiblePropertiesOnly : public Policy {
-    static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) {
+    static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act,
+                      Permission &perm) {
         // Location objects should always use LocationPolicy.
         MOZ_ASSERT(!WrapperFactory::IsLocationObject(js::UnwrapObject(wrapper)));
-        return AccessCheck::isCrossOriginAccessPermitted(cx, wrapper, id, act);
-    }
-    static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) {
+
+        if (AccessCheck::isCrossOriginAccessPermitted(cx, wrapper, id, act)) {
+            perm = PermitPropertyAccess;
+            return true;
+        }
+        perm = DenyAccess;
+        JSAutoCompartment ac(cx, wrapper);
         AccessCheck::deny(cx, id);
         return false;
     }
 };
 
 // We need a special security policy for Location objects.
 //
 // Location objects are special because their effective principal is that of
@@ -87,54 +109,46 @@ struct CrossOriginAccessiblePropertiesOn
 // we need to do a dynamic security check to determine whether the outer window is
 // same-origin with the caller.
 //
 // So this policy first checks whether the access is something that any code,
 // same-origin or not, is allowed to make. If it isn't, it _also_ checks the
 // state of the outer window to determine whether we happen to be same-origin
 // at the moment.
 struct LocationPolicy : public Policy {
-    static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) {
+    static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act,
+                      Permission &perm) {
         // We should only be dealing with Location objects here.
         MOZ_ASSERT(WrapperFactory::IsLocationObject(js::UnwrapObject(wrapper)));
 
+        // Default to deny.
+        perm = DenyAccess;
+
         // Location object security is complicated enough. Don't allow punctures.
         if (act != js::Wrapper::PUNCTURE &&
             (AccessCheck::isCrossOriginAccessPermitted(cx, wrapper, id, act) ||
              AccessCheck::isLocationObjectSameOrigin(cx, wrapper))) {
+            perm = PermitPropertyAccess;
             return true;
         }
-        return false;
-    }
-    static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) {
+
+        JSAutoCompartment ac(cx, wrapper);
         AccessCheck::deny(cx, id);
         return false;
     }
 };
 
 // This policy only permits access to properties if they appear in the
 // objects exposed properties list.
 struct ExposedPropertiesOnly : public Policy {
-    static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act);
-
-    static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) {
-        // For gets, silently fail.
-        if (act == js::Wrapper::GET)
-            return true;
-        // For sets,throw an exception.
-        AccessCheck::deny(cx, id);
-        return false;
-    }
+    static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act,
+                      Permission &perm);
 };
 
 // Components specific policy
 struct ComponentsObjectPolicy : public Policy {
-    static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act);
-
-    static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) {
-        AccessCheck::deny(cx, id);
-        return false;
-    }
+    static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act,
+                      Permission &perm);
 };
 
 }
 
 #endif /* __AccessCheck_h__ */
--- a/js/xpconnect/wrappers/FilteringWrapper.cpp
+++ b/js/xpconnect/wrappers/FilteringWrapper.cpp
@@ -24,27 +24,34 @@ FilteringWrapper<Base, Policy>::Filterin
 {
 }
 
 template <typename Base, typename Policy>
 FilteringWrapper<Base, Policy>::~FilteringWrapper()
 {
 }
 
+typedef Wrapper::Permission Permission;
+
+static const Permission PermitObjectAccess = Wrapper::PermitObjectAccess;
+static const Permission PermitPropertyAccess = Wrapper::PermitPropertyAccess;
+static const Permission DenyAccess = Wrapper::DenyAccess;
+
 template <typename Policy>
 static bool
 Filter(JSContext *cx, JSObject *wrapper, AutoIdVector &props)
 {
     size_t w = 0;
     for (size_t n = 0; n < props.length(); ++n) {
         jsid id = props[n];
-        if (Policy::check(cx, wrapper, id, Wrapper::GET))
+        Permission perm;
+        if (!Policy::check(cx, wrapper, id, Wrapper::GET, perm))
+            return false; // Error
+        if (perm != DenyAccess)
             props[w++] = id;
-        else if (JS_IsExceptionPending(cx))
-            return false;
     }
     props.resize(w);
     return true;
 }
 
 template <typename Base, typename Policy>
 bool
 FilteringWrapper<Base, Policy>::getOwnPropertyNames(JSContext *cx, JSObject *wrapper, AutoIdVector &props)
@@ -80,26 +87,24 @@ FilteringWrapper<Base, Policy>::iterate(
     return js::BaseProxyHandler::iterate(cx, wrapper, flags, vp);
 }
 
 template <typename Base, typename Policy>
 bool
 FilteringWrapper<Base, Policy>::enter(JSContext *cx, JSObject *wrapper, jsid id,
                                       Wrapper::Action act, bool *bp)
 {
-    if (!Policy::check(cx, wrapper, id, act)) {
-        if (JS_IsExceptionPending(cx)) {
-            *bp = false;
-            return false;
-        }
-        JSAutoCompartment ac(cx, wrapper);
-        *bp = Policy::deny(cx, id, act);
+    Permission perm;
+    if (!Policy::check(cx, wrapper, id, act, perm)) {
+        *bp = false;
         return false;
     }
     *bp = true;
+    if (perm == DenyAccess)
+        return false;
     return Base::enter(cx, wrapper, id, act, bp);
 }
 
 #define SOW FilteringWrapper<CrossCompartmentSecurityWrapper, OnlyIfSubjectIsSystem>
 #define SCSOW FilteringWrapper<SameCompartmentSecurityWrapper, OnlyIfSubjectIsSystem>
 #define XOW FilteringWrapper<XrayWrapper<CrossCompartmentSecurityWrapper>, \
                              CrossOriginAccessiblePropertiesOnly>
 #define DXOW   FilteringWrapper<XrayDOM, \
--- a/toolkit/identity/tests/chrome/sandbox_content_perms.html
+++ b/toolkit/identity/tests/chrome/sandbox_content_perms.html
@@ -13,36 +13,27 @@
         try {
           aFunc();
         } catch (ex) {
           return true;
         }
         return false;
       }
 
-      function CcDenied() {
-        try {
-          Components.classes;
-          return false;
-        } catch (e) {
-          return !!/denied/.exec(e);
-        }
-      }
-
       // Build an object with test results (true = pass)
       let results = {
         windowTop: window.top == window,
 
         qiWindow: expectException(function() {
           let isForced = window.QueryInterface(Ci.nsIInterfaceRequestor)
                                .getInterface(Ci.nsIDOMWindowUtils)
                                .docCharsetIsForced;
         }),
 
-        ccAccess: !!CcDenied(),
+        ccAccess: SpecialPowers.Components.classes == null,
       };
 
       let resultsJSON = JSON.stringify(results);
 
       // Send the results to the mochitest server so the test file can retrieve them.
       let stateURL = TEST_BASE + "sandbox_content.sjs"
       let xhr = new XMLHttpRequest();
       xhr.open("GET", stateURL + "?" + encodeURIComponent(resultsJSON), true);