Bug 720619 - Attempt a puncture for the [[DefaultValue]] trap; r=bholley

js/src/jswrapper.cpp

@@ -200,6 +200,43 @@ IndirectWrapper::enumerate(JSContext *cx, JSObject *wrapper, AutoIdVector &props
     GET(IndirectProxyHandler::enumerate(cx, wrapper, props));
 }
 
+/*
+ * Ordinarily, the convert trap would require a PUNCTURE. However, the default
+ * implementation of convert, JS_ConvertStub, obtains a default value by calling
+ * the toString/valueOf method on the wrapper, if any. Doing a PUNCTURE in this
+ * case would be overly conservative. To make matters worse, XPConnect sometimes
+ * installs a custom convert trap that obtains a default value by calling the
+ * toString method on the wrapper. Doing a puncture in this case would be overly
+ * conservative as well. We deal with these anomalies by clearing the pending
+ * exception and falling back to the DefaultValue algorithm whenever the
+ * PUNCTURE fails.
+ */
+bool
+IndirectWrapper::defaultValue(JSContext *cx, JSObject *wrapper_, JSType hint, Value *vp)
+{
+    RootedObject wrapper(cx, wrapper_);
+
+    bool status;
+    if (!enter(cx, wrapper_, JSID_VOID, PUNCTURE, &status)) {
+        RootedValue v(cx);
+        JS_ClearPendingException(cx);
+        if (!DefaultValue(cx, wrapper, hint, &v))
+            return false;
+        *vp = v;
+        return true;
+    }
+    /*
+     * We enter the compartment of the wrappee here, even if we're not a cross
+     * compartment wrapper. Moreover, cross compartment wrappers do not enter
+     * the compartment of the wrappee before calling this function. This is
+     * necessary because the DefaultValue algorithm above operates on the
+     * wrapper, not the wrappee, so we want to delay the decision to switch
+     * compartments until this point.
+     */
+    AutoCompartment call(cx, wrappedObject(wrapper));
+    return IndirectProxyHandler::defaultValue(cx, wrapper_, hint, vp);
+}
+
 DirectWrapper::DirectWrapper(unsigned flags, bool hasPrototype) : Wrapper(flags),
         DirectProxyHandler(&sWrapperFamily)
 {
@@ -262,6 +299,43 @@ DirectWrapper::enumerate(JSContext *cx, JSObject *wrapper, AutoIdVector &props)
     GET(DirectProxyHandler::enumerate(cx, wrapper, props));
 }
 
