Skip to content

Commit 6417dc3

Browse files
committed
util: fix TextEncoder.encodeInto underfilling
Correct the two-byte UTF-8 boundary and keep Latin-1 input unsigned while finding the prefix that fits. Handle surrogate pairs atomically in the scalar tail. Use replacement-aware UTF-16 sizing and validate during conversion so valid input avoids a separate validation pass. Fixes: #65994 Signed-off-by: XadillaX <i@2333.moe>
1 parent 565f69f commit 6417dc3

2 files changed

Lines changed: 125 additions & 50 deletions

File tree

src/encoding_binding.cc

Lines changed: 71 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,19 @@ constexpr bool isSurrogatePair(uint16_t lead, uint16_t trail) {
8686
return (lead & 0xfc00) == 0xd800 && (trail & 0xfc00) == 0xdc00;
8787
}
8888

89-
constexpr size_t simpleUtfEncodingLength(uint16_t c) {
89+
// V8 exposes one-byte strings as unsigned Latin-1. Keeping that type here
90+
// prevents bytes >= 0x80 from being sign-extended before their length is
91+
// calculated. Every non-ASCII Latin-1 code unit takes two UTF-8 bytes.
92+
constexpr size_t simpleUtfEncodingLength(uint8_t c) {
93+
return c < 0x80 ? 1 : 2;
94+
}
95+
96+
// UTF-16 code units below U+0800 take at most two UTF-8 bytes. Surrogates
97+
// reach this helper only when they are unpaired and therefore encode as the
98+
// three-byte replacement character; valid pairs are handled below.
99+
constexpr size_t simpleUtfEncodingLength(char16_t c) {
90100
if (c < 0x80) return 1;
91-
if (c < 0x400) return 2;
101+
if (c < 0x800) return 2;
92102
return 3;
93103
}
94104

@@ -114,6 +124,12 @@ constexpr size_t simpleUtfEncodingLength(uint16_t c) {
114124
// multi-byte heavy content.
115125
template <typename Char>
116126
size_t findBestFit(const Char* data, size_t length, size_t bufferSize) {
127+
// TODO: Replace prefix sizing with a bounded simdutf conversion that returns
128+
// both input code units consumed and output bytes written, while replacing
129+
// invalid UTF-16. The current safe conversion APIs return only the number of
130+
// output bytes, but encodeInto() requires both values. Such an API would let
131+
// this function and the separate conversion pass be replaced by one SIMD
132+
// operation.
117133
size_t pos = 0;
118134
size_t utf8Accumulated = 0;
119135
constexpr size_t CHUNK = 257;
@@ -141,14 +157,19 @@ size_t findBestFit(const Char* data, size_t length, size_t bufferSize) {
141157

142158
size_t chunkUtf8Len;
143159
if constexpr (UTF16) {
144-
// TODO(anonrig): Use utf8_length_from_utf16_with_replacement when
145-
// available For now, validate and use utf8_length_from_utf16
160+
// Keep valid surrogate pairs in the same chunk. Otherwise the lead and
161+
// trail would each be measured as an unpaired U+FFFD replacement.
146162
size_t newPos = pos + chunkSize;
147163
if (newPos < length && isSurrogatePair(data[newPos - 1], data[newPos]))
148164
chunkSize--;
149-
chunkUtf8Len = simdutf::utf8_length_from_utf16(data + pos, chunkSize);
165+
// Replacement-aware sizing matches TextEncoder for malformed UTF-16 and
166+
// avoids a separate validation pass over the entire input.
167+
chunkUtf8Len = simdutf::utf8_length_from_utf16_with_replacement(
168+
data + pos, chunkSize)
169+
.count;
150170
} else {
151-
chunkUtf8Len = simdutf::utf8_length_from_latin1(data + pos, chunkSize);
171+
chunkUtf8Len = simdutf::utf8_length_from_latin1(
172+
reinterpret_cast<const char*>(data + pos), chunkSize);
152173
}
153174

154175
if (utf8Accumulated + chunkUtf8Len > bufferSize) {
@@ -161,20 +182,27 @@ size_t findBestFit(const Char* data, size_t length, size_t bufferSize) {
161182
}
162183
}
163184

185+
// Finish near the destination boundary without invoking a SIMD length
186+
// routine for each remaining code unit.
164187
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++;
188+
size_t codeUnits = 1;
189+
size_t extra;
190+
if constexpr (UTF16) {
191+
// Consume a valid pair atomically and keep scanning if space remains.
192+
// Counting its halves separately as three bytes each would underfill
193+
// the destination because the pair encodes to four bytes in total.
194+
if (pos + 1 < length && isSurrogatePair(data[pos], data[pos + 1])) {
195+
codeUnits = 2;
196+
extra = 4;
197+
} else {
198+
extra = simpleUtfEncodingLength(data[pos]);
199+
}
175200
} else {
176-
pos--;
201+
extra = simpleUtfEncodingLength(data[pos]);
177202
}
203+
if (utf8Accumulated + extra > bufferSize) break;
204+
pos += codeUnits;
205+
utf8Accumulated += extra;
178206
}
179207
return pos;
180208
}
@@ -239,9 +267,11 @@ void BindingData::EncodeInto(const FunctionCallbackInfo<Value>& args) {
239267
std::min(static_cast<size_t>(view.length()), dest_length);
240268

241269
if (view.is_one_byte()) {
242-
auto data = reinterpret_cast<const char*>(view.data8());
243-
simdutf::result result =
244-
simdutf::validate_ascii_with_errors(data, length_that_fits);
270+
// Keep V8's unsigned representation while doing scalar length checks.
271+
// simdutf accepts const char* but interprets the same bytes as Latin-1.
272+
const uint8_t* data = view.data8();
273+
simdutf::result result = simdutf::validate_ascii_with_errors(
274+
reinterpret_cast<const char*>(data), length_that_fits);
245275
written = read = result.count;
246276
memcpy(write_result, data, read);
247277
write_result += read;
@@ -250,8 +280,9 @@ void BindingData::EncodeInto(const FunctionCallbackInfo<Value>& args) {
250280
dest_length -= read;
251281
if (length_that_fits != 0 && dest_length != 0) {
252282
if (size_t rest = findBestFit(data, length_that_fits, dest_length)) {
253-
DCHECK_LE(simdutf::utf8_length_from_latin1(data, rest), dest_length);
254-
written += simdutf::convert_latin1_to_utf8(data, rest, write_result);
283+
const char* latin1 = reinterpret_cast<const char*>(data);
284+
DCHECK_LE(simdutf::utf8_length_from_latin1(latin1, rest), dest_length);
285+
written += simdutf::convert_latin1_to_utf8(latin1, rest, write_result);
255286
read += rest;
256287
}
257288
}
@@ -266,34 +297,24 @@ void BindingData::EncodeInto(const FunctionCallbackInfo<Value>& args) {
266297
length_that_fits--;
267298
}
268299

269-
// Check if input has unpaired surrogates - if so, convert to well-formed
270-
// first
271-
simdutf::result validation_result =
272-
simdutf::validate_utf16_with_errors(data, length_that_fits);
273-
274-
if (validation_result.error == simdutf::SUCCESS) {
275-
// Valid UTF-16 - use the fast path
276-
read = findBestFit(data, length_that_fits, dest_length);
277-
if (read != 0) {
278-
DCHECK_LE(simdutf::utf8_length_from_utf16(data, read), dest_length);
279-
written = simdutf::convert_utf16_to_utf8(data, read, write_result);
280-
}
281-
} else {
282-
// Invalid UTF-16 with unpaired surrogates - convert to well-formed first
283-
// TODO(anonrig): Use utf8_length_from_utf16_with_replacement when
284-
// available
285-
MaybeStackBuffer<char16_t, MAX_SIZE_FOR_STACK_ALLOC> conversion_buffer(
286-
length_that_fits);
287-
simdutf::to_well_formed_utf16(
288-
data, length_that_fits, conversion_buffer.out());
289-
290-
// Now use findBestFit with the well-formed data
291-
read =
292-
findBestFit(conversion_buffer.out(), length_that_fits, dest_length);
293-
if (read != 0) {
294-
DCHECK_LE(
295-
simdutf::utf8_length_from_utf16(conversion_buffer.out(), read),
296-
dest_length);
300+
// Prefix sizing already accounts for replacement characters. Validate
301+
// while converting so well-formed UTF-16 needs no separate validation
302+
// scan.
303+
read = findBestFit(data, length_that_fits, dest_length);
304+
if (read != 0) {
305+
DCHECK_LE(
306+
simdutf::utf8_length_from_utf16_with_replacement(data, read).count,
307+
dest_length);
308+
simdutf::result conversion_result =
309+
simdutf::convert_utf16_to_utf8_with_errors(data, read, write_result);
310+
if (conversion_result.error == simdutf::SUCCESS) {
311+
written = conversion_result.count;
312+
} else {
313+
// The failed conversion may have written a valid prefix. Overwrite it
314+
// with a well-formed copy where every unpaired surrogate is U+FFFD.
315+
MaybeStackBuffer<char16_t, MAX_SIZE_FOR_STACK_ALLOC> conversion_buffer(
316+
read);
317+
simdutf::to_well_formed_utf16(data, read, conversion_buffer.out());
297318
written = simdutf::convert_utf16_to_utf8(
298319
conversion_buffer.out(), read, write_result);
299320
}
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
'use strict';
2+
3+
require('../common');
4+
const assert = require('assert');
5+
6+
const encoder = new TextEncoder();
7+
8+
function assertExactFit(character, expectedBytes) {
9+
const input = character.repeat(33);
10+
const destination = new Uint8Array(expectedBytes.length);
11+
12+
assert.deepStrictEqual(
13+
encoder.encodeInto(input, destination),
14+
{ read: character.length, written: expectedBytes.length });
15+
assert.deepStrictEqual(destination, Uint8Array.from(expectedBytes));
16+
17+
assert.deepStrictEqual(
18+
encoder.encodeInto(input, new Uint8Array(expectedBytes.length - 1)),
19+
{ read: 0, written: 0 });
20+
}
21+
22+
// Exercise the optimized path with UTF-8 length boundaries.
23+
assertExactFit('\u03ff', [0xcf, 0xbf]);
24+
assertExactFit('\u0400', [0xd0, 0x80]);
25+
assertExactFit('\u07ff', [0xdf, 0xbf]);
26+
assertExactFit('\u0800', [0xe0, 0xa0, 0x80]);
27+
28+
// One-byte V8 strings must treat Latin-1 code units as unsigned.
29+
assertExactFit('\x80', [0xc2, 0x80]);
30+
assertExactFit('\xff', [0xc3, 0xbf]);
31+
32+
// Appending a two-byte character changes V8's string representation but must
33+
// not affect how much of the preceding text can be encoded.
34+
{
35+
const destination = new Uint8Array(2);
36+
assert.deepStrictEqual(
37+
encoder.encodeInto(`${'\xe9'.repeat(33)}\u263a`, destination),
38+
{ read: 1, written: 2 });
39+
assert.deepStrictEqual(destination, Uint8Array.from([0xc3, 0xa9]));
40+
}
41+
42+
// Keep surrogate handling covered when validation happens during conversion.
43+
assertExactFit('\ud83d\ude00', [0xf0, 0x9f, 0x98, 0x80]);
44+
assertExactFit('\ud800', [0xef, 0xbf, 0xbd]);
45+
46+
// Continue filling the destination after a surrogate pair in the scalar tail.
47+
{
48+
const input = '\u0800\u0800\ud83d\ude00a'.repeat(7);
49+
const destination = new Uint8Array(22);
50+
assert.deepStrictEqual(
51+
encoder.encodeInto(input, destination),
52+
{ read: 10, written: 22 });
53+
assert.deepStrictEqual(destination, encoder.encode(input.slice(0, 10)));
54+
}

0 commit comments

Comments
 (0)