Make the 'name' property on error prototypes read-only and dont-delete

to avoid leaking of error objects to accessor methods when formatting
error messages internally.

Also, do not call overwritten toString methods on error objects when
formatting messages internally.

Review URL: http://codereview.chromium.org/6272004

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

src/messages.js

@@ -97,6 +97,12 @@ function ToDetailString(obj) {
     var constructorName = constructor.name;
     if (!constructorName) return ToString(obj);
     return "#<" + GetInstanceName(constructorName) + ">";
+  } else if (obj instanceof $Error) {
+    // When formatting internally created error messages, do not
+    // invoke overwritten error toString methods but explicitly use
+    // the error to string method. This is to avoid leaking error
+    // objects between script tags in a browser setting.
+    return %_CallFunction(obj, errorToString);
   } else {
     return ToString(obj);
   }
@@ -943,7 +949,12 @@ function DefineError(f) {
   }
   %FunctionSetInstanceClassName(f, 'Error');
   %SetProperty(f.prototype, 'constructor', f, DONT_ENUM);
-  f.prototype.name = name;
+  // 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, READ_ONLY | DONT_DELETE);
   %SetCode(f, function(m) {
     if (%_IsConstructCall()) {
       // Define all the expected properties directly on the error
@@ -995,14 +1006,15 @@ $Error.captureStackTrace = captureStackTrace;
 // Setup extra properties of the Error.prototype object.
 $Error.prototype.message = '';
 
-%SetProperty($Error.prototype, 'toString', function toString() {
+function errorToString() {
   var type = this.type;
   if (type && !this.hasOwnProperty("message")) {
     return this.name + ": " + FormatMessage({ type: type, args: this.arguments });
   }
   var message = this.hasOwnProperty("message") ? (": " + this.message) : "";
   return this.name + message;
-}, DONT_ENUM);
+}
+%SetProperty($Error.prototype, 'toString', errorToString, DONT_ENUM);
 
 
 // Boilerplate for exceptions for stack overflows. Used from

test/cctest/test-api.cc

@@ -2357,6 +2357,30 @@ TEST(TryCatchInTryFinally) {
 }
 
 
+static void check_reference_error_message(
+    v8::Handle<v8::Message> message,
+    v8::Handle<v8::Value> data) {
+  const char* reference_error = "Uncaught ReferenceError: asdf is not defined";
+  CHECK(message->Get()->Equals(v8_str(reference_error)));
+}
+
+
+// Test that overwritten toString methods are not invoked on uncaught
+// exception formatting. However, they are invoked when performing
+// normal error string conversions.
+THREADED_TEST(APIThrowMessageOverwrittenToString) {
+  v8::HandleScope scope;
+  v8::V8::AddMessageListener(check_reference_error_message);
+  LocalContext context;
+  CompileRun("ReferenceError.prototype.toString ="
+             "  function() { return 'Whoops' }");
+  CompileRun("asdf;");
+  v8::Handle<Value> string = CompileRun("try { asdf; } catch(e) { e + ''; }");
+  CHECK(string->Equals(v8_str("Whoops")));
+  v8::V8::RemoveMessageListeners(check_message);
+}
+
+
 static void receive_message(v8::Handle<v8::Message> message,
                             v8::Handle<v8::Value> data) {
   message->Get();

test/mjsunit/error-constructors.js

@@ -48,4 +48,14 @@ 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);