Reviewers: Søren Gjesse,
Description:
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.
Please review this at http://codereview.chromium.org/6272004/
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/messages.js
M test/cctest/test-api.cc
M test/mjsunit/error-constructors.js
Index: src/messages.js
===================================================================
--- src/messages.js (revision 6298)
+++ src/messages.js (working copy)
@@ -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
Index: test/cctest/test-api.cc
===================================================================
--- test/cctest/test-api.cc (revision 6298)
+++ test/cctest/test-api.cc (working copy)
@@ -2288,6 +2288,30 @@
}
+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();
Index: test/mjsunit/error-constructors.js
===================================================================
--- test/mjsunit/error-constructors.js (revision 6298)
+++ test/mjsunit/error-constructors.js (working copy)
@@ -48,4 +48,13 @@
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;
+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