+/*
+ * Ordinarily, the convert trap would require a PUNCTURE. However, the default
+ * implementation of convert, JS_ConvertStub, obtains a default value by calling
+ * the toString/valueOf method on the wrapper, if any. Doing a PUNCTURE in this
+ * case would be overly conservative. To make matters worse, XPConnect sometimes
+ * installs a custom convert trap that obtains a default value by calling the
+ * toString method on the wrapper. Doing a puncture in this case would be overly
+ * conservative as well. We deal with these anomalies by clearing the pending
+ * exception and falling back to the DefaultValue algorithm whenever the
+ * PUNCTURE fails.
+ */
+bool
+DirectWrapper::defaultValue(JSContext *cx, JSObject *wrapper_, JSType hint, Value *vp)
+{
+    RootedObject wrapper(cx, wrapper_);
+
+    bool status;
+    if (!enter(cx, wrapper_, JSID_VOID, PUNCTURE, &status)) {
+        RootedValue v(cx);
+        JS_ClearPendingException(cx);
+        if (!DefaultValue(cx, wrapper, hint, &v))
+            return false;
+        *vp = v;
+        return true;
+    }
+    /*
+     * We enter the compartment of the wrappee here, even if we're not a cross
+     * compartment wrapper. Moreover, cross compartment wrappers do not enter
+     * the compartment of the wrappee before calling this function. This is
+     * necessary because the DefaultValue algorithm above operates on the
+     * wrapper, not the wrappee, so we want to delay the decision to switch
+     * compartments until this point.
+     */
+    AutoCompartment call(cx, wrappedObject(wrapper));
+    return DirectProxyHandler::defaultValue(cx, wrapper_, hint, vp);
+}
+
 bool
 DirectWrapper::has(JSContext *cx, JSObject *wrapper, jsid id, bool *bp)
 {
@@ -752,11 +826,8 @@ CrossCompartmentWrapper::regexp_toShared(JSContext *cx, JSObject *wrapper, RegEx
 bool
 CrossCompartmentWrapper::defaultValue(JSContext *cx, JSObject *wrapper, JSType hint, Value *vp)
 {
-    {
+    if (!DirectWrapper::defaultValue(cx, wrapper, hint, vp))
-        AutoCompartment call(cx, wrappedObject(wrapper));
-        if (!IndirectProxyHandler::defaultValue(cx, wrapper, hint, vp))
         return false;
-    }
     return cx->compartment->wrap(cx, vp);
 }
 

js/src/jswrapper.h

@@ -152,6 +152,10 @@ class JS_FRIEND_API(IndirectWrapper) : public Wrapper,
                          bool *bp) MOZ_OVERRIDE;
     virtual bool enumerate(JSContext *cx, JSObject *wrapper,
                            AutoIdVector &props) MOZ_OVERRIDE;
+
+    /* Spidermonkey extensions. */
+    virtual bool defaultValue(JSContext *cx, JSObject *wrapper_, JSType hint,
+                              Value *vp) MOZ_OVERRIDE;
 };
 
 /*
@@ -207,6 +211,8 @@ class JS_FRIEND_API(DirectWrapper) : public Wrapper, public DirectProxyHandler
     virtual bool hasInstance(JSContext *cx, HandleObject wrapper, MutableHandleValue v, bool *bp) MOZ_OVERRIDE;
     virtual JSString *obj_toString(JSContext *cx, JSObject *wrapper) MOZ_OVERRIDE;
     virtual JSString *fun_toString(JSContext *cx, JSObject *wrapper, unsigned indent) MOZ_OVERRIDE;
+    virtual bool defaultValue(JSContext *cx, JSObject *wrapper_, JSType hint,
+                              Value *vp) MOZ_OVERRIDE;
 
     static DirectWrapper singleton;
     static DirectWrapper singletonWithPrototype;

js/xpconnect/tests/chrome/Makefile.in

@@ -59,6 +59,7 @@ MOCHITEST_CHROME_FILES = \
 		test_weakmaps.xul \
 		test_weakref.xul \
 		test_wrappers.xul \
+		test_bug720619.xul \
 		$(NULL)
 
 # Disabled until this test gets updated to test the new proxy based

js/xpconnect/tests/chrome/test_bug720619.xul

@@ -0,0 +1,49 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css"
+                 type="text/css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=720619
+-->
+<window title="Mozilla Bug 720619"
+  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+
+  <!-- test results are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml">
+  <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=720619"
+     target="_blank">Mozilla Bug 720619</a>
+
+  <!-- test code goes here -->
+  <script type="application/javascript"><![CDATA[
+  /** Test for Bug 720619 **/
+  const Cu = Components.utils;
+
+  var obj = {
+    valueOf: function () {
+      return 42;
+    },
+    toString: function () {
+      return 'str';
+    }
+  };
+
+  var content = new Cu.Sandbox("about:blank");
+  content.obj = obj;
+
+  ok(Cu.evalInSandbox("obj + ''", content) == "[object Object]");
+  ok(Cu.evalInSandbox("'' + obj", content) == "[object Object]");
+  ok(isNaN(Cu.evalInSandbox("obj - 0", content)));
+  ok(Cu.evalInSandbox("String(obj)", content) == "[object Object]");
+
+  var chrome = new Cu.Sandbox(window);
+  chrome.obj = obj;
+
+  ok(Cu.evalInSandbox("obj + ''", chrome) == "42");
+  ok(Cu.evalInSandbox("'' + obj", chrome) == "42");
+  ok(Cu.evalInSandbox("obj - 0", chrome) == 42);
+  ok(Cu.evalInSandbox("String(obj)", chrome) == "str");
+  ]]></script>
+  </body>
+</window>

js/xpconnect/tests/mochitest/Makefile.in

@@ -75,6 +75,8 @@ MOCHITEST_FILES =	bug500931_helper.html \
 		test_lookupMethod.html \
 		file_bug738244.html \
 		file_mozMatchesSelector.html \
+		file_bug720619.html \
+		test_bug720619.html \
 		$(NULL)
 
 MOCHITEST_CHROME_FILES	= \

js/xpconnect/tests/mochitest/file_bug720619.html

@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <script>
+            valueOf = function() { return "v"; }
+            toString = function() { return "s"; }
+        </script>
+    </head>
+    <body></body>
+</html>

js/xpconnect/tests/mochitest/test_bug720619.html

@@ -0,0 +1,55 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=720619
+-->
+<head>
+  <title>Test for Bug 629227</title>
+  <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=720619">Mozilla Bug 720619</a>
+<p id="display">
+  <iframe id="testTarget"></iframe>
+</p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 720619 **/
+SimpleTest.waitForExplicitFinish();
+
+function checkThrows(f, exception) {
+    try {
+        f();
+        ok(false, "should have thrown: " + f);
+    } catch (e) {
+        ok(exception.test(e.toString()), "correctly threw");
+    }
+}
+
+function go() {
+    var loc = $('ifr').contentWindow.location;
+    checkThrows(function() {loc + '';}, /Permission denied/);
+    checkThrows(function() {'' + loc;}, /Permission denied/);
+    checkThrows(function() {String(loc);}, /Permission denied/);
+
+    var win = $('ifr').contentWindow;
+    checkThrows(function() {win + '';}, /Permission denied/);
+    checkThrows(function() {'' + win;}, /Permission denied/);
+    checkThrows(function() {String(win);}, /Permission denied/);
+
+    SimpleTest.finish();
+}
+
+</script>
+
+<iframe id="ifr" onload="go()"
+        src="http://example.org/tests/js/xpconnect/tests/mochitest/file_bug720619.html">
+</iframe>
+</pre>
+</body>
+</html>