Make Error.prototype.name writable again, as required by the spec and the web.

Address http://code.google.com/p/chromium/issues/detail?id=69187 by instead
ignoring getters on ReferenceError.prototype.name in Error.prototype.toString.

And while we're at it, do the same for SyntaxError and TypeError, and the
properties "message", "type", and "arguments" on all of them, which
potentially have similar issues.

R=danno@chromium.org
BUG=69187
TEST=

Review URL: https://chromiumcodereview.appspot.com/10234004

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@11529 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/messages.js b/src/messages.js
index a3adcf8..f8b5766 100644 (file)
--- a/src/messages.js
+++ b/src/messages.js
@@ -1125,13 +1125,7 @@ function SetUpError() {
     }
     %FunctionSetInstanceClassName(f, 'Error');
     %SetProperty(f.prototype, 'constructor', f, DONT_ENUM);
-    // The name property on the prototype of error objects is not
-    // specified as being read-one and dont-delete. However, allowing
-    // overwriting allows leaks of error objects between script blocks
-    // in the same context in a browser setting. Therefore we fix the
-    // name.
-    %SetProperty(f.prototype, "name", name,
-                 DONT_ENUM | DONT_DELETE | READ_ONLY)  ;
+    %SetProperty(f.prototype, "name", name, DONT_ENUM);
     %SetCode(f, function(m) {
       if (%_IsConstructCall()) {
         // Define all the expected properties directly on the error
@@ -1147,10 +1141,8 @@ function SetUpError() {
               return FormatMessage(%NewMessageObject(obj.type, obj.arguments));
           });
         } else if (!IS_UNDEFINED(m)) {
-          %IgnoreAttributesAndSetProperty(this,
-                                          'message',
-                                          ToString(m),
-                                          DONT_ENUM);
+          %IgnoreAttributesAndSetProperty(
+            this, 'message', ToString(m), DONT_ENUM);
         }
         captureStackTrace(this, f);
       } else {
@@ -1180,16 +1172,41 @@ $Error.captureStackTrace = captureStackTrace;
 var visited_errors = new InternalArray();
 var cyclic_error_marker = new $Object();
 
+function GetPropertyWithoutInvokingMonkeyGetters(error, name) {
+  // Climb the prototype chain until we find the holder.
+  while (error && !%HasLocalProperty(error, name)) {
+    error = error.__proto__;
+  }
+  if (error === null) return void 0;
+  if (!IS_OBJECT(error)) return error[name];
+  // If the property is an accessor on one of the predefined errors that can be
+  // generated statically by the compiler, don't touch it. This is to address
+  // http://code.google.com/p/chromium/issues/detail?id=69187
+  var desc = %GetOwnProperty(error, name);
+  if (desc && desc[IS_ACCESSOR_INDEX]) {
+    var isName = name === "name";
+    if (error === $ReferenceError.prototype)
+      return isName ? "ReferenceError" : void 0;
+    if (error === $SyntaxError.prototype)
+      return isName ? "SyntaxError" : void 0;
+    if (error === $TypeError.prototype)
+      return isName ? "TypeError" : void 0;
+  }
+  // Otherwise, read normally.
+  return error[name];
+}
+
 function ErrorToStringDetectCycle(error) {
   if (!%PushIfAbsent(visited_errors, error)) throw cyclic_error_marker;
   try {
-    var type = error.type;
-    var name = error.name;
+    var type = GetPropertyWithoutInvokingMonkeyGetters(error, "type");
+    var name = GetPropertyWithoutInvokingMonkeyGetters(error, "name");
     name = IS_UNDEFINED(name) ? "Error" : TO_STRING_INLINE(name);
-    var message = error.message;
+    var message = GetPropertyWithoutInvokingMonkeyGetters(error, "message");
     var hasMessage = %_CallFunction(error, "message", ObjectHasOwnProperty);
     if (type && !hasMessage) {
-      message = FormatMessage(%NewMessageObject(type, error.arguments));
+      var args = GetPropertyWithoutInvokingMonkeyGetters(error, "arguments");
+      message = FormatMessage(%NewMessageObject(type, args));
     }
     message = IS_UNDEFINED(message) ? "" : TO_STRING_INLINE(message);
     if (name === "") return message;

diff --git a/test/mjsunit/error-constructors.js b/test/mjsunit/error-constructors.js
index 966a162..107164d 100644 (file)
--- a/test/mjsunit/error-constructors.js
+++ b/test/mjsunit/error-constructors.js
@@ -25,39 +25,7 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-var e = new Error();
-assertFalse(e.hasOwnProperty('message'));
-Error.prototype.toString = Object.prototype.toString;
-assertEquals("[object Error]", Error.prototype.toString());
-assertEquals(Object.prototype, Error.prototype.__proto__);
-
-// Check that error construction does not call setters for the
-// properties on error objects in prototypes.
-function fail() { assertTrue(false); };
-ReferenceError.prototype.__defineSetter__('stack', fail);
-ReferenceError.prototype.__defineSetter__('message', fail);
-ReferenceError.prototype.__defineSetter__('type', fail);
-ReferenceError.prototype.__defineSetter__('arguments', fail);
-var e0 = new ReferenceError();
-var e1 = new ReferenceError('123');
-assertTrue(e1.hasOwnProperty('message'));
-assertTrue(e0.hasOwnProperty('stack'));
-assertTrue(e1.hasOwnProperty('stack'));
-assertTrue(e0.hasOwnProperty('type'));
-assertTrue(e1.hasOwnProperty('type'));
-assertTrue(e0.hasOwnProperty('arguments'));
-assertTrue(e1.hasOwnProperty('arguments'));
-
-// Check that the name property on error prototypes is read-only and
-// dont-delete. This is not specified, but allowing overwriting the
-// name property with a getter can leaks error objects from different
-// script tags in the same context in a browser setting. We therefore
-// disallow changes to the name property on error objects.
-assertEquals("ReferenceError", ReferenceError.prototype.name);
-delete ReferenceError.prototype.name;
-assertEquals("ReferenceError", ReferenceError.prototype.name);
-ReferenceError.prototype.name = "not a reference error";
-assertEquals("ReferenceError", ReferenceError.prototype.name);
+// Flags: --allow-natives-syntax
 
 // Check that message and name are not enumerable on Error objects.
 var desc = Object.getOwnPropertyDescriptor(Error.prototype, 'name');
@@ -75,8 +43,75 @@ assertFalse(desc['enumerable']);
 desc = Object.getOwnPropertyDescriptor(e, 'stack');
 assertFalse(desc['enumerable']);
 
+var e = new Error();
+assertFalse(e.hasOwnProperty('message'));
+
 // name is not tested above, but in addition we should have no enumerable
 // properties, so we simply assert that.
 for (var v in e) {
   assertUnreachable();
 }
+
+// Check that error construction does not call setters for the
+// properties on error objects in prototypes.
+function fail() { assertUnreachable(); };
+ReferenceError.prototype.__defineSetter__('name', fail);
+ReferenceError.prototype.__defineSetter__('message', fail);
+ReferenceError.prototype.__defineSetter__('type', fail);
+ReferenceError.prototype.__defineSetter__('arguments', fail);
+ReferenceError.prototype.__defineSetter__('stack', fail);
+
+var e = new ReferenceError();
+assertTrue(e.hasOwnProperty('stack'));
+assertTrue(e.hasOwnProperty('type'));
+assertTrue(e.hasOwnProperty('arguments'));
+
+var e = new ReferenceError('123');
+assertTrue(e.hasOwnProperty('message'));
+assertTrue(e.hasOwnProperty('stack'));
+assertTrue(e.hasOwnProperty('type'));
+assertTrue(e.hasOwnProperty('arguments'));
+
+var e = %MakeReferenceError("my_test_error", [0, 1]);
+assertTrue(e.hasOwnProperty('stack'));
+assertTrue(e.hasOwnProperty('type'));
+assertTrue(e.hasOwnProperty('arguments'));
+assertEquals("my_test_error", e.type)
+
+// Check that intercepting property access from toString is prevented for
+// compiler errors. This is not specified, but allowing interception
+// through a getter can leak error objects from different
+// script tags in the same context in a browser setting.
+var errors = [SyntaxError, ReferenceError, TypeError];
+for (var i in errors) {
+  var name = errors[i].prototype.toString();
+  // Monkey-patch prototype.
+  var props = ["name", "message", "type", "arguments", "stack"];
+  for (var j in props) {
+    errors[i].prototype.__defineGetter__(props[j], fail);
+  }
+  // String conversion should not invoke monkey-patched getters on prototype.
+  var e = new errors[i];
+  assertEquals(name, e.toString());
+  // Custom getters in actual objects are welcome.
+  e.__defineGetter__("name", function() { return "mine"; });
+  assertEquals("mine", e.toString());
+}
+
+// Monkey-patching non-static errors should still be observable.
+function MyError() {}
+MyError.prototype = new Error;
+var errors = [Error, RangeError, EvalError, URIError, MyError];
+for (var i in errors) {
+  errors[i].prototype.__defineGetter__("name", function() { return "my"; });
+  errors[i].prototype.__defineGetter__("message", function() { return "moo"; });
+  var e = new errors[i];
+  assertEquals("my: moo", e.toString());
+}
+
+
+Error.prototype.toString = Object.prototype.toString;
+assertEquals("[object Error]", Error.prototype.toString());
+assertEquals(Object.prototype, Error.prototype.__proto__);
+var e = new Error("foo");
+assertEquals("[object Error]", e.toString());