Bug 683361, part 5 - Handle transparent proxies correctly in [[Class]] == "X" queries (r=waldo)
authorLuke Wagner <luke@mozilla.com>
Tue, 20 Sep 2011 16:48:50 -0700
changeset 79091 b4f351db9863b881323261ea5087c9f0f6fb4e60
parent 79090 90ff7402febc04ffdf63673e9f6d3d7c16d9c276
child 79092 95bbaf6cb2a6c9a4d3375da8381cb8db909ec4a0
push idunknown
push userunknown
push dateunknown
reviewerswaldo
bugs683361
milestone10.0a1
Bug 683361, part 5 - Handle transparent proxies correctly in [[Class]] == "X" queries (r=waldo)
js/src/jit-test/tests/basic/testCrossCompartmentTransparency2.js
js/src/jsapi.cpp
js/src/jsarray.cpp
js/src/jsbool.cpp
js/src/jsbool.h
js/src/jsobj.h
js/src/jsobjinlines.h
js/src/json.cpp
js/src/jsproxy.cpp
js/src/jsproxy.h
js/src/jswrapper.cpp
js/src/jswrapper.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/testCrossCompartmentTransparency2.js
@@ -0,0 +1,15 @@
+var g = newGlobal('new-compartment');
+
+var array = g.eval("new Array(1,2,3)");
+assertEq([array,array].concat().toString(), "1,2,3,1,2,3");
+assertEq(Array.isArray(array), true);
+
+var number = g.eval("new Number(42)");
+var bool = g.eval("new Boolean(false)");
+var string = g.eval("new String('ponies')");
+assertEq(JSON.stringify({n:number, b:bool, s:string}), "{\"n\":42,\"b\":false,\"s\":\"ponies\"}");
+assertEq(JSON.stringify({arr:array}), "{\"arr\":[1,2,3]}");
+assertEq(JSON.stringify({2:'ponies', unicorns:'not real'}, array), "{\"2\":\"ponies\"}");
+assertEq(JSON.stringify({42:true, ponies:true, unicorns:'sad'}, [number, string]), "{\"42\":true,\"ponies\":true}");
+assertEq(JSON.stringify({a:true,b:false}, undefined, number), "{\n          \"a\": true,\n          \"b\": false\n}");
+assertEq(JSON.stringify({a:true,b:false}, undefined, string), "{\nponies\"a\": true,\nponies\"b\": false\n}");
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -4079,18 +4079,17 @@ JS_NewArrayObject(JSContext *cx, jsint l
     assertSameCompartment(cx, JSValueArray(vector, vector ? (jsuint)length : 0));
     return NewDenseCopiedArray(cx, (jsuint)length, vector);
 }
 
 JS_PUBLIC_API(JSBool)
 JS_IsArrayObject(JSContext *cx, JSObject *obj)
 {
     assertSameCompartment(cx, obj);
-    return obj->isArray() ||
-           (obj->isWrapper() && obj->unwrap()->isArray());
+    return ObjectClassIs(*obj, ESClass_Array, cx);
 }
 
 JS_PUBLIC_API(JSBool)
 JS_GetArrayLength(JSContext *cx, JSObject *obj, jsuint *lengthp)
 {
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, obj);
     return js_GetLengthProperty(cx, obj, lengthp);
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -2883,25 +2883,25 @@ array_concat(JSContext *cx, uintN argc, 
     }
 
     /* Loop over [0, argc] to concat args into nobj, expanding all Arrays. */
     for (uintN i = 0; i <= argc; i++) {
         if (!JS_CHECK_OPERATION_LIMIT(cx))
             return false;
         const Value &v = p[i];
         if (v.isObject()) {
-            aobj = &v.toObject();
-            if (aobj->isArray() || (aobj->isWrapper() && aobj->unwrap()->isArray())) {
+            JSObject &obj = v.toObject();
+            if (ObjectClassIs(obj, ESClass_Array, cx)) {
                 jsuint alength;
-                if (!js_GetLengthProperty(cx, aobj, &alength))
+                if (!js_GetLengthProperty(cx, &obj, &alength))
                     return false;
                 for (uint32 slot = 0; slot < alength; slot++) {
                     JSBool hole;
                     Value tmp;
-                    if (!JS_CHECK_OPERATION_LIMIT(cx) || !GetElement(cx, aobj, slot, &hole, &tmp))
+                    if (!JS_CHECK_OPERATION_LIMIT(cx) || !GetElement(cx, &obj, slot, &hole, &tmp))
                         return false;
 
                     /*
                      * Per ECMA 262, 15.4.4.4, step 9, ignore nonexistent
                      * properties.
                      */
                     if (!hole && !SetArrayElement(cx, nobj, length + slot, tmp))
                         return false;
@@ -3331,21 +3331,21 @@ static JSBool
 array_every(JSContext *cx, uintN argc, Value *vp)
 {
     return array_extra(cx, EVERY, argc, vp);
 }
 
 static JSBool
 array_isArray(JSContext *cx, uintN argc, Value *vp)
 {
-    JSObject *obj;
-    vp->setBoolean(argc > 0 &&
-                   vp[2].isObject() &&
-                   ((obj = &vp[2].toObject())->isArray() ||
-                    (obj->isWrapper() && obj->unwrap()->isArray())));
+    CallArgs args = CallArgsFromVp(argc, vp);
+    bool isArray = args.length() > 0 &&
+                   args[0].isObject() &&
+                   ObjectClassIs(args[0].toObject(), ESClass_Array, cx);
+    args.rval().setBoolean(isArray);
     return true;
 }
 
 #define GENERIC JSFUN_GENERIC_NATIVE
 
 static JSFunctionSpec array_methods[] = {
 #if JS_HAS_TOSOURCE
     JS_FN(js_toSource_str,      array_toSource,     0,0),
--- a/js/src/jsbool.cpp
+++ b/js/src/jsbool.cpp
@@ -187,16 +187,43 @@ js_BooleanToString(JSContext *cx, JSBool
 
 /* This function implements E-262-3 section 9.8, toString. */
 bool
 js::BooleanToStringBuffer(JSContext *cx, JSBool b, StringBuffer &sb)
 {
     return b ? sb.append("true") : sb.append("false");
 }
 
+namespace js {
+
+bool
+BooleanGetPrimitiveValueSlow(JSContext *cx, JSObject &obj, Value *vp)
+{
+    JS_ASSERT(ObjectClassIs(obj, ESClass_Boolean, cx));
+    JS_ASSERT(obj.isProxy());
+
+    /*
+     * To respect the proxy abstraction, we can't simply unwrap and call
+     * getPrimitiveThis on the wrapped object. All we know is that obj says
+     * its [[Class]] is "Boolean". Boolean.prototype.valueOf is specified to
+     * return the [[PrimitiveValue]] internal property, so call that instead.
+     */
+    InvokeArgsGuard args;
+    if (!cx->stack.pushInvokeArgs(cx, 0, &args))
+        return false;
+    args.calleev().setUndefined();
+    args.thisv().setObject(obj);
+    if (!obj.getProxyHandler()->nativeCall(cx, &obj, &BooleanClass, bool_valueOf, args))
+        return false;
+    *vp = args.rval();
+    return true;
+}
+
+}  /* namespace js */
+
 JSBool
 js_ValueToBoolean(const Value &v)
 {
     if (v.isInt32())
         return v.toInt32() != 0;
     if (v.isString())
         return v.toString()->length() != 0;
     if (v.isObject())
--- a/js/src/jsbool.h
+++ b/js/src/jsbool.h
@@ -52,14 +52,26 @@ js_InitBooleanClass(JSContext *cx, JSObj
 extern JSString *
 js_BooleanToString(JSContext *cx, JSBool b);
 
 namespace js {
 
 extern bool
 BooleanToStringBuffer(JSContext *cx, JSBool b, StringBuffer &sb);
 
+inline bool
+BooleanGetPrimitiveValue(JSContext *cx, JSObject &obj, Value *vp)
+{
+    if (obj.isBoolean()) {
+        *vp = obj.getPrimitiveThis();
+        return true;
+    }
+
+    extern bool BooleanGetPrimitiveValueSlow(JSContext *, JSObject &, Value *);
+    return BooleanGetPrimitiveValueSlow(cx, obj, vp);
+}
+
 } /* namespace js */
 
 extern JSBool
 js_ValueToBoolean(const js::Value &v);
 
 #endif /* jsbool_h___ */
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -2295,11 +2295,26 @@ HandleNonGenericMethodClassMismatch(JSCo
  * Boolean, Number, and String (e.g., ES5 15.6.4.2). If 'true' is returned, the
  * extracted primitive is stored in |*v|. If 'false' is returned, the caller
  * must immediately 'return *ok'. For details, see NonGenericMethodGuard.
  */
 template <typename T>
 inline bool
 BoxedPrimitiveMethodGuard(JSContext *cx, CallArgs args, T *v, bool *ok);
 
+/*
+ * Enumeration describing possible values of the [[Class]] internal property
+ * value of objects.
+ */
+enum ESClassValue { ESClass_Array, ESClass_Number, ESClass_String, ESClass_Boolean };
+
+/*
+ * Return whether the given object has the given [[Class]] internal property
+ * value. Beware, this query says nothing about the js::Class of the JSObject
+ * so the caller must not assume anything about obj's representation (e.g., obj
+ * may be a proxy).
+ */
+inline bool
+ObjectClassIs(JSObject &obj, ESClassValue classValue, JSContext *cx);
+
 }  /* namespace js */
 
 #endif /* jsobj_h___ */
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -1742,16 +1742,32 @@ BoxedPrimitiveMethodGuard(JSContext *cx,
 
     if (!NonGenericMethodGuard(cx, args, Behavior::getClass(), ok))
         return false;
 
     *v = Behavior::extract(thisv.toObject().getPrimitiveThis());
     return true;
 }
 
+inline bool
+ObjectClassIs(JSObject &obj, ESClassValue classValue, JSContext *cx)
+{
+    if (JS_UNLIKELY(obj.isProxy()))
+        return obj.getProxyHandler()->classPropertyIs(cx, &obj, classValue);
+
+    switch (classValue) {
+      case ESClass_Array: return obj.isArray();
+      case ESClass_Number: return obj.isNumber();
+      case ESClass_String: return obj.isString();
+      case ESClass_Boolean: return obj.isBoolean();
+    }
+    JS_NOT_REACHED("bad classValue");
+    return false;
+}
+
 } /* namespace js */
 
 inline JSObject *
 js_GetProtoIfDenseArray(JSObject *obj)
 {
     return obj->isDenseArray() ? obj->getProto() : obj;
 }
 
--- a/js/src/json.cpp
+++ b/js/src/json.cpp
@@ -357,30 +357,30 @@ PreprocessValue(JSContext *cx, JSObject 
 
         if (!Invoke(cx, args))
             return false;
         *vp = args.rval();
     }
 
     /* Step 4. */
     if (vp->isObject()) {
-        JSObject *obj = &vp->toObject();
-        Class *clasp = obj->getClass();
-        if (clasp == &NumberClass) {
+        JSObject &obj = vp->toObject();
+        if (ObjectClassIs(obj, ESClass_Number, cx)) {
             double d;
             if (!ToNumber(cx, *vp, &d))
                 return false;
             vp->setNumber(d);
-        } else if (clasp == &StringClass) {
+        } else if (ObjectClassIs(obj, ESClass_String, cx)) {
             JSString *str = js_ValueToString(cx, *vp);
             if (!str)
                 return false;
             vp->setString(str);
-        } else if (clasp == &BooleanClass) {
-            *vp = obj->getPrimitiveThis();
+        } else if (ObjectClassIs(obj, ESClass_Boolean, cx)) {
+            if (!BooleanGetPrimitiveValue(cx, obj, vp))
+                return false;
             JS_ASSERT(vp->isBoolean());
         }
     }
 
     return true;
 }
 
 /*
@@ -596,35 +596,39 @@ Str(JSContext *cx, const Value &v, Strin
         if (!NumberValueToStringBuffer(cx, v, sb))
             return false;
 
         return scx->sb.append(sb.begin(), sb.length());
     }
 
     /* Step 10. */
     JS_ASSERT(v.isObject());
-    JSBool ok;
+    JSObject *obj = &v.toObject();
 
     scx->depth++;
-    ok = (JS_IsArrayObject(cx, &v.toObject()) ? JA : JO)(cx, &v.toObject(), scx);
+    JSBool ok;
+    if (ObjectClassIs(v.toObject(), ESClass_Array, cx))
+        ok = JA(cx, obj, scx);
+    else
+        ok = JO(cx, obj, scx);
     scx->depth--;
 
     return ok;
 }
 
 /* ES5 15.12.3. */
 JSBool
 js_Stringify(JSContext *cx, Value *vp, JSObject *replacer, Value space, StringBuffer &sb)
 {
     /* Step 4. */
     AutoIdVector propertyList(cx);
     if (replacer) {
         if (replacer->isCallable()) {
             /* Step 4a(i): use replacer to transform values.  */
-        } else if (JS_IsArrayObject(cx, replacer)) {
+        } else if (ObjectClassIs(*replacer, ESClass_Array, cx)) {
             /*
              * Step 4b: The spec algorithm is unhelpfully vague about the exact
              * steps taken when the replacer is an array, regarding the exact
              * sequence of [[Get]] calls for the array's elements, when its
              * overall length is calculated, whether own or own plus inherited
              * properties are considered, and so on.  A rewrite was proposed in
              * <https://mail.mozilla.org/pipermail/es5-discuss/2011-April/003976.html>,
              * whose steps are copied below, and which are implemented here.
@@ -675,17 +679,19 @@ js_Stringify(JSContext *cx, Value *vp, J
                     if (v.isNumber() && ValueFitsInInt32(v, &n) && INT_FITS_IN_JSID(n)) {
                         id = INT_TO_JSID(n);
                     } else {
                         if (!js_ValueToStringId(cx, v, &id))
                             return false;
                         id = js_CheckForStringIndex(id);
                     }
                 } else if (v.isString() ||
-                           (v.isObject() && (v.toObject().isString() || v.toObject().isNumber())))
+                           (v.isObject() &&
+                            (ObjectClassIs(v.toObject(), ESClass_String, cx) ||
+                             ObjectClassIs(v.toObject(), ESClass_Number, cx))))
                 {
                     /* Step 4b(iv)(3), 4b(iv)(5). */
                     if (!js_ValueToStringId(cx, v, &id))
                         return false;
                     id = js_CheckForStringIndex(id);
                 } else {
                     continue;
                 }
@@ -701,22 +707,22 @@ js_Stringify(JSContext *cx, Value *vp, J
         } else {
             replacer = NULL;
         }
     }
 
     /* Step 5. */
     if (space.isObject()) {
         JSObject &spaceObj = space.toObject();
-        if (spaceObj.isNumber()) {
+        if (ObjectClassIs(spaceObj, ESClass_Number, cx)) {
             jsdouble d;
             if (!ToNumber(cx, space, &d))
                 return false;
             space = NumberValue(d);
-        } else if (spaceObj.isString()) {
+        } else if (ObjectClassIs(spaceObj, ESClass_String, cx)) {
             JSString *str = js_ValueToString(cx, space);
             if (!str)
                 return false;
             space = StringValue(str);
         }
     }
 
     StringBuffer gap(cx);
@@ -778,16 +784,18 @@ Walk(JSContext *cx, JSObject *holder, js
     Value val;
     if (!holder->getGeneric(cx, name, &val))
         return false;
 
     /* Step 2. */
     if (val.isObject()) {
         JSObject *obj = &val.toObject();
 
+        /* 'val' must have been produced by the JSON parser, so not a proxy. */
+        JS_ASSERT(!obj->isProxy());
         if (obj->isArray()) {
             /* Step 2a(ii). */
             jsuint length = obj->getArrayLength();
 
             /* Step 2a(i), 2a(iii-iv). */
             for (jsuint i = 0; i < length; i++) {
                 jsid id;
                 if (!IndexToId(cx, i, &id))
--- a/js/src/jsproxy.cpp
+++ b/js/src/jsproxy.cpp
@@ -308,16 +308,23 @@ ProxyHandler::hasInstance(JSContext *cx,
 
 JSType
 ProxyHandler::typeOf(JSContext *cx, JSObject *proxy)
 {
     JS_ASSERT(OperationInProgress(cx, proxy));
     return proxy->isFunctionProxy() ? JSTYPE_FUNCTION : JSTYPE_OBJECT;
 }
 
+bool
+ProxyHandler::classPropertyIs(JSContext *cx, JSObject *proxy, ESClassValue classValue)
+{
+    JS_ASSERT(OperationInProgress(cx, proxy));
+    return false;
+}
+
 void
 ProxyHandler::finalize(JSContext *cx, JSObject *proxy)
 {
 }
 
 void
 ProxyHandler::trace(JSTracer *trc, JSObject *proxy)
 {
--- a/js/src/jsproxy.h
+++ b/js/src/jsproxy.h
@@ -77,16 +77,17 @@ class JS_FRIEND_API(ProxyHandler) {
     virtual bool iterate(JSContext *cx, JSObject *proxy, uintN flags, Value *vp);
 
     /* Spidermonkey extensions. */
     virtual bool call(JSContext *cx, JSObject *proxy, uintN argc, Value *vp);
     virtual bool construct(JSContext *cx, JSObject *proxy, uintN argc, Value *argv, Value *rval);
     virtual bool nativeCall(JSContext *cx, JSObject *proxy, Class *clasp, Native native, CallArgs args);
     virtual bool hasInstance(JSContext *cx, JSObject *proxy, const Value *vp, bool *bp);
     virtual JSType typeOf(JSContext *cx, JSObject *proxy);
+    virtual bool classPropertyIs(JSContext *cx, JSObject *obj, ESClassValue classValue);
     virtual JSString *obj_toString(JSContext *cx, JSObject *proxy);
     virtual JSString *fun_toString(JSContext *cx, JSObject *proxy, uintN indent);
     virtual bool defaultValue(JSContext *cx, JSObject *obj, JSType hint, Value *vp);
     virtual void finalize(JSContext *cx, JSObject *proxy);
     virtual void trace(JSTracer *trc, JSObject *proxy);
 
     virtual bool isOuterWindow() {
         return false;
--- a/js/src/jswrapper.cpp
+++ b/js/src/jswrapper.cpp
@@ -286,16 +286,22 @@ Wrapper::hasInstance(JSContext *cx, JSOb
 }
 
 JSType
 Wrapper::typeOf(JSContext *cx, JSObject *wrapper)
 {
     return TypeOfValue(cx, ObjectValue(*wrappedObject(wrapper)));
 }
 
+bool
+Wrapper::classPropertyIs(JSContext *cx, JSObject *wrapper, ESClassValue classValue)
+{
+    return ObjectClassIs(*wrappedObject(wrapper), classValue, cx);
+}
+
 JSString *
 Wrapper::obj_toString(JSContext *cx, JSObject *wrapper)
 {
     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]");
@@ -732,17 +738,18 @@ CrossCompartmentWrapper::construct(JSCon
 
     call.leave();
     return call.origin->wrap(cx, rval);
 }
 
 bool
 CrossCompartmentWrapper::nativeCall(JSContext *cx, JSObject *wrapper, Class *clasp, Native native, CallArgs srcArgs)
 {
-    JS_ASSERT(srcArgs.callee().getFunctionPrivate()->native() == native);
+    JS_ASSERT_IF(!srcArgs.calleev().isUndefined(),
+                 srcArgs.callee().getFunctionPrivate()->native() == native);
     JS_ASSERT(&srcArgs.thisv().toObject() == wrapper);
     JS_ASSERT(!wrapper->unwrap(NULL)->isProxy());
 
     JSObject *wrapped = wrappedObject(wrapper);
     AutoCompartment call(cx, wrapped);
     if (!call.enter())
         return false;
 
--- a/js/src/jswrapper.h
+++ b/js/src/jswrapper.h
@@ -82,16 +82,17 @@ class JS_FRIEND_API(Wrapper) : public Pr
     virtual bool iterate(JSContext *cx, JSObject *wrapper, uintN flags, Value *vp);
 
     /* Spidermonkey extensions. */
     virtual bool call(JSContext *cx, JSObject *wrapper, uintN argc, Value *vp);
     virtual bool construct(JSContext *cx, JSObject *wrapper, uintN argc, Value *argv, Value *rval);
     virtual bool nativeCall(JSContext *cx, JSObject *wrapper, Class *clasp, Native native, CallArgs args);
     virtual bool hasInstance(JSContext *cx, JSObject *wrapper, const Value *vp, bool *bp);
     virtual JSType typeOf(JSContext *cx, JSObject *proxy);
+    virtual bool classPropertyIs(JSContext *cx, JSObject *obj, ESClassValue classValue);
     virtual JSString *obj_toString(JSContext *cx, JSObject *wrapper);
     virtual JSString *fun_toString(JSContext *cx, JSObject *wrapper, uintN indent);
     virtual bool defaultValue(JSContext *cx, JSObject *wrapper, JSType hint, Value *vp);
 
     virtual void trace(JSTracer *trc, JSObject *wrapper);
 
     /* Policy enforcement traps. */
     enum Action { GET, SET, CALL };