string_decoder: throw ERR_STRING_TOO_LONG for UTF-8

String::NewFromUtf8 doesn't generate an exception in V8 when the string is too long but is guaranteed to return an empty MaybeLocal only in that case. Generate a Node.js exception when it happens. Fixes: #35676 PR-URL: #36661 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Jun 11, 2021 · 1dc4dea · 1dc4dea
1 parent 3215a58
commit 1dc4dea
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 1 deletion.
diff --git a/src/string_decoder.cc b/src/string_decoder.cc
@@ -3,6 +3,7 @@
 
 #include "env-inl.h"
 #include "node_buffer.h"
+#include "node_errors.h"
 #include "string_bytes.h"
 #include "util.h"
 
@@ -29,11 +30,17 @@ MaybeLocal<String> MakeString(Isolate* isolate,
   Local<Value> error;
   MaybeLocal<Value> ret;
   if (encoding == UTF8) {
-    return String::NewFromUtf8(
+    MaybeLocal<String> utf8_string = String::NewFromUtf8(
         isolate,
         data,
         v8::NewStringType::kNormal,
         length);
+    if (utf8_string.IsEmpty()) {
+      isolate->ThrowException(node::ERR_STRING_TOO_LONG(isolate));
+      return MaybeLocal<String>();
+    } else {
+      return utf8_string;
+    }
   } else {
     ret = StringBytes::Encode(
         isolate,

diff --git a/test/parallel/test-string-decoder.js b/test/parallel/test-string-decoder.js
@@ -201,6 +201,13 @@ assert.throws(
   }
 );
 
+assert.throws(
+  () => new StringDecoder().write(Buffer.alloc(0x1fffffe8 + 1).fill('a')),
+  {
+    code: 'ERR_STRING_TOO_LONG',
+  }
+);
+
 // Test verifies that StringDecoder will correctly decode the given input
 // buffer with the given encoding to the expected output. It will attempt all
 // possible ways to write() the input buffer, see writeSequences(). The