Bug 720619 - Attempt a puncture for the [[DefaultValue]] trap; r=bholley
authorEddy Bruel <ejpbruel@mozilla.com>
Tue, 11 Sep 2012 21:42:01 +0200
changeset 106929 634a2b9859ab064155c822926b53b004e0d1c72a
parent 106928 eba1d55e5c0c3e0feccf78ba0dc0a1549ed5b4c8
child 106930 8cebabaead164c6c923c7fd0f4b3e138c2c8a8c1
push id74
push usershu@rfrn.org
push dateTue, 18 Sep 2012 19:23:47 +0000
reviewersbholley
bugs720619
milestone18.0a1
Bug 720619 - Attempt a puncture for the [[DefaultValue]] trap; r=bholley
js/src/jswrapper.cpp
js/src/jswrapper.h
js/xpconnect/tests/chrome/Makefile.in
js/xpconnect/tests/chrome/test_bug720619.xul
js/xpconnect/tests/mochitest/Makefile.in
js/xpconnect/tests/mochitest/file_bug720619.html
js/xpconnect/tests/mochitest/test_bug720619.html
--- a/js/src/jswrapper.cpp
+++ b/js/src/jswrapper.cpp
@@ -195,16 +195,53 @@ IndirectWrapper::delete_(JSContext *cx, 
 bool
 IndirectWrapper::enumerate(JSContext *cx, JSObject *wrapper, AutoIdVector &props)
 {
     // if we refuse to perform this action, props remains empty
     static jsid id = JSID_VOID;
     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)
 {
     setHasPrototype(hasPrototype);
 }
 
 DirectWrapper::~DirectWrapper()
 {
@@ -257,16 +294,53 @@ bool
 DirectWrapper::enumerate(JSContext *cx, JSObject *wrapper, AutoIdVector &props)
 {
     JS_ASSERT(!hasPrototype()); // Should never be called when there's a prototype.
     // if we refuse to perform this action, props remains empty
     static jsid id = JSID_VOID;
     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)
 {
     JS_ASSERT(!hasPrototype()); // Should never be called when there's a prototype.
     *bp = false; // default result if we refuse to perform this action
     GET(DirectProxyHandler::has(cx, wrapper, id, bp));
 }
 
@@ -747,21 +821,18 @@ CrossCompartmentWrapper::regexp_toShared
 {
     AutoCompartment call(cx, wrappedObject(wrapper));
     return DirectWrapper::regexp_toShared(cx, wrapper, g);
 }
 
 bool
 CrossCompartmentWrapper::defaultValue(JSContext *cx, JSObject *wrapper, JSType hint, Value *vp)
 {
-    {
-        AutoCompartment call(cx, wrappedObject(wrapper));
-        if (!IndirectProxyHandler::defaultValue(cx, wrapper, hint, vp))
-            return false;
-    }
+    if (!DirectWrapper::defaultValue(cx, wrapper, hint, vp))
+        return false;
     return cx->compartment->wrap(cx, vp);
 }
 
 bool
 CrossCompartmentWrapper::iteratorNext(JSContext *cx, JSObject *wrapper, Value *vp)
 {
     PIERCE(cx, wrapper, GET,
            NOTHING,
--- a/js/src/jswrapper.h
+++ b/js/src/jswrapper.h
@@ -147,16 +147,20 @@ class JS_FRIEND_API(IndirectWrapper) : p
     virtual bool defineProperty(JSContext *cx, JSObject *wrapper, jsid id,
                                 PropertyDescriptor *desc) MOZ_OVERRIDE;
     virtual bool getOwnPropertyNames(JSContext *cx, JSObject *wrapper,
                                      AutoIdVector &props) MOZ_OVERRIDE;
     virtual bool delete_(JSContext *cx, JSObject *wrapper, jsid id,
                          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;
 };
 
 /*
  * DirectWrapper forwards its traps by forwarding them to DirectProxyHandler.
  * In effect, DirectWrapper behaves the same as DirectProxyHandler, except that
  * it adds policy enforcement checks to each trap.
  */
 class JS_FRIEND_API(DirectWrapper) : public Wrapper, public DirectProxyHandler
@@ -202,16 +206,18 @@ class JS_FRIEND_API(DirectWrapper) : pub
     /* Spidermonkey extensions. */
     virtual bool call(JSContext *cx, JSObject *wrapper, unsigned argc, Value *vp) MOZ_OVERRIDE;
     virtual bool construct(JSContext *cx, JSObject *wrapper, unsigned argc, Value *argv, Value *rval) MOZ_OVERRIDE;
     virtual bool nativeCall(JSContext *cx, IsAcceptableThis test, NativeImpl impl,
                             CallArgs args) MOZ_OVERRIDE;
     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;
 
     static void *getWrapperFamily();
 };
 
 /* Base class for all cross compartment wrapper handlers. */
--- a/js/xpconnect/tests/chrome/Makefile.in
+++ b/js/xpconnect/tests/chrome/Makefile.in
@@ -54,16 +54,17 @@ MOCHITEST_CHROME_FILES = \
 		test_getweakmapkeys.xul \
 		test_mozMatchesSelector.xul \
 		test_nodelists.xul \
 		test_precisegc.xul \
 		test_sandboxImport.xul \
 		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
 # wrappers.
 #		test_wrappers-2.xul \
 
 # Disabled due to apparent conservative stack scanner false positives on Linux64 debug.
 #		test_watchpoints.xul \
new file mode 100644
--- /dev/null
+++ b/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>
--- a/js/xpconnect/tests/mochitest/Makefile.in
+++ b/js/xpconnect/tests/mochitest/Makefile.in
@@ -70,16 +70,18 @@ MOCHITEST_FILES =	bug500931_helper.html 
 		file_nodelists.html \
 		file_exnstack.html \
 		file_expandosharing.html \
 		file_empty.html \
 		file_documentdomain.html \
 		test_lookupMethod.html \
 		file_bug738244.html \
 		file_mozMatchesSelector.html \
+		file_bug720619.html \
+		test_bug720619.html \
 		$(NULL)
 
 MOCHITEST_CHROME_FILES	= \
 		test_bug361111.xul \
 		test_bug760131.html \
 		$(NULL)
 
 ifneq ($(OS_TARGET),Android)
new file mode 100644
--- /dev/null
+++ b/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>
new file mode 100644
--- /dev/null
+++ b/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>