Skip to content

Commit dbe5898

Browse files
mcollinaaduh95
authored andcommitted
stream: fix UTF-8 character corruption in fast-utf8-stream
Fix releaseWritingBuf() to correctly handle partial writes that split multi-byte UTF-8 characters. The previous implementation incorrectly converted byte counts to character counts, causing: - 3-byte characters (CJK) to be silently dropped - 4-byte characters (emoji) to leave lone surrogates in the buffer The fix backs up from the byte position to find a valid UTF-8 character boundary by checking for continuation bytes (pattern 10xxxxxx), then decodes the properly-aligned bytes to get the correct character count. Also fixes a typo where this._asyncDrainScheduled was used instead of the private field this.#asyncDrainScheduled. Fixes: #61744 PR-URL: #61745 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it>
1 parent bd34e53 commit dbe5898

2 files changed

Lines changed: 340 additions & 6 deletions

File tree

‎lib/internal/streams/fast-utf8-stream.js‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ class Utf8Stream extends EventEmitter {
237237

238238
this.on('newListener',(name)=>{
239239
if(name==='drain'){
240-
this._asyncDrainScheduled=false;
240+
this.#asyncDrainScheduled=false;
241241
}
242242
});
243243

@@ -894,11 +894,23 @@ class Utf8Stream extends EventEmitter {
894894
* @returns {{writingBuf: string | Buffer, len: number}} released writingBuf and length
895895
*/
896896
functionreleaseWritingBuf(writingBuf,len,n){
897-
// if Buffer.byteLength is equal to n, that means writingBuf contains no multi-byte character
898-
if(typeofwritingBuf==='string'&&Buffer.byteLength(writingBuf)!==n){
899-
// Since the fs.write callback parameter `n` means how many bytes the passed of string
900-
// We calculate the original string length for avoiding the multi-byte character issue
901-
n=Buffer.from(writingBuf).subarray(0,n).toString().length;
897+
if(typeofwritingBuf==='string'){
898+
constbyteLength=Buffer.byteLength(writingBuf);
899+
if(byteLength!==n){
900+
// Since fs.write returns the number of bytes written, we need to find
901+
// how many complete characters fit within those n bytes.
902+
// If a partial write splits a multi-byte UTF-8 character, we must back up
903+
// to the start of that character to avoid data corruption.
904+
constbuf=Buffer.from(writingBuf);
905+
// Back up from position n to find a valid UTF-8 character boundary.
906+
// UTF-8 continuation bytes have the pattern 10xxxxxx (0x80-0xBF).
907+
// We need to find the start of the character that was split.
908+
while(n>0&&(buf[n]&0xC0)===0x80){
909+
n--;
910+
}
911+
// Decode the properly-aligned bytes to get the character count.
912+
n=buf.subarray(0,n).toString().length;
913+
}
902914
}
903915
len=MathMax(len-n,0);
904916
writingBuf=writingBuf.slice(n);
Lines changed: 322 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,322 @@
1+
'use strict';
2+
3+
// Tests for UTF-8 character preservation when partial writes split multi-byte characters.
4+
// See: https://github.com/nodejs/node/issues/61744
5+
6+
constcommon=require('../common');
7+
consttmpdir=require('../common/tmpdir');
8+
constassert=require('node:assert');
9+
const{
10+
openSync,
11+
write,
12+
writeSync,
13+
}=require('node:fs');
14+
const{ Utf8Stream }=require('node:fs');
15+
const{ join }=require('node:path');
16+
const{ isMainThread }=require('node:worker_threads');
17+
18+
tmpdir.refresh();
19+
if(isMainThread){
20+
process.umask(0o000);
21+
}
22+
23+
letfileCounter=0;
24+
25+
functiongetTempFile(){
26+
returnjoin(tmpdir.path,`fastutf8stream-partial-${process.pid}-${Date.now()}-${fileCounter++}.log`);
27+
}
28+
29+
runTests(false);
30+
runTests(true);
31+
32+
functionrunTests(sync){
33+
// Test 1: Partial write splitting a 3-byte UTF-8 character (CJK)
34+
// "abc中def" where "中" is 3 bytes (E4 B8 AD)
35+
// Simulate partial write of 4 bytes: "abc" (3 bytes) + first byte of "中"
36+
// The remaining buffer should be "中def" (not "def")
37+
{
38+
constdest=getTempFile();
39+
constfd=openSync(dest,'w');
40+
41+
letfirstWrite=true;
42+
constwrittenChunks=[];
43+
constfsOverride={};
44+
45+
if(sync){
46+
fsOverride.writeSync=common.mustCall((...args)=>{
47+
constdata=args[1];
48+
writtenChunks.push(typeofdata==='string' ? data : data.toString());
49+
if(firstWrite){
50+
firstWrite=false;
51+
// Simulate partial write: only 4 bytes written out of 9
52+
// This splits the 3-byte "中" character
53+
return4;
54+
}
55+
returnwriteSync(...args);
56+
},2);
57+
}else{
58+
fsOverride.write=common.mustCall((...args)=>{
59+
constdata=args[1];
60+
writtenChunks.push(typeofdata==='string' ? data : data.toString());
61+
constcallback=args[args.length-1];
62+
if(firstWrite){
63+
firstWrite=false;
64+
// Simulate partial write: only 4 bytes written out of 9
65+
process.nextTick(callback,null,4);
66+
return;
67+
}
68+
returnwrite(...args);
69+
},2);
70+
}
71+
72+
conststream=newUtf8Stream({
73+
fd,
74+
sync,
75+
minLength: 0,
76+
fs: fsOverride,
77+
});
78+
79+
stream.on('ready',common.mustCall(()=>{
80+
stream.write('abc中def');
81+
stream.end();
82+
83+
stream.on('finish',common.mustCall(()=>{
84+
// Verify the second chunk contains the preserved CJK character
85+
assert.strictEqual(writtenChunks.length,2);
86+
assert.strictEqual(writtenChunks[0],'abc中def');// First attempt
87+
assert.strictEqual(writtenChunks[1],'中def');// Retry with preserved char
88+
}));
89+
}));
90+
}
91+
92+
// Test 2: Partial write splitting a 4-byte UTF-8 character (emoji)
93+
// "hello🌍world" where "🌍" is 4 bytes (F0 9F 8C 8D)
94+
// Simulate partial write of 7 bytes: "hello" (5 bytes) + first 2 bytes of "🌍"
95+
// The remaining buffer should be "🌍world" (not a lone surrogate + "world")
96+
{
97+
constdest=getTempFile();
98+
constfd=openSync(dest,'w');
99+
100+
letfirstWrite=true;
101+
constwrittenChunks=[];
102+
constfsOverride={};
103+
104+
if(sync){
105+
fsOverride.writeSync=common.mustCall((...args)=>{
106+
constdata=args[1];
107+
writtenChunks.push(typeofdata==='string' ? data : data.toString());
108+
if(firstWrite){
109+
firstWrite=false;
110+
// Simulate partial write: only 7 bytes written
111+
return7;
112+
}
113+
returnwriteSync(...args);
114+
},2);
115+
}else{
116+
fsOverride.write=common.mustCall((...args)=>{
117+
constdata=args[1];
118+
writtenChunks.push(typeofdata==='string' ? data : data.toString());
119+
constcallback=args[args.length-1];
120+
if(firstWrite){
121+
firstWrite=false;
122+
process.nextTick(callback,null,7);
123+
return;
124+
}
125+
returnwrite(...args);
126+
},2);
127+
}
128+
129+
conststream=newUtf8Stream({
130+
fd,
131+
sync,
132+
minLength: 0,
133+
fs: fsOverride,
134+
});
135+
136+
stream.on('ready',common.mustCall(()=>{
137+
stream.write('hello🌍world');
138+
stream.end();
139+
140+
stream.on('finish',common.mustCall(()=>{
141+
assert.strictEqual(writtenChunks.length,2);
142+
assert.strictEqual(writtenChunks[0],'hello🌍world');// First attempt
143+
assert.strictEqual(writtenChunks[1],'🌍world');// Retry with preserved emoji
144+
145+
// Verify no lone surrogates in the retry chunk
146+
constretryChunk=writtenChunks[1];
147+
for(leti=0;i<retryChunk.length;i++){
148+
constcode=retryChunk.charCodeAt(i);
149+
if(code>=0xD800&&code<=0xDBFF){
150+
// High surrogate - next must be low surrogate
151+
constnext=retryChunk.charCodeAt(i+1);
152+
assert.ok(next>=0xDC00&&next<=0xDFFF,
153+
`Found lone high surrogate at position ${i}`);
154+
i++;// Skip the low surrogate we just verified
155+
}elseif(code>=0xDC00&&code<=0xDFFF){
156+
// Low surrogate without preceding high surrogate
157+
assert.fail(`Found lone low surrogate at position ${i}: 0x${code.toString(16)}`);
158+
}
159+
}
160+
}));
161+
}));
162+
}
163+
164+
// Test 3: Partial write at exactly 0 bytes (edge case)
165+
{
166+
constdest=getTempFile();
167+
constfd=openSync(dest,'w');
168+
169+
letfirstWrite=true;
170+
constwrittenChunks=[];
171+
constfsOverride={};
172+
173+
if(sync){
174+
fsOverride.writeSync=common.mustCall((...args)=>{
175+
constdata=args[1];
176+
writtenChunks.push(typeofdata==='string' ? data : data.toString());
177+
if(firstWrite){
178+
firstWrite=false;
179+
return0;// No bytes written
180+
}
181+
returnwriteSync(...args);
182+
},2);
183+
}else{
184+
fsOverride.write=common.mustCall((...args)=>{
185+
constdata=args[1];
186+
writtenChunks.push(typeofdata==='string' ? data : data.toString());
187+
constcallback=args[args.length-1];
188+
if(firstWrite){
189+
firstWrite=false;
190+
process.nextTick(callback,null,0);
191+
return;
192+
}
193+
returnwrite(...args);
194+
},2);
195+
}
196+
197+
conststream=newUtf8Stream({
198+
fd,
199+
sync,
200+
minLength: 0,
201+
fs: fsOverride,
202+
});
203+
204+
stream.on('ready',common.mustCall(()=>{
205+
stream.write('中文');
206+
stream.end();
207+
208+
stream.on('finish',common.mustCall(()=>{
209+
assert.strictEqual(writtenChunks.length,2);
210+
assert.strictEqual(writtenChunks[0],'中文');
211+
assert.strictEqual(writtenChunks[1],'中文');// Entire string retried
212+
}));
213+
}));
214+
}
215+
216+
// Test 4: Partial write splitting between characters (not mid-character)
217+
// This should work the same as before - no character preservation needed
218+
{
219+
constdest=getTempFile();
220+
constfd=openSync(dest,'w');
221+
222+
letfirstWrite=true;
223+
constwrittenChunks=[];
224+
constfsOverride={};
225+
226+
if(sync){
227+
fsOverride.writeSync=common.mustCall((...args)=>{
228+
constdata=args[1];
229+
writtenChunks.push(typeofdata==='string' ? data : data.toString());
230+
if(firstWrite){
231+
firstWrite=false;
232+
// Write exactly 3 bytes ("abc"), which is a clean character boundary
233+
return3;
234+
}
235+
returnwriteSync(...args);
236+
},2);
237+
}else{
238+
fsOverride.write=common.mustCall((...args)=>{
239+
constdata=args[1];
240+
writtenChunks.push(typeofdata==='string' ? data : data.toString());
241+
constcallback=args[args.length-1];
242+
if(firstWrite){
243+
firstWrite=false;
244+
process.nextTick(callback,null,3);
245+
return;
246+
}
247+
returnwrite(...args);
248+
},2);
249+
}
250+
251+
conststream=newUtf8Stream({
252+
fd,
253+
sync,
254+
minLength: 0,
255+
fs: fsOverride,
256+
});
257+
258+
stream.on('ready',common.mustCall(()=>{
259+
stream.write('abc中def');
260+
stream.end();
261+
262+
stream.on('finish',common.mustCall(()=>{
263+
assert.strictEqual(writtenChunks.length,2);
264+
assert.strictEqual(writtenChunks[0],'abc中def');
265+
assert.strictEqual(writtenChunks[1],'中def');// Remaining after 3 bytes
266+
}));
267+
}));
268+
}
269+
270+
// Test 5: Single multi-byte character with partial write of 1 byte
271+
{
272+
constdest=getTempFile();
273+
constfd=openSync(dest,'w');
274+
275+
letfirstWrite=true;
276+
constwrittenChunks=[];
277+
constfsOverride={};
278+
279+
if(sync){
280+
fsOverride.writeSync=common.mustCall((...args)=>{
281+
constdata=args[1];
282+
writtenChunks.push(typeofdata==='string' ? data : data.toString());
283+
if(firstWrite){
284+
firstWrite=false;
285+
// Write only 1 byte of a 3-byte character
286+
return1;
287+
}
288+
returnwriteSync(...args);
289+
},2);
290+
}else{
291+
fsOverride.write=common.mustCall((...args)=>{
292+
constdata=args[1];
293+
writtenChunks.push(typeofdata==='string' ? data : data.toString());
294+
constcallback=args[args.length-1];
295+
if(firstWrite){
296+
firstWrite=false;
297+
process.nextTick(callback,null,1);
298+
return;
299+
}
300+
returnwrite(...args);
301+
},2);
302+
}
303+
304+
conststream=newUtf8Stream({
305+
fd,
306+
sync,
307+
minLength: 0,
308+
fs: fsOverride,
309+
});
310+
311+
stream.on('ready',common.mustCall(()=>{
312+
stream.write('中');
313+
stream.end();
314+
315+
stream.on('finish',common.mustCall(()=>{
316+
assert.strictEqual(writtenChunks.length,2);
317+
assert.strictEqual(writtenChunks[0],'中');
318+
assert.strictEqual(writtenChunks[1],'中');// Full character retried
319+
}));
320+
}));
321+
}
322+
}

0 commit comments

Comments
 (0)