From 953b4f5811ffc09eb4255c9261a236cdd7274d08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yusufhan=20Sa=C3=A7ak?= Date: Sat, 29 Aug 2026 07:27:11 +0300 Subject: [PATCH] src: fix TextDecoder large-input and error paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 ucnv_toUnicode()'s target-range validation limit of 0x3fffffff UChars, which loses nothing since larger results cannot fit in a V8 string anyway. This decodes every input whose result fits in a V8 string. 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 98ec909f2bf2, which restored the effective capacity that an earlier targetLimit arithmetic bug had provided by accident. The min_char_size() multiplier itself is older, from ed21cb1774d3. Fixes: https://github.com/nodejs/node/issues/47645 Refs: https://github.com/nodejs/node/pull/41026 Refs: https://github.com/nodejs/node/pull/61559 Signed-off-by: Yusufhan Saçak --- src/node_i18n.cc | 31 +++++--- ...hatwg-encoding-custom-textdecoder-large.js | 68 ++++++++++++++++ ...twg-encoding-custom-textdecoder-toolong.js | 79 +++++++++++++++++++ 3 files changed, 166 insertions(+), 12 deletions(-) create mode 100644 test/pummel/test-whatwg-encoding-custom-textdecoder-large.js create mode 100644 test/pummel/test-whatwg-encoding-custom-textdecoder-toolong.js diff --git a/src/node_i18n.cc b/src/node_i18n.cc index e743941734a7..fbadf3141fb2 100644 --- a/src/node_i18n.cc +++ b/src/node_i18n.cc @@ -66,6 +66,7 @@ #include #include #include +#include #include "nbytes.h" #ifdef NODE_HAVE_SMALL_ICU @@ -446,18 +447,22 @@ void ConverterObject::Decode(const FunctionCallbackInfo& 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( - ucnv_toUCountPending(converter->conv(), &status)))); + // ucnv_toUnicode() rejects target ranges larger than this in its + // argument validation. See deps/icu-small/source/common/ucnv.cpp. + constexpr size_t kMaxTargetUChars = 0x3fffffff; + + // Each character consumes at least min_char_size() bytes and produces at + // most 2 UChars (a surrogate pair). 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 to the ICU + // cap loses nothing: any result that can become a V8 string needs at + // most String::kMaxLength UChars, well under the cap. + int32_t pending = ucnv_toUCountPending(converter->conv(), &status); status = U_ZERO_ERROR; + size_t limit = std::min( + 2 * (input.length() + (pending > 0 ? static_cast(pending) : 0)) / + converter->min_char_size(), + kMaxTargetUChars); if (limit > 0) result.AllocateSufficientStorage(limit); @@ -519,8 +524,10 @@ void ConverterObject::Decode(const FunctionCallbackInfo& 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; } node::THROW_ERR_ENCODING_INVALID_ENCODED_DATA( diff --git a/test/pummel/test-whatwg-encoding-custom-textdecoder-large.js b/test/pummel/test-whatwg-encoding-custom-textdecoder-large.js new file mode 100644 index 000000000000..92d5c37e1555 --- /dev/null +++ b/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); diff --git a/test/pummel/test-whatwg-encoding-custom-textdecoder-toolong.js b/test/pummel/test-whatwg-encoding-custom-textdecoder-toolong.js new file mode 100644 index 000000000000..350f37fc0fa4 --- /dev/null +++ b/test/pummel/test-whatwg-encoding-custom-textdecoder-toolong.js @@ -0,0 +1,79 @@ +'use strict'; +const common = require('../common'); + +// The working set is around 3 GiB, far beyond a 32-bit heap. +common.skipIf32Bits(); + +if (!common.hasIntl) + common.skip('missing Intl'); + +// Peak RSS is around 3 GiB: a 1 GiB input, a 2 GiB ICU target buffer, and +// a transient 1 GiB StringBytes 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) => { + // Constrained machines can fail the 1 GiB copy StringBytes makes while + // building the string, before the length limit is reached. + if (e.code === 'ERR_MEMORY_ALLOCATION_FAILED') + common.skip('insufficient memory for the StringBytes copy'); + + assert.strictEqual(e.code, 'ERR_STRING_TOO_LONG'); + return true; + }); +} + +{ + // One UTF-16 code unit beyond the maximum string length: the decode + // completes inside ICU but the resulting kStringMaxLength + 1 characters + // cannot be materialised as a string, which must surface as + // ERR_STRING_TOO_LONG rather than ERR_ENCODING_INVALID_ENCODED_DATA. + // The ICU target buffer request is 2 * (size / 2) = size UChars, which + // must stay <= 0x3fffffff (ICU's targetLimit cap) for the conversion to + // run at all; size = 2 * kStringMaxLength + 2 = 1073741778 satisfies + // that. + const size = 2 * kStringMaxLength + 2; + const input = allocOrSkip(size); + input.fill(0x20); + assertThrowsTooLong(() => new TextDecoder('utf-16le').decode(input)); +} + +{ + // Same limit through a min_char_size() == 1 encoding: pure-ASCII input + // of kStringMaxLength + 1 bytes decodes to kStringMaxLength + 1 + // characters. The 2x target bound is clamped to ICU's cap, so only the + // output length decides the outcome. gb18030 needs full-icu; skip the + // case silently on small-icu builds (the utf-16le case 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 + 1); + input.fill(0x41); + assertThrowsTooLong(() => decoder.decode(input)); + } +}