Skip to content

Commit

Permalink
string_decoder: throw ERR_STRING_TOO_LONG for UTF-8
Browse files Browse the repository at this point in the history
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 <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
targos committed Jun 11, 2021
1 parent 3215a58 commit 1dc4dea
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
9 changes: 8 additions & 1 deletion src/string_decoder.cc
Expand Up @@ -3,6 +3,7 @@

#include "env-inl.h"
#include "node_buffer.h"
#include "node_errors.h"
#include "string_bytes.h"
#include "util.h"

Expand All @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions test/parallel/test-string-decoder.js
Expand Up @@ -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
Expand Down

0 comments on commit 1dc4dea

Please sign in to comment.