src: fix TextDecoder large-input and error paths

ConverterObject::Decode() sized its ICU target buffer as the input
length, or the pending byte count when flushing if that is larger,
times min_char_size(), times 2. min_char_size() is the minimum number
of bytes per character, so multiplying by it inflates the bound
instead of tightening it: for UTF-16 (min_char_size() == 2) a 256 MiB
input requested 2^30 UChars, which fails ucnv_toUnicode()'s internal
targetLimit validation before any input is examined, and the failure
was then reported as ERR_ENCODING_INVALID_ENCODED_DATA. Bound the
buffer by 2 * (input length + pending bytes) / min_char_size instead:
each character consumes at least min_char_size bytes and emits at
most one surrogate pair, and bytes carried over from previous chunks
complete a character in this one. The request is also clamped to
String::kMaxLength + 1 UChars, one extra for a leading BOM that the
success path strips: a result that overflows the clamped buffer
cannot become a V8 string even after the strip, so it is reported as
ERR_STRING_TOO_LONG, the same error StringBytes::Encode() throws for
oversized results. This decodes every input whose result fits in a
V8 string; inputs above ICU's 2 GiB single-call source limit keep
their existing error behaviour.

Also return after a failed StringBytes::Encode() instead of falling
through, so the exception it scheduled (such as ERR_STRING_TOO_LONG
for results beyond the string limit) is no longer masked by
ERR_ENCODING_INVALID_ENCODED_DATA.

The `2 *` factor dates to 98ec909, which restored the effective
capacity that an earlier targetLimit arithmetic bug had provided by
accident. The min_char_size() multiplier itself is older, from
ed21cb1.

Fixes: #47645
Refs: #41026
Refs: #61559
Signed-off-by: Yusufhan Saçak <yusufhansacak@icloud.com>

Author: JosephDoUrden

src/node_i18n.cc

@@ -66,6 +66,7 @@
 #include <unicode/utypes.h>
 #include <unicode/uvernum.h>
 #include <unicode/uversion.h>
+#include <algorithm>
 #include "nbytes.h"
 
 #ifdef NODE_HAVE_SMALL_ICU
