Skip to content

Commit

Permalink
src: don't print garbage errors
Browse files Browse the repository at this point in the history
If JS throws an object whose toString() method throws, then Node
attempts to print an empty message, but actually prints garbage.
This commit checks for this case, and prints a message instead.

Fixes: #4079
PR-URL: #4112
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
  • Loading branch information
cjihrig authored and rvagg committed Dec 8, 2015
1 parent 1d50819 commit da3137d
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 5 deletions.
6 changes: 4 additions & 2 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1511,8 +1511,10 @@ static void ReportException(Environment* env,
name.IsEmpty() ||
name->IsUndefined()) {
// Not an error object. Just print as-is.
node::Utf8Value message(env->isolate(), er);
PrintErrorString("%s\n", *message);
String::Utf8Value message(er);

PrintErrorString("%s\n", *message ? *message :
"<toString() threw exception>");
} else {
node::Utf8Value name_string(env->isolate(), name);
node::Utf8Value message_string(env->isolate(), message);
Expand Down
5 changes: 5 additions & 0 deletions test/fixtures/throws_error7.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
throw {
toString: function() {
throw this;
}
};
9 changes: 6 additions & 3 deletions test/parallel/test-error-reporting.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ function errExec(script, callback) {

// Count the tests
exits++;

console.log('.');
});
}

Expand Down Expand Up @@ -64,6 +62,11 @@ errExec('throws_error6.js', function(err, stdout, stderr) {
assert.ok(/SyntaxError/.test(stderr));
});

// Object that throws in toString() doesn't print garbage
errExec('throws_error7.js', function(err, stdout, stderr) {
assert.ok(/<toString\(\) threw exception/.test(stderr));
});

process.on('exit', function() {
assert.equal(6, exits);
assert.equal(7, exits);
});

0 comments on commit da3137d

Please sign in to comment.