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 <[email protected]>
Reviewed-By: Chris Dickinson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
  • Loading branch information
cjihrig authored and Myles Borins committed Jan 19, 2016
1 parent c9100d7 commit c96eca1
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 @@ -1548,8 +1548,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 c96eca1

Please sign in to comment.