@@ -446,18 +447,30 @@ void ConverterObject::Decode(const FunctionCallbackInfo<Value>& args) {
 
   UBool flush = (flags & CONVERTER_FLAGS_FLUSH) == CONVERTER_FLAGS_FLUSH;
 
-  // When flushing the final chunk, the limit is the maximum
-  // of either the input buffer length or the number of pending
-  // characters times the min char size, multiplied by 2 as unicode may
-  // take up to 2 UChars to encode a character
-  size_t limit = 2 * converter->min_char_size() *
-      (!flush ?
-          input.length() :
-          std::max(
-              input.length(),
-              static_cast<size_t>(
-                  ucnv_toUCountPending(converter->conv(), &status))));
+  // The result has to be materialised as a V8 string, which holds at most
+  // String::kMaxLength UChars; StringBytes::Encode() rejects anything
+  // longer. One extra UChar leaves room for a leading BOM, which the
+  // success path below strips before the string is created.
+  constexpr size_t kMaxTargetUChars = String::kMaxLength + 1;
+
+  // Each character consumes at least min_char_size() bytes and produces
+  // at most 2 UChars (a surrogate pair). ICU's data format allows longer
+  // per-character outputs, but no converter reachable through
+  // TextDecoder ships one: lib/internal/encoding.js handles UTF-8 and
+  // the single-byte encodings in JS, and the UTF-16 and CJK multibyte
+  // converters that reach this path all emit at most one UChar per input
+  // byte. Were that ever to change, ucnv_toUnicode() reports
+  // U_BUFFER_OVERFLOW_ERROR rather than overrunning the target. Count
+  // the bytes the converter is still holding from previous chunks too:
+  // they belong to a character whose remaining bytes may arrive in this
+  // chunk. Clamping loses nothing: a result that does not fit the
+  // clamped buffer cannot become a string either way.
+  int32_t pending = ucnv_toUCountPending(converter->conv(), &status);
   status = U_ZERO_ERROR;
+  size_t limit = std::min(
+      2 * (input.length() + (pending > 0 ? static_cast<size_t>(pending) : 0)) /
+          converter->min_char_size(),
+      kMaxTargetUChars);
 
   if (limit > 0)
     result.AllocateSufficientStorage(limit);
@@ -519,8 +532,19 @@ void ConverterObject::Decode(const FunctionCallbackInfo<Value>& args) {
     if (StringBytes::Encode(env->isolate(), value, length, UCS2)
             .ToLocal(&ret)) {
       args.GetReturnValue().Set(ret);
-      return;
     }
+    // If Encode() failed, it has already scheduled an exception; do not
+    // replace it with ERR_ENCODING_INVALID_ENCODED_DATA below.
+    return;
+  }
+
+  if (status == U_BUFFER_OVERFLOW_ERROR) {
+    // The result did not fit the clamped target buffer, so it cannot fit
+    // a V8 string even after a leading BOM is stripped. Surface the same
+    // error Encode() throws for oversized results instead of mislabelling
+    // the input as invalid.
+    env->isolate()->ThrowException(ERR_STRING_TOO_LONG(env->isolate()));
+    return;
   }
 
   node::THROW_ERR_ENCODING_INVALID_ENCODED_DATA(

test/pummel/test-whatwg-encoding-custom-textdecoder-large.js

@@ -0,0 +1,68 @@
+'use strict';
+const common = require('../common');
+
+// Input large enough that the old 4x target bound exceeded ICU's
+// 0x3fffffff UChar limit; also needs more than a 32-bit heap.
+common.skipIf32Bits();
+
+if (!common.hasIntl)
+  common.skip('missing Intl');
+
+// Peak RSS is around 1.6 GiB: the input, the ICU target buffer, and two
+// result strings.
+if (require('os').totalmem() < 8 * 2 ** 30)
+  common.skip('less than 8 GiB of total memory');
+
+const assert = require('assert');
+
+const size = 2 ** 27;
+
+let input;
+
+try {
+  input = Buffer.allocUnsafe(size * 2);
+} catch (e) {
+  if (
+    e.code === 'ERR_MEMORY_ALLOCATION_FAILED' ||
+    /Array buffer allocation failed/.test(e.message)
+  ) {
+    common.skip('insufficient space for Buffer.allocUnsafe');
+  }
+
+  throw e;
+}
+
+// Non-uniform repeating pattern of A, a U+1F600 surrogate pair and 中,
+// written as explicit little-endian bytes so the input is identical on
+// big-endian hosts. Corrupted or misplaced output cannot match it.
+input.fill(Buffer.from([0x41, 0x00, 0x3D, 0xD8, 0x00, 0xDE, 0x2D, 0x4E]));
+
+const decoder = new TextDecoder('utf-16le');
+
+// 2 ** 27 UTF-16 code units used to fail with
+// ERR_ENCODING_INVALID_ENCODED_DATA because the target buffer request
+// exceeded ICU's internal targetLimit validation.
+// Refs: https://github.com/nodejs/node/issues/47645
+const result = decoder.decode(input);
+assert.strictEqual(result.length, size);
+assert.strictEqual(result[0], 'A');
+assert.strictEqual(result[1], '\uD83D');
+assert.strictEqual(result[2], '\uDE00');
+assert.strictEqual(result[size / 2], 'A');
+assert.strictEqual(result[size - 1], '中');
+
+// Guard against over-correction: one code unit below the failure boundary
+// decodes at HEAD too and must keep doing so. The truncation removes the
+// trailing 中, so it does not split a surrogate pair.
+assert.strictEqual(decoder.decode(input.subarray(0, size * 2 - 2)).length,
+                   size - 1);
+
+// Streaming with an odd byte split lands mid-code-unit, so one byte stays
+// pending in the converter across the chunk boundary. The full content is
+// compared against the non-streaming result, so any corruption at the
+// boundary fails the test.
+const split = 2 ** 26 + 1;
+const streamed = decoder.decode(input.subarray(0, split), { stream: true }) +
+                 decoder.decode(input.subarray(split));
+assert.strictEqual(streamed.length, result.length);
+assert.strictEqual(streamed, result);

test/pummel/test-whatwg-encoding-custom-textdecoder-toolong.js

@@ -0,0 +1,89 @@
+'use strict';
+const common = require('../common');
+
+// The working set peaks around 4 GiB, far beyond a 32-bit heap.
+common.skipIf32Bits();
+
+if (!common.hasIntl)
+  common.skip('missing Intl');
+
+// Peak RSS is around 4 GiB in the BOM case: a 1 GiB input, an ICU target
+// buffer of String::kMaxLength + 1 UChars (about 1 GiB), and the decoded
+// string with its transient copy.
+if (require('os').totalmem() < 8 * 2 ** 30)
+  common.skip('less than 8 GiB of total memory');
+
+const assert = require('assert');
+const kStringMaxLength = require('buffer').constants.MAX_STRING_LENGTH;
+
+function allocOrSkip(bytes) {
+  try {
+    return Buffer.allocUnsafe(bytes);
+  } catch (e) {
+    if (
+      e.code === 'ERR_MEMORY_ALLOCATION_FAILED' ||
+      /Array buffer allocation failed/.test(e.message)
+    ) {
+      common.skip('insufficient space for Buffer.allocUnsafe');
+    }
+
+    throw e;
+  }
+}
+
+function assertThrowsTooLong(fn) {
+  assert.throws(fn, (e) => {
+    assert.strictEqual(e.code, 'ERR_STRING_TOO_LONG');
+    return true;
+  });
+}
+
+{
+  // One UTF-16 code unit beyond the maximum string length. The target
+  // buffer is clamped to kStringMaxLength + 1 UChars, so the result fits
+  // the buffer exactly, there is no leading BOM to strip, and
+  // StringBytes::Encode() rejects the oversized result. That must surface
+  // as ERR_STRING_TOO_LONG rather than ERR_ENCODING_INVALID_ENCODED_DATA.
+  const size = 2 * kStringMaxLength + 2;
+  const input = allocOrSkip(size);
+  input.fill(0x20);
+  assertThrowsTooLong(() => new TextDecoder('utf-16le').decode(input));
+}
+
+{
+  // A BOM plus exactly kStringMaxLength characters: the decode fills the
+  // clamped buffer exactly, the BOM is stripped, and the result is the
+  // longest possible string. This must succeed; it is why the clamp is
+  // kStringMaxLength + 1 and not kStringMaxLength.
+  const size = 2 * kStringMaxLength + 2;
+  const input = allocOrSkip(size);
+  input.fill(0x20);
+  input[0] = 0xFF;
+  input[1] = 0xFE;
+  const result = new TextDecoder('utf-16le').decode(input);
+  assert.strictEqual(result.length, kStringMaxLength);
+  assert.strictEqual(result.charCodeAt(0), 0x2020);
+}
+
+{
+  // Two characters beyond the buffer through a min_char_size() == 1
+  // encoding: pure-ASCII input of kStringMaxLength + 2 bytes wants
+  // kStringMaxLength + 2 UChars, overflows the clamped target buffer
+  // inside ICU, and the U_BUFFER_OVERFLOW_ERROR path must report
+  // ERR_STRING_TOO_LONG rather than ERR_ENCODING_INVALID_ENCODED_DATA.
+  // gb18030 needs full-icu; skip the case silently on small-icu builds
+  // (the utf-16le cases above ran).
+  let decoder;
+  try {
+    decoder = new TextDecoder('gb18030');
+  } catch (e) {
+    if (e.code !== 'ERR_ENCODING_NOT_SUPPORTED')
+      throw e;
+  }
+
+  if (decoder !== undefined) {
+    const input = allocOrSkip(kStringMaxLength + 2);
+    input.fill(0x41);
+    assertThrowsTooLong(() => decoder.decode(input));
+  }
+}