Revision: 6322
Author: [email protected]
Date: Fri Jan 14 04:51:04 2011
Log: 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
http://code.google.com/p/v8/source/detail?r=6322
Modified:
/branches/bleeding_edge/src/messages.js
/branches/bleeding_edge/test/cctest/test-api.cc
/branches/bleeding_edge/test/mjsunit/error-constructors.js
=======================================
--- /branches/bleeding_edge/src/messages.js Wed Jan 12 22:56:54 2011
+++ /branches/bleeding_edge/src/messages.js Fri Jan 14 04:51:04 2011
@@ -97,6 +97,12 @@
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 @@
}
%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 @@
// 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
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Thu Jan 13 07:56:33 2011
+++ /branches/bleeding_edge/test/cctest/test-api.cc Fri Jan 14 04:51:04 2011
@@ -2355,6 +2355,30 @@
"}");
CHECK(result->IsTrue());
}
+
+
+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,
=======================================
--- /branches/bleeding_edge/test/mjsunit/error-constructors.js Wed Jan 12
22:56:54 2011
+++ /branches/bleeding_edge/test/mjsunit/error-constructors.js Fri Jan 14
04:51:04 2011
@@ -48,4 +48,14 @@
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);
+
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
