src: don't print garbage errors

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]>
nodejs · Dec 5, 2015 · 1ec09b0 · 1ec09b0
1 parent e6e7891
commit 1ec09b0
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 5 deletions.
diff --git a/src/node.cc b/src/node.cc
@@ -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);

diff --git a/test/fixtures/throws_error7.js b/test/fixtures/throws_error7.js
@@ -0,0 +1,5 @@
+throw {
+  toString: function() {
+    throw this;
+  }
+};
diff --git a/test/parallel/test-error-reporting.js b/test/parallel/test-error-reporting.js
@@ -24,8 +24,6 @@ function errExec(script, callback) {
 
     // Count the tests
     exits++;
-
-    console.log('.');
   });
 }
 
@@ -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);
 });