Bug 891585. Don't JS_WrapValue same-compartment typed array return values in bindings code (e.g. ImageData.data). r=peterv

dom/bindings/BindingUtils.h

@@ -528,23 +528,27 @@ SetSystemOnlyWrapper(JSObject* obj, nsWrapperCache* cache, JSObject& wrapper)
   cache->SetHasSystemOnlyWrapper();
 }
 
-// If rval is a gcthing and is not in the compartment of cx, wrap rval
-// into the compartment of cx (typically by replacing it with an Xray or
-// cross-compartment wrapper around the original object).
-MOZ_ALWAYS_INLINE bool
-MaybeWrapValue(JSContext* cx, JS::MutableHandle<JS::Value> rval)
+// Make sure to wrap the given string value into the right compartment, as
+// needed.
+MOZ_ALWAYS_INLINE
+bool
+MaybeWrapStringValue(JSContext* cx, JS::MutableHandle<JS::Value> rval)
 {
-  if (rval.isString()) {
-    JSString* str = rval.toString();
-    if (JS::GetGCThingZone(str) != js::GetContextZone(cx)) {
-      return JS_WrapValue(cx, rval.address());
-    }
-    return true;
+  MOZ_ASSERT(rval.isString());
+  JSString* str = rval.toString();
+  if (JS::GetGCThingZone(str) != js::GetContextZone(cx)) {
+    return JS_WrapValue(cx, rval.address());
   }
+  return true;
+}
 
-  if (!rval.isObject()) {
-    return true;
-  }
+// Make sure to wrap the given object value into the right compartment as
+// needed.  This will work correctly, but possibly slowly, on all objects.
+MOZ_ALWAYS_INLINE
+bool
+MaybeWrapObjectValue(JSContext* cx, JS::MutableHandle<JS::Value> rval)
+{
+  MOZ_ASSERT(rval.isObject());
 
   JSObject* obj = &rval.toObject();
   if (js::GetObjectCompartment(obj) != js::GetContextCompartment(cx)) {
@@ -559,9 +563,69 @@ MaybeWrapValue(JSContext* cx, JS::MutableHandle<JS::Value> rval)
     return true;
   }
 
+  // It's not a WebIDL object.  But it might be an XPConnect one, in which case
+  // we may need to outerize here, so make sure to call JS_WrapValue.
   return JS_WrapValue(cx, rval.address());
 }
 
+// Like MaybeWrapObjectValue, but also allows null
+MOZ_ALWAYS_INLINE
+bool
+MaybeWrapObjectOrNullValue(JSContext* cx, JS::MutableHandle<JS::Value> rval)
+{
+  MOZ_ASSERT(rval.isObjectOrNull());
+  if (rval.isNull()) {
+    return true;
+  }
+  return MaybeWrapObjectValue(cx, rval);
+}
+
+// Wrapping for objects that are known to not be DOM or XPConnect objects
+MOZ_ALWAYS_INLINE
+bool
+MaybeWrapNonDOMObjectValue(JSContext* cx, JS::MutableHandle<JS::Value> rval)
+{
+  MOZ_ASSERT(rval.isObject());
+  MOZ_ASSERT(!GetDOMClass(&rval.toObject()));
+  MOZ_ASSERT(!(js::GetObjectClass(&rval.toObject())->flags &
+               JSCLASS_PRIVATE_IS_NSISUPPORTS));
+
+  JSObject* obj = &rval.toObject();
+  if (js::GetObjectCompartment(obj) == js::GetContextCompartment(cx)) {
+    return true;
+  }
+  return JS_WrapValue(cx, rval.address());
+}
+
+// Like MaybeWrapNonDOMObjectValue but allows null
+MOZ_ALWAYS_INLINE
+bool
+MaybeWrapNonDOMObjectOrNullValue(JSContext* cx, JS::MutableHandle<JS::Value> rval)
+{
+  MOZ_ASSERT(rval.isObjectOrNull());
+  if (rval.isNull()) {
+    return true;
+  }
+  return MaybeWrapNonDOMObjectValue(cx, rval);
+}
+
+// If rval is a gcthing and is not in the compartment of cx, wrap rval
+// into the compartment of cx (typically by replacing it with an Xray or
+// cross-compartment wrapper around the original object).
+MOZ_ALWAYS_INLINE bool
+MaybeWrapValue(JSContext* cx, JS::MutableHandle<JS::Value> rval)
+{
+  if (rval.isString()) {
+    return MaybeWrapStringValue(cx, rval);
+  }
+
+  if (!rval.isObject()) {
+    return true;
+  }
+
+  return MaybeWrapObjectValue(cx, rval);
+}
+
 static inline void
 WrapNewBindingForSameCompartment(JSContext* cx, JSObject* obj, void* value,
                                  JS::MutableHandle<JS::Value> rval)

dom/bindings/Codegen.py

@@ -3887,16 +3887,31 @@ def getWrapTemplateForType(type, descriptorProvider, result, successCode,
     # if body.
     exceptionCodeIndented = CGIndenter(CGGeneric(exceptionCode))
 
-    def setValue(value, callWrapValue=False):
+    def setValue(value, callWrapValue=None):
         """
-        Returns the code to set the jsval to value. If "callWrapValue" is true
-        MaybeWrapValue will be called on the jsval.
+        Returns the code to set the jsval to value.
+
+        "callWrapValue" can be set to the following values:
+          * None: no wrapping will be done.
+          * "object": will wrap using MaybeWrapObjectValue.
+          * "objectOrNull": will wrap using MaybeWrapObjectOrNullValue.
+          * "nonDOMObject": will wrap using MaybeWrapNonDOMObjectValue.
+          * "nonDOMObjectOrNull": will wrap using MaybeWrapNonDOMObjectOrNullValue
+          * "value": will wrap using MaybeWrapValue.
         """
         if not callWrapValue:
             tail = successCode
         else:
-            tail = ("if (!MaybeWrapValue(cx, ${jsvalHandle})) {\n" +
-                    ("%s\n" % exceptionCodeIndented.define()) +
+            methodMap = {
+                "object": "MaybeWrapObjectValue",
+                "objectOrNull": "MaybeWrapObjectOrNullValue",
+                "nonDOMObject": "MaybeWrapNonDOMObjectValue",
+                "nonDOMObjectOrNull": "MaybeWrapNonDOMObjectOrNullValue",
+                "value": "MaybeWrapValue"
+                }
+            tail = (("if (!%s(cx, ${jsvalHandle})) {\n"
+                     "%s\n" % (methodMap[callWrapValue],
+                               exceptionCodeIndented.define())) +
                     "}\n" +
                     successCode)
         return ("${jsvalRef}.set(%s);\n" +
@@ -4068,11 +4083,13 @@ if (!returnArray) {
         # to wrap here.
         # NB: setValue(..., True) calls JS_WrapValue(), so is fallible
         if descriptorProvider.workers:
-            return (setValue("JS::ObjectOrNullValue(%s)" % result, True), False)
+            return (setValue("JS::ObjectOrNullValue(%s)" % result,
+                             "objectOrNull"),
+                    False)
 
         wrapCode = setValue(
             "JS::ObjectValue(*GetCallbackFromCallbackObject(%(result)s))",
-            True)
+            "object")
         if type.nullable():
             wrapCode = (
                 "if (%(result)s) {\n" +
@@ -4087,17 +4104,25 @@ if (!returnArray) {
         # See comments in WrapNewBindingObject explaining why we need
         # to wrap here.
         # NB: setValue(..., True) calls JS_WrapValue(), so is fallible
-        return (setValue(result, True), False)
+        return (setValue(result, "value"), False)
 
     if type.isObject() or type.isSpiderMonkeyInterface():
         # See comments in WrapNewBindingObject explaining why we need
         # to wrap here.
         if type.nullable():
             toValue = "JS::ObjectOrNullValue(%s)"
+            if type.isSpiderMonkeyInterface():
+                wrapType = "nonDOMObjectOrNull"
+            else:
+                wrapType = "objectOrNull"
         else:
             toValue = "JS::ObjectValue(*%s)"
+            if type.isSpiderMonkeyInterface():
+                wrapType = "nonDOMObject"
+            else:
+                wrapType = "object"
         # NB: setValue(..., True) calls JS_WrapValue(), so is fallible
-        return (setValue(toValue % result, True), False)
+        return (setValue(toValue % result, wrapType), False)
 
     if type.isUnion():
         if type.nullable():