From 80068e5cca2254c88ce3b4a10b413ab0292d5814 Mon Sep 17 00:00:00 2001
From: Andreas Gal <gal@mozilla.com>
Date: Sat, 29 Jan 2011 18:48:30 -0800
Subject: [PATCH 1/4] Silenty return undefined instead of throwing when content
 tries to access non-exposed chrome properties (bug 594999, r=mrbkap).

---
 js/src/jswrapper.cpp                          | 51 +++++++++++++++----
 js/src/jswrapper.h                            |  2 +-
 js/src/xpconnect/wrappers/AccessCheck.cpp     | 26 +++++++---
 js/src/xpconnect/wrappers/AccessCheck.h       | 33 +++++++++---
 .../xpconnect/wrappers/CrossOriginWrapper.cpp |  3 +-
 .../xpconnect/wrappers/CrossOriginWrapper.h   |  2 +-
 .../xpconnect/wrappers/FilteringWrapper.cpp   | 34 ++++---------
 js/src/xpconnect/wrappers/FilteringWrapper.h  |  2 +-
 js/src/xpconnect/wrappers/XrayWrapper.cpp     | 21 +++++---
 9 files changed, 113 insertions(+), 61 deletions(-)

diff --git a/js/src/jswrapper.cpp b/js/src/jswrapper.cpp
index 9da5e3afa224..661b03a6b143 100644
--- a/js/src/jswrapper.cpp
+++ b/js/src/jswrapper.cpp
@@ -97,8 +97,9 @@ JSWrapper::~JSWrapper()
 
 #define CHECKED(op, act)                                                     \
     JS_BEGIN_MACRO                                                           \
-        if (!enter(cx, wrapper, id, act))                                    \
-            return false;                                                    \
+        bool status;                                                         \
+        if (!enter(cx, wrapper, id, act, &status))                           \
+            return status;                                                   \
         bool ok = (op);                                                      \
         leave(cx, wrapper);                                                  \
         return ok;                                                           \
@@ -111,6 +112,7 @@ bool
 JSWrapper::getPropertyDescriptor(JSContext *cx, JSObject *wrapper, jsid id,
                                  bool set, PropertyDescriptor *desc)
 {
+    desc->obj= NULL; // default result if we refuse to perform this action
     CHECKED(JS_GetPropertyDescriptorById(cx, wrappedObject(wrapper), id, JSRESOLVE_QUALIFIED,
                                          Jsvalify(desc)), set ? SET : GET);
 }
@@ -129,6 +131,7 @@ bool
 JSWrapper::getOwnPropertyDescriptor(JSContext *cx, JSObject *wrapper, jsid id, bool set,
                                     PropertyDescriptor *desc)
 {
+    desc->obj= NULL; // default result if we refuse to perform this action
     CHECKED(GetOwnPropertyDescriptor(cx, wrappedObject(wrapper), id, JSRESOLVE_QUALIFIED,
                                      Jsvalify(desc)), set ? SET : GET);
 }
@@ -144,6 +147,7 @@ JSWrapper::defineProperty(JSContext *cx, JSObject *wrapper, jsid id,
 bool
 JSWrapper::getOwnPropertyNames(JSContext *cx, JSObject *wrapper, AutoIdVector &props)
 {
+    // if we refuse to perform this action, props remains empty
     jsid id = JSID_VOID;
     GET(GetPropertyNames(cx, wrappedObject(wrapper), JSITER_OWNONLY | JSITER_HIDDEN, &props));
 }
@@ -158,6 +162,7 @@ ValueToBoolean(Value *vp, bool *bp)
 bool
 JSWrapper::delete_(JSContext *cx, JSObject *wrapper, jsid id, bool *bp)
 {
+    *bp = true; // default result if we refuse to perform this action
     Value v;
     SET(JS_DeletePropertyById2(cx, wrappedObject(wrapper), id, Jsvalify(&v)) &&
         ValueToBoolean(&v, bp));
@@ -166,6 +171,7 @@ JSWrapper::delete_(JSContext *cx, JSObject *wrapper, jsid id, bool *bp)
 bool
 JSWrapper::enumerate(JSContext *cx, JSObject *wrapper, AutoIdVector &props)
 {
+    // if we refuse to perform this action, props remains empty
     static jsid id = JSID_VOID;
     GET(GetPropertyNames(cx, wrappedObject(wrapper), 0, &props));
 }
@@ -187,6 +193,7 @@ Cond(JSBool b, bool *bp)
 bool
 JSWrapper::has(JSContext *cx, JSObject *wrapper, jsid id, bool *bp)
 {
+    *bp = false; // default result if we refuse to perform this action
     JSBool found;
     GET(JS_HasPropertyById(cx, wrappedObject(wrapper), id, &found) &&
         Cond(found, bp));
@@ -195,6 +202,7 @@ JSWrapper::has(JSContext *cx, JSObject *wrapper, jsid id, bool *bp)
 bool
 JSWrapper::hasOwn(JSContext *cx, JSObject *wrapper, jsid id, bool *bp)
 {
+    *bp = false; // default result if we refuse to perform this action
     PropertyDescriptor desc;
     JSObject *wobj = wrappedObject(wrapper);
     GET(JS_GetPropertyDescriptorById(cx, wobj, id, JSRESOLVE_QUALIFIED, Jsvalify(&desc)) &&
@@ -204,6 +212,7 @@ JSWrapper::hasOwn(JSContext *cx, JSObject *wrapper, jsid id, bool *bp)
 bool
 JSWrapper::get(JSContext *cx, JSObject *wrapper, JSObject *receiver, jsid id, Value *vp)
 {
+    vp->setUndefined(); // default result if we refuse to perform this action
     GET(wrappedObject(wrapper)->getProperty(cx, receiver, id, vp));
 }
 
@@ -217,6 +226,7 @@ JSWrapper::set(JSContext *cx, JSObject *wrapper, JSObject *receiver, jsid id, Va
 bool
 JSWrapper::keys(JSContext *cx, JSObject *wrapper, AutoIdVector &props)
 {
+    // if we refuse to perform this action, props remains empty
     const jsid id = JSID_VOID;
     GET(GetPropertyNames(cx, wrappedObject(wrapper), JSITER_OWNONLY, &props));
 }
@@ -224,6 +234,7 @@ JSWrapper::keys(JSContext *cx, JSObject *wrapper, AutoIdVector &props)
 bool
 JSWrapper::iterate(JSContext *cx, JSObject *wrapper, uintN flags, Value *vp)
 {
+    vp->setUndefined(); // default result if we refuse to perform this action
     const jsid id = JSID_VOID;
     GET(GetIterator(cx, wrappedObject(wrapper), flags, vp));
 }
@@ -231,20 +242,23 @@ JSWrapper::iterate(JSContext *cx, JSObject *wrapper, uintN flags, Value *vp)
 bool
 JSWrapper::call(JSContext *cx, JSObject *wrapper, uintN argc, Value *vp)
 {
+    vp->setUndefined(); // default result if we refuse to perform this action
     const jsid id = JSID_VOID;
     CHECKED(JSProxyHandler::call(cx, wrapper, argc, vp), CALL);
 }
 
 bool
-JSWrapper::construct(JSContext *cx, JSObject *wrapper, uintN argc, Value *argv, Value *rval)
+JSWrapper::construct(JSContext *cx, JSObject *wrapper, uintN argc, Value *argv, Value *vp)
 {
+    vp->setUndefined(); // default result if we refuse to perform this action
     const jsid id = JSID_VOID;
-    GET(JSProxyHandler::construct(cx, wrapper, argc, argv, rval));
+    GET(JSProxyHandler::construct(cx, wrapper, argc, argv, vp));
 }
 
 bool
 JSWrapper::hasInstance(JSContext *cx, JSObject *wrapper, const Value *vp, bool *bp)
 {
+    *bp = true; // default result if we refuse to perform this action
     const jsid id = JSID_VOID;
     JSBool b;
     GET(JS_HasInstance(cx, wrappedObject(wrapper), Jsvalify(*vp), &b) && Cond(b, bp));
@@ -259,10 +273,15 @@ JSWrapper::typeOf(JSContext *cx, JSObject *wrapper)
 JSString *
 JSWrapper::obj_toString(JSContext *cx, JSObject *wrapper)
 {
-    JSString *str;
-    if (!enter(cx, wrapper, JSID_VOID, GET))
+    bool status;
+    if (!enter(cx, wrapper, JSID_VOID, GET, &status)) {
+        if (status) {
+            // Perform some default behavior that doesn't leak any information.
+            return JS_NewStringCopyZ(cx, "[object Object]");
+        }
         return NULL;
-    str = obj_toStringHelper(cx, wrappedObject(wrapper));
+    }
+    JSString *str = obj_toStringHelper(cx, wrappedObject(wrapper));
     leave(cx, wrapper);
     return str;
 }
@@ -270,10 +289,19 @@ JSWrapper::obj_toString(JSContext *cx, JSObject *wrapper)
 JSString *
 JSWrapper::fun_toString(JSContext *cx, JSObject *wrapper, uintN indent)
 {
-    JSString *str;
-    if (!enter(cx, wrapper, JSID_VOID, GET))
+    bool status;
+    if (!enter(cx, wrapper, JSID_VOID, GET, &status)) {
+        if (status) {
+            // Perform some default behavior that doesn't leak any information.
+            if (wrapper->isCallable())
+                return JS_NewStringCopyZ(cx, "function () {\n    [native code]\n}");
+            js::Value v = ObjectValue(*wrapper);
+            js_ReportIsNotFunction(cx, &v, 0);
+            return NULL;
+        }
         return NULL;
-    str = JSProxyHandler::fun_toString(cx, wrapper, indent);
+    }
+    JSString *str = JSProxyHandler::fun_toString(cx, wrapper, indent);
     leave(cx, wrapper);
     return str;
 }
@@ -285,8 +313,9 @@ JSWrapper::trace(JSTracer *trc, JSObject *wrapper)
 }
 
 bool
-JSWrapper::enter(JSContext *cx, JSObject *wrapper, jsid id, Action act)
+JSWrapper::enter(JSContext *cx, JSObject *wrapper, jsid id, Action act, bool *bp)
 {
+    *bp = true;
     return true;
 }
 
diff --git a/js/src/jswrapper.h b/js/src/jswrapper.h
index c76234e71146..3625762f10a7 100644
--- a/js/src/jswrapper.h
+++ b/js/src/jswrapper.h
@@ -92,7 +92,7 @@ class JS_FRIEND_API(JSWrapper) : public js::JSProxyHandler {
 
     /* Policy enforcement traps. */
     enum Action { GET, SET, CALL };
-    virtual bool enter(JSContext *cx, JSObject *wrapper, jsid id, Action act);
+    virtual bool enter(JSContext *cx, JSObject *wrapper, jsid id, Action act, bool *bp);
     virtual void leave(JSContext *cx, JSObject *wrapper);
 
     static JSWrapper singleton;
diff --git a/js/src/xpconnect/wrappers/AccessCheck.cpp b/js/src/xpconnect/wrappers/AccessCheck.cpp
index d56de878b24c..24075f93ea4c 100644
--- a/js/src/xpconnect/wrappers/AccessCheck.cpp
+++ b/js/src/xpconnect/wrappers/AccessCheck.cpp
@@ -425,8 +425,20 @@ AccessCheck::deny(JSContext *cx, jsid id)
 
 enum Access { READ = (1<<0), WRITE = (1<<1), NO_ACCESS = 0 };
 
+static bool
+Deny(JSContext *cx, jsid id, JSWrapper::Action act)
+{
+    // Refuse to perform the action and just return the default value.
+    if (act == JSWrapper::GET)
+        return true;
+    // If its a set, deny it and throw an exception.
+    AccessCheck::deny(cx, id);
+    return false;
+}
+
 bool
-PermitIfUniversalXPConnect(ExposedPropertiesOnly::Permission &perm)
+PermitIfUniversalXPConnect(JSContext *cx, jsid id, JSWrapper::Action act,
+                           ExposedPropertiesOnly::Permission &perm)
 {
     // If UniversalXPConnect is enabled, allow access even if __exposedProps__ doesn't
     // exists.
@@ -441,8 +453,8 @@ PermitIfUniversalXPConnect(ExposedPropertiesOnly::Permission &perm)
         return true; // Allow
     }
 
-    // Use default
-    return true;
+    // Deny
+    return Deny(cx, id, act);
 }
 
 bool
@@ -481,7 +493,7 @@ ExposedPropertiesOnly::check(JSContext *cx, JSObject *wrapper, jsid id, JSWrappe
             perm = PermitPropertyAccess;
             return true;
         }
-        return PermitIfUniversalXPConnect(perm); // Deny
+        return PermitIfUniversalXPConnect(cx, id, act, perm); // Deny
     }
 
     if (id == JSID_VOID) {
@@ -495,7 +507,7 @@ ExposedPropertiesOnly::check(JSContext *cx, JSObject *wrapper, jsid id, JSWrappe
         return false;
 
     if (JSVAL_IS_VOID(exposedProps) || JSVAL_IS_NULL(exposedProps)) {
-        return PermitIfUniversalXPConnect(perm); // Deny
+        return PermitIfUniversalXPConnect(cx, id, act, perm); // Deny
     }
 
     if (!JSVAL_IS_OBJECT(exposedProps)) {
@@ -512,7 +524,7 @@ ExposedPropertiesOnly::check(JSContext *cx, JSObject *wrapper, jsid id, JSWrappe
         return false; // Error
     }
     if (desc.obj == NULL || !(desc.attrs & JSPROP_ENUMERATE)) {
-        return PermitIfUniversalXPConnect(perm); // Deny
+        return PermitIfUniversalXPConnect(cx, id, act, perm); // Deny
     }
 
     if (!JSVAL_IS_STRING(desc.value)) {
@@ -557,7 +569,7 @@ ExposedPropertiesOnly::check(JSContext *cx, JSObject *wrapper, jsid id, JSWrappe
 
     if ((act == JSWrapper::SET && !(access & WRITE)) ||
         (act != JSWrapper::SET && !(access & READ))) {
-        return PermitIfUniversalXPConnect(perm); // Deny
+        return PermitIfUniversalXPConnect(cx, id, act, perm); // Deny
     }
 
     perm = PermitPropertyAccess;
diff --git a/js/src/xpconnect/wrappers/AccessCheck.h b/js/src/xpconnect/wrappers/AccessCheck.h
index be739599236e..8074ee440f16 100644
--- a/js/src/xpconnect/wrappers/AccessCheck.h
+++ b/js/src/xpconnect/wrappers/AccessCheck.h
@@ -84,10 +84,16 @@ struct Permissive : public Policy {
 struct OnlyIfSubjectIsSystem : public Policy {
     static bool check(JSContext *cx, JSObject *wrapper, jsid id, JSWrapper::Action act,
                       Permission &perm) {
-        perm = DenyAccess;
-        if (AccessCheck::isSystemOnlyAccessPermitted(cx))
+        if (AccessCheck::isSystemOnlyAccessPermitted(cx)) {
             perm = PermitObjectAccess;
-        return true;
+            return true;
+        }
+        perm = DenyAccess;
+        JSAutoEnterCompartment ac;
+        if (!ac.enter(cx, wrapper))
+            return false;
+        AccessCheck::deny(cx, id);
+        return false;
     }
 };
 
@@ -96,10 +102,16 @@ struct OnlyIfSubjectIsSystem : public Policy {
 struct CrossOriginAccessiblePropertiesOnly : public Policy {
     static bool check(JSContext *cx, JSObject *wrapper, jsid id, JSWrapper::Action act,
                       Permission &perm) {
-        perm = DenyAccess;
-        if (AccessCheck::isCrossOriginAccessPermitted(cx, wrapper, id, act))
+        if (AccessCheck::isCrossOriginAccessPermitted(cx, wrapper, id, act)) {
             perm = PermitPropertyAccess;
-        return true;
+            return true;
+        }
+        perm = DenyAccess;
+        JSAutoEnterCompartment ac;
+        if (!ac.enter(cx, wrapper))
+            return false;
+        AccessCheck::deny(cx, id);
+        return false;
     }
 };
 
@@ -108,12 +120,17 @@ struct CrossOriginAccessiblePropertiesOnly : public Policy {
 struct SameOriginOrCrossOriginAccessiblePropertiesOnly : public Policy {
     static bool check(JSContext *cx, JSObject *wrapper, jsid id, JSWrapper::Action act,
                       Permission &perm) {
-        perm = DenyAccess;
         if (AccessCheck::isCrossOriginAccessPermitted(cx, wrapper, id, act) ||
             AccessCheck::isLocationObjectSameOrigin(cx, wrapper)) {
             perm = PermitPropertyAccess;
+            return true;
         }
-        return true;
+        perm = DenyAccess;
+        JSAutoEnterCompartment ac;
+        if (!ac.enter(cx, wrapper))
+            return false;
+        AccessCheck::deny(cx, id);
+        return false;
     }
 };
 
diff --git a/js/src/xpconnect/wrappers/CrossOriginWrapper.cpp b/js/src/xpconnect/wrappers/CrossOriginWrapper.cpp
index 608f934a831d..3f6747e0703a 100644
--- a/js/src/xpconnect/wrappers/CrossOriginWrapper.cpp
+++ b/js/src/xpconnect/wrappers/CrossOriginWrapper.cpp
@@ -108,8 +108,9 @@ CrossOriginWrapper::construct(JSContext *cx, JSObject *wrapper,
 }
 
 bool
-NoWaiverWrapper::enter(JSContext *cx, JSObject *wrapper, jsid id, Action act)
+NoWaiverWrapper::enter(JSContext *cx, JSObject *wrapper, jsid id, Action act, bool *bp)
 {
+    *bp = true; // always allowed
     nsIScriptSecurityManager *ssm = XPCWrapper::GetSecurityManager();
     if (!ssm) {
         return true;
diff --git a/js/src/xpconnect/wrappers/CrossOriginWrapper.h b/js/src/xpconnect/wrappers/CrossOriginWrapper.h
index 0f722a27cda3..d36dbbd2a654 100644
--- a/js/src/xpconnect/wrappers/CrossOriginWrapper.h
+++ b/js/src/xpconnect/wrappers/CrossOriginWrapper.h
@@ -50,7 +50,7 @@ class NoWaiverWrapper : public JSCrossCompartmentWrapper {
     NoWaiverWrapper(uintN flags);
     virtual ~NoWaiverWrapper();
 
-    virtual bool enter(JSContext *cx, JSObject *wrapper, jsid id, Action act);
+    virtual bool enter(JSContext *cx, JSObject *wrapper, jsid id, Action act, bool *bp);
     virtual void leave(JSContext *cx, JSObject *wrapper);
 
     static NoWaiverWrapper singleton;
diff --git a/js/src/xpconnect/wrappers/FilteringWrapper.cpp b/js/src/xpconnect/wrappers/FilteringWrapper.cpp
index 9999bdd626b4..07dfe8a31bb3 100644
--- a/js/src/xpconnect/wrappers/FilteringWrapper.cpp
+++ b/js/src/xpconnect/wrappers/FilteringWrapper.cpp
@@ -82,28 +82,6 @@ Filter(JSContext *cx, JSObject *wrapper, AutoIdVector &props)
     return true;
 }
 
-template <typename Policy>
-static bool
-CheckAndReport(JSContext *cx, JSObject *wrapper, jsid id, JSWrapper::Action act, Permission &perm)
-{
-    if (!Policy::check(cx, wrapper, id, act, perm)) {
-        return false;
-    }
-    if (perm == DenyAccess) {
-        // Reporting an error here indicates a problem entering the
-        // compartment. Therefore, any errors that we throw should be
-        // thrown in our *caller's* compartment, so they can inspect
-        // the error object.
-        JSAutoEnterCompartment ac;
-        if (!ac.enter(cx, wrapper))
-            return false;
-
-        AccessCheck::deny(cx, id);
-        return false;
-    }
-    return true;
-}
-
 template <typename Base, typename Policy>
 bool
 FilteringWrapper<Base, Policy>::getOwnPropertyNames(JSContext *cx, JSObject *wrapper, AutoIdVector &props)
@@ -142,11 +120,17 @@ FilteringWrapper<Base, Policy>::iterate(JSContext *cx, JSObject *wrapper, uintN
 template <typename Base, typename Policy>
 bool
 FilteringWrapper<Base, Policy>::enter(JSContext *cx, JSObject *wrapper, jsid id,
-                                      JSWrapper::Action act)
+                                      JSWrapper::Action act, bool *bp)
 {
     Permission perm;
-    return CheckAndReport<Policy>(cx, wrapper, id, act, perm) &&
-           Base::enter(cx, wrapper, id, act);
+    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<JSCrossCompartmentWrapper, OnlyIfSubjectIsSystem>
diff --git a/js/src/xpconnect/wrappers/FilteringWrapper.h b/js/src/xpconnect/wrappers/FilteringWrapper.h
index 737e7bd22275..3b5b606b2df7 100644
--- a/js/src/xpconnect/wrappers/FilteringWrapper.h
+++ b/js/src/xpconnect/wrappers/FilteringWrapper.h
@@ -53,7 +53,7 @@ class FilteringWrapper : public Base {
     virtual bool keys(JSContext *cx, JSObject *wrapper, js::AutoIdVector &props);
     virtual bool iterate(JSContext *cx, JSObject *proxy, uintN flags, js::Value *vp);
 
-    virtual bool enter(JSContext *cx, JSObject *wrapper, jsid id, JSWrapper::Action act);
+    virtual bool enter(JSContext *cx, JSObject *wrapper, jsid id, JSWrapper::Action act, bool *bp);
 
     static FilteringWrapper singleton;
 };
diff --git a/js/src/xpconnect/wrappers/XrayWrapper.cpp b/js/src/xpconnect/wrappers/XrayWrapper.cpp
index a9c498d9ce2a..4bd0a3ed12c8 100644
--- a/js/src/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/src/xpconnect/wrappers/XrayWrapper.cpp
@@ -421,8 +421,11 @@ XrayWrapper<Base>::resolveOwnProperty(JSContext *cx, JSObject *wrapper, jsid id,
     JSPropertyDescriptor *desc = Jsvalify(desc_in);
 
     if (id == nsXPConnect::GetRuntimeInstance()->GetStringID(XPCJSRuntime::IDX_WRAPPED_JSOBJECT)) {
-        if (!this->enter(cx, wrapper, id, set ? JSWrapper::SET : JSWrapper::GET))
-            return false;
+        bool status;
+        JSWrapper::Action action = set ? JSWrapper::SET : JSWrapper::GET;
+        desc->obj = NULL; // default value
+        if (!this->enter(cx, wrapper, id, action, &status))
+            return status;
 
         AutoLeaveHelper<Base> helper(*this, cx, wrapper);
 
@@ -493,8 +496,11 @@ XrayWrapper<Base>::getPropertyDescriptor(JSContext *cx, JSObject *wrapper, jsid
         return true;
     }
 
-    if (!this->enter(cx, wrapper, id, set ? JSWrapper::SET : JSWrapper::GET))
-        return false;
+    bool status;
+    JSWrapper::Action action = set ? JSWrapper::SET : JSWrapper::GET;
+    desc->obj = NULL; // default value
+    if (!this->enter(cx, wrapper, id, action, &status))
+        return status;
 
     AutoLeaveHelper<Base> helper(*this, cx, wrapper);
 
@@ -558,8 +564,11 @@ XrayWrapper<Base>::getOwnPropertyDescriptor(JSContext *cx, JSObject *wrapper, js
         return true;
     }
 
-    if (!this->enter(cx, wrapper, id, set ? JSWrapper::SET : JSWrapper::GET))
-        return false;
+    bool status;
+    JSWrapper::Action action = set ? JSWrapper::SET : JSWrapper::GET;
+    desc->obj = NULL; // default value
+    if (!this->enter(cx, wrapper, id, action, &status))
+        return status;
 
     AutoLeaveHelper<Base> helper(*this, cx, wrapper);
 

From 8ae905de30bea5bb02b2edd5a9fec482808c154e Mon Sep 17 00:00:00 2001
From: Jonas Sicking <jonas@sicking.cc>
Date: Sat, 29 Jan 2011 18:53:59 -0800
Subject: [PATCH 2/4] Tests for bug 594999 (r=mrbkap).

---
 js/src/xpconnect/tests/chrome/test_cows.xul   | 194 ++++++++++++++----
 js/src/xpconnect/tests/mochitest/Makefile.in  |   1 +
 .../tests/mochitest/test_bug628410.html       |  32 +++
 3 files changed, 185 insertions(+), 42 deletions(-)
 create mode 100644 js/src/xpconnect/tests/mochitest/test_bug628410.html

diff --git a/js/src/xpconnect/tests/chrome/test_cows.xul b/js/src/xpconnect/tests/chrome/test_cows.xul
index f40dcf365098..a5018bd362ec 100644
--- a/js/src/xpconnect/tests/chrome/test_cows.xul
+++ b/js/src/xpconnect/tests/chrome/test_cows.xul
@@ -29,9 +29,9 @@ var test_utils = window.QueryInterface(Ci.nsIInterfaceRequestor).
                  getInterface(Ci.nsIDOMWindowUtils);
 
 function getCOW(x) {
-  if (typeof x == "function")
-    return eval(uneval(x));
   var rval = {};
+  if (typeof x == "function")
+    rval = eval(uneval(x));
   for (var i in x) {
     if (x.__lookupGetter__(i))
       rval.__defineGetter__(i, eval(uneval(x.__lookupGetter__(i))))
@@ -65,61 +65,148 @@ function COWTests() {
     // functions like assertIsWritable(myObj, 'someproperty') might
     // be useful.
 
-    function isProp(obj, propName, value, msg) {
+    function isProp(obj, propName, value, desc) {
+      try {
+          is(obj[propName], value, "getting " + propName + " on " + desc);
+          ok(propName in obj,
+             propName + " on " + desc + " should exist");
+          //ok(Object.hasOwnProperty.call(obj, propName),
+          //   propName + " on " + desc + " should exist");
+      } catch (e) {
+          ok(false, "getting " + propName + " on " + desc + " threw " + e);
+      }
+    }
+    function isPropHidden(obj, propName, desc) {
       try {
-          propValue = obj[propName];
-          is(propValue, value, msg);
+          is(obj[propName], undefined,
+             "getting " + propName + " on " + desc + " should return undefined");
+          ok(!(propName in obj),
+             propName + " on " + desc + " should act as if it doesn't exist");
+          //ok(!Object.hasOwnProperty.call(obj, propName),
+          //   propName + " on " + desc + " should act as if it doesn't exist");
       } catch (e) {
-          ok(false, msg + " (accessing '" + propName + "' threw " + e + ")");
+          ok(false, "getting " + propName + " on " + desc + " threw " + e);
       }
     }
 
+    //var cow = getCOW({ foo: "fooval", __exposedProps__: {}});
+    //Math.sin(1);
+    //is(cow.foo, undefined, "one test to rule them all");
+    //return;
+
+    const PROPS_TO_TEST = ['foo', 'bar', '__proto__', 'prototype', 'constructor'];
+
     var empty = {};
-    isProp(getCOW(empty), "foo", undefined, "empty.foo is undefined");
+    // Once we flip the default for __exposedProps__, this should behave
+    // the same as for function objects below.
+    is(getCOW(empty).foo, undefined,
+       "shouldn't throw when accessing exposed properties that doesn't exist");
 
-    const PROPS_TO_TEST = ['foo', '__proto__', 'prototype', 'constructor'];
+    // Test function objects without __exposedProps__
+    var func = function(x) { return 42; };
+    func.foo = "foo property";
+    var funcCOW = getCOW(func);
+    PROPS_TO_TEST.forEach(function(name) {
+        isPropHidden(funcCOW, name, "function without exposedProps");
+    });
+    is([name for (name in funcCOW)].length, 0,
+       "function without exposedProps shouldn't have any properties");
+    is(funcCOW(), 42, "COW without exposedProps should still be callable");
 
-    var strict = { __exposedProps__: {} };
-    var strictCOW = getCOW(strict);
+    // Test function objects with __exposedProps__
+    var func = function(x) { return 42; };
+    func.foo = "foo property";
+    func.__exposedProps__ = { foo: "r" };
+    var funcCOWr = getCOW(func);
+    PROPS_TO_TEST.forEach(function(name) {
+        if (name == "foo") {
+            isProp(funcCOWr, name, "foo property",
+                   "function with exposedProps");
+        }
+        else {
+            isPropHidden(funcCOWr, name, "function with exposedProps");
+        }
+    });
+    is([name for (name in funcCOWr)].length, 1,
+       "function with exposedProps should only enumerate exposed props");
+    is([name for (name in funcCOWr)][0], "foo",
+       "function with exposedProps should only enumerate exposed props");
+    is(funcCOWr(), 42, "COW with exposedProps should be callable");
 
+    // Test function objects with __exposedProps__
+    var func = function(x) { return 42; };
+    func.foo = "foo property";
+    func.__exposedProps__ = { foo: "r", bar: "r" };
+    var funcCOWr2 = getCOW(func);
     PROPS_TO_TEST.forEach(function(name) {
-        try {
-            strictCOW[name];
-            ok(false, "COWs didn't restrict access to " + uneval(name));
-        } catch (e) {
-            ok(/Permission denied/.test(e.message),
-               "should get 'Permission denied' trying to access arbitrary property " +
-               uneval(name) + ", got: " + e.message);
+        if (name == "foo") {
+            isProp(funcCOWr2, name, "foo property",
+                   "function with more exposedProps");
+        }
+        else {
+            isPropHidden(funcCOWr2, name, "function with more exposedProps");
         }
     });
+    is([name for (name in funcCOWr2)].length, 1,
+       "function with exposedProps should only enumerate exposed props");
+    is([name for (name in funcCOWr2)][0], "foo",
+       "function with exposedProps should only enumerate exposed props");
 
-    try {
-        if (strictCOW.foo)
-            ok(false, "nonexistent property shouldn't be truthy.");
-        else
-            ok(true, "'duck-typing' detection on nonexistent prop " +
-                     "should work.");
-    } catch (e) {
-        todo(false,
-             "'duck-typing' detection on a non-exposed prop of a COWed " +
-             "obj should not throw");
-    }
+    // Test object with empty exposedProps
+    var strict = { __exposedProps__: {}, foo: "foo property" };
+    var strictCOW = getCOW(strict);
+    PROPS_TO_TEST.forEach(function(name) {
+        isPropHidden(strictCOW, name, "object with empty exposedProps");
+    });
+    is([name for (name in strictCOW)].length, 0,
+       "object with empty exposedProps shouldn't have any properties");
 
+    // Test object with one exposed property
+    var strict = { __exposedProps__: { foo: "r" }, foo: "foo property" };
+    var strictCOWr = getCOW(strict);
+    PROPS_TO_TEST.forEach(function(name) {
+        if (name == "foo") {
+            isProp(strictCOWr, name, "foo property",
+                   "object with exposed 'foo'");
+        }
+        else {
+            isPropHidden(strictCOW, name, "object with exposed 'foo'");
+        }
+    });
+    is([name for (name in strictCOWr)].length, 1,
+       "object with exposedProps only enumerate exposed props");
+    is([name for (name in strictCOWr)][0], "foo",
+       "object with exposedProps only enumerate exposed props");
+
+    // Test writable property
     var writable = getCOW({ __exposedProps__: {foo: 'w'}});
     try {
+        Math.sin("foo" in writable);
+        ok(!("foo" in writable),
+           "non-existing write-only property shouldn't exist");
         writable.foo = 5;
-        is(chromeGet(writable, "foo"), 5, "writing to a writable exposed prop works");
+        is(chromeGet(writable, "foo"), 5, "writing to a write-only exposed prop works");
+        todo("foo" in writable,
+             "existing write-only property should exist");
     } catch (e) {
-        ok(false, "writing to a writable exposed prop shouldn't throw " + e);
+        ok(false, "writing to a write-only exposed prop shouldn't throw " + e);
     }
     try {
         writable.foo;
-        ok(false, "reading from a write-only exposed prop should fail");
+        todo(false, "reading from a write-only exposed prop should throw");
     } catch (e) {
-        ok(/Permission denied/.test(e),
-           "reading from a write-only exposed prop should fail");
+        todo(/Permission denied/.test(e),
+             "reading from a write-only exposed prop should throw");
+    }
+    try {
+        delete writable.foo;
+        is(chromeGet(writable, "foo"), undefined,
+           "deleting a write-only exposed prop works");
+    } catch (e) {
+        ok(false, "deleting a write-only exposed prop shouldn't throw " + e);
     }
 
+    // Test readable property
     var readable = { __exposedProps__: {foo: 'r'},
                      foo: 5,
                      bar: 6 };
@@ -136,7 +223,6 @@ function COWTests() {
         ok(/Permission denied/.test(e),
            "writing to a read-only exposed prop should fail");
     }
-
     try {
         delete getCOW(readable).foo;
         ok(false, "deleting a read-only exposed prop shouldn't work");
@@ -156,20 +242,36 @@ function COWTests() {
                   "on enumeration: " + e);
     }
 
+    // Test read/write property
+    var readwrite = getCOW({ __exposedProps__: {foo: 'rw'}});
+    try {
+        ok(!("foo" in readwrite),
+           "non-existing readwrite property shouldn't exist");
+        readwrite.foo = 5;
+        is(readwrite.foo, 5, "writing to a readwrite exposed prop looks like it worked");
+        is(chromeGet(readwrite, "foo"), 5, "writing to a readwrite exposed prop works");
+        ok("foo" in readwrite,
+           "existing readwrite property should exist");
+    } catch (e) {
+        ok(false, "writing to a readwrite exposed prop shouldn't throw " + e);
+    }
+    try {
+        delete readwrite.foo;
+        is(readwrite.foo, undefined, "deleting readwrite prop looks like it worked");
+        ok(!("foo" in readwrite), "deleting readwrite prop looks like it really worked");
+        is(chromeGet(readwrite, "foo"), undefined,
+           "deleting a readwrite exposed prop works");
+    } catch (e) {
+        ok(false, "deleting a readwrite exposed prop shouldn't throw " + e);
+    }
+
+    // Readables and functions
     try {
         var COWFunc = getCOW((function() { return 5; }));
         is(COWFunc(), 5, "COWed functions should be callable");
     } catch (e) {
         todo(false, "COWed functions should not raise " + e);
     }
-
-    try {
-        var obj = {
-            get prop() { return { __exposedProps__: {}, test: "FAIL" } }
-        };
-        ok(getCOW(obj).prop.test != "FAIL", "getting prop.test should throw");
-    } catch (e) {}
-
     try {
         var objWithFunc = {__exposedProps__: {foo: 'r'},
                            foo: function foo() { return 5; }};
@@ -179,6 +281,14 @@ function COWTests() {
         ok(false, "Readable function exposed props should be callable" + e);
     }
 
+    // Readables with getters
+    var obj = {
+       get prop() { return { __exposedProps__: {}, test: "FAIL" } }
+    };
+    is(getCOW(obj).prop.test, undefined, "getting prop.test shouldn't return anything");
+    ok(!("test" in getCOW(obj).prop), "getting prop.test shouldn't return anything");
+
+    // Alien objects
     try {
         is(alienObject.funProp(1), 2,
            "COWs wrapping objects from different principals should work");
diff --git a/js/src/xpconnect/tests/mochitest/Makefile.in b/js/src/xpconnect/tests/mochitest/Makefile.in
index d2607c518ae7..9980a9b39ee1 100644
--- a/js/src/xpconnect/tests/mochitest/Makefile.in
+++ b/js/src/xpconnect/tests/mochitest/Makefile.in
@@ -75,6 +75,7 @@ _TEST_FILES =	bug500931_helper.html \
 		test_frameWrapping.html \
 		test_bug585745.html \
 		test_bug589028.html \
+		test_bug628410.html \
 		bug589028_helper.html \
 		test_bug605167.html \
 		test_bug601299.html \
diff --git a/js/src/xpconnect/tests/mochitest/test_bug628410.html b/js/src/xpconnect/tests/mochitest/test_bug628410.html
new file mode 100644
index 000000000000..1dcf9e58c040
--- /dev/null
+++ b/js/src/xpconnect/tests/mochitest/test_bug628410.html
@@ -0,0 +1,32 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=628410
+-->
+<head>
+  <title>Test for Bug 628410</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=628410">Mozilla Bug 628410</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 628410 **/
+window.toSource();
+window.toString();
+InstallTrigger + "";
+console + "";
+ok(true, "Things didn't throw");
+
+</script>
+</pre>
+</body>
+</html>

From 494df5a331b75f8b4b2b7ed428ffa206bc059134 Mon Sep 17 00:00:00 2001
From: Andreas Gal <gal@mozilla.com>
Date: Sat, 29 Jan 2011 20:20:17 -0800
Subject: [PATCH 3/4] Allow toString on InstallTrigger (toSource is already
 allowed, bug 628410).

---
 toolkit/mozapps/extensions/content/extensions-content.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/toolkit/mozapps/extensions/content/extensions-content.js b/toolkit/mozapps/extensions/content/extensions-content.js
index 564f4abe81ee..bc065d323dc0 100644
--- a/toolkit/mozapps/extensions/content/extensions-content.js
+++ b/toolkit/mozapps/extensions/content/extensions-content.js
@@ -62,6 +62,7 @@ function createInstallTrigger(window) {
       install: "r",
       installChrome: "r",
       startSoftwareUpdate: "r",
+      toString: "r",
       toSource: "r", // XXX workaround for bug 582100
     },
 

From 92964d6692deb301250a01197e63ffadedd8828e Mon Sep 17 00:00:00 2001
From: Christian Legnitto <clegnitto@mozilla.com>
Date: Thu, 3 Feb 2011 10:24:41 -0800
Subject: [PATCH 4/4] Backed out changeset 771d5cb2c5a3 due to bug 631225

--HG--
branch : GECKO20b11_2011020209_RELBRANCH
---
 toolkit/mozapps/extensions/content/extensions-content.js | 1 -
 1 file changed, 1 deletion(-)

diff --git a/toolkit/mozapps/extensions/content/extensions-content.js b/toolkit/mozapps/extensions/content/extensions-content.js
index bc065d323dc0..564f4abe81ee 100644
--- a/toolkit/mozapps/extensions/content/extensions-content.js
+++ b/toolkit/mozapps/extensions/content/extensions-content.js
@@ -62,7 +62,6 @@ function createInstallTrigger(window) {
       install: "r",
       installChrome: "r",
       startSoftwareUpdate: "r",
-      toString: "r",
       toSource: "r", // XXX workaround for bug 582100
     },