Skip to content

Commit de82d63

Browse files
committed
src: fix two encodeInto() bugs that reject input that fits
TextEncoder.encodeInto() could report that a code point does not fit in the destination Uint8Array even when it does. 1. simpleUtfEncodingLength() used 0x400 as the boundary between 2-byte and 3-byte UTF-8 encodings, but the correct boundary is 0x800: code points in [0x80, 0x800) need 2 bytes in UTF-8, and only code points >= 0x800 need 3. 2. The same function is called with a raw `char` from the Latin1 (one-byte string) code path. `char` is signed on some platforms, so a byte >= 0x80 gets sign-extended to a large uint16_t value instead of the intended code point, which also made encodeInto() behave differently for the same prefix depending on whether the rest of the source string forced V8 to represent it as one-byte (Latin1) or two-byte (UTF-16) internally. Also make the character-by-character fallback in findBestFit() consume a UTF-16 surrogate pair as one atomic 4-byte unit instead of scoring each half separately and patching up the boundary afterwards, which is more straightforward to reason about than the previous post-hoc adjustment. Fixes: #65994 Signed-off-by: agape1225 <49804691+agape1225@users.noreply.github.com>
1 parent 7203d9b commit de82d63

2 files changed

Lines changed: 76 additions & 12 deletions

File tree

src/encoding_binding.cc

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
#include <algorithm>
1212
#include <cstdint>
13+
#include <type_traits>
1314

1415
namespace node {
1516
namespace encoding_binding {
@@ -88,7 +89,7 @@ constexpr bool isSurrogatePair(uint16_t lead, uint16_t trail) {
8889

8990
constexpr size_t simpleUtfEncodingLength(uint16_t c) {
9091
if (c < 0x80) return 1;
91-
if (c < 0x400) return 2;
92+
if (c < 0x800) return 2;
9293
return 3;
9394
}
9495

@@ -162,19 +163,30 @@ size_t findBestFit(const Char* data, size_t length, size_t bufferSize) {
162163
}
163164

164165
while (pos < length && utf8Accumulated < bufferSize) {
165-
size_t extra = simpleUtfEncodingLength(data[pos]);
166-
if (utf8Accumulated + extra > bufferSize) break;
167-
pos++;
168-
utf8Accumulated += extra;
169-
}
170-
171-
if (UTF16 && pos != 0 && pos != length &&
172-
isSurrogatePair(data[pos - 1], data[pos])) {
173-
if (utf8Accumulated < bufferSize) {
174-
pos++;
166+
size_t codeUnits = 1;
167+
size_t extra;
168+
if constexpr (UTF16) {
169+
// A valid surrogate pair must be consumed together: it encodes to 4
170+
// UTF-8 bytes total, not 3 for each half measured separately (which
171+
// is also what an isolated, unpaired surrogate encodes to).
172+
if (pos + 1 < length && isSurrogatePair(data[pos], data[pos + 1])) {
173+
codeUnits = 2;
174+
extra = 4;
175+
} else {
176+
extra = simpleUtfEncodingLength(static_cast<uint16_t>(data[pos]));
177+
}
175178
} else {
176-
pos--;
179+
// `char` is signed on some platforms/ABIs, so widening a byte >= 0x80
180+
// straight to uint16_t would sign-extend it into a bogus code point.
181+
// Go through the Char type's unsigned counterpart first to get the
182+
// right code unit.
183+
using UnsignedChar = std::make_unsigned_t<Char>;
184+
extra = simpleUtfEncodingLength(
185+
static_cast<uint16_t>(static_cast<UnsignedChar>(data[pos])));
177186
}
187+
if (utf8Accumulated + extra > bufferSize) break;
188+
pos += codeUnits;
189+
utf8Accumulated += extra;
178190
}
179191
return pos;
180192
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
'use strict';
2+
3+
// This tests that TextEncoder.encodeInto() does not underestimate how many
4+
// bytes a code point needs when computing how much of the source string fits
5+
// into the destination.
6+
7+
require('../common');
8+
const assert = require('assert');
9+
10+
// Long enough to bypass the small-string fast path (kSmallStringThreshold = 32
11+
// in src/encoding_binding.cc) and exercise the chunked encoding logic.
12+
const encoder = new TextEncoder();
13+
14+
{
15+
// Code points in [0x80, 0x800) are 2 bytes in UTF-8; treating them as 3
16+
// bytes causes encodeInto() to reject input that would actually fit.
17+
const text = 'Ѐ'.repeat(33);
18+
const result = encoder.encodeInto(text, new Uint8Array(2));
19+
assert.strictEqual(result.read, 1);
20+
assert.strictEqual(result.written, 2);
21+
}
22+
23+
{
24+
// A one-byte (Latin1) source string takes a different internal path than
25+
// a two-byte (UTF-16) one. Bytes >= 0x80 must be treated as unsigned there
26+
// too, or they get sign-extended into a bogus, oversized code point.
27+
const text = 'é'.repeat(33);
28+
const result = encoder.encodeInto(text, new Uint8Array(2));
29+
assert.strictEqual(result.read, 1);
30+
assert.strictEqual(result.written, 2);
31+
32+
// Appending a two-byte character forces the whole string to be stored as
33+
// UTF-16 internally, which must not change how the Latin1-only prefix
34+
// encodes.
35+
const withTrailingChar = encoder.encodeInto(
36+
text + '☺', new Uint8Array(2));
37+
assert.strictEqual(withTrailingChar.read, result.read);
38+
assert.strictEqual(withTrailingChar.written, result.written);
39+
}
40+
41+
{
42+
// A surrogate pair (U+1F600 here) is 2 UTF-16 code units that must be
43+
// read together: it encodes to 4 UTF-8 bytes, not 3 bytes per code unit.
44+
const text = '\u{1F600}'.repeat(17); // 34 UTF-16 code units.
45+
for (let n = 0; n <= 8; n++) {
46+
const { read, written } = encoder.encodeInto(text, new Uint8Array(n));
47+
// `read` must land on a whole number of surrogate pairs, and the byte
48+
// count for that many pairs must be exactly 4 per pair.
49+
assert.strictEqual(read % 2, 0);
50+
assert.strictEqual(written, (read / 2) * 4);
51+
}
52+
}

0 commit comments

Comments
 (0)