Skip to content

stream: fix UTF-8 character corruption in fast-utf8-stream#61745

Open
mcollina wants to merge 1 commit intonodejs:mainfrom
mcollina:fix-utf8-stream-partial-write
Open

stream: fix UTF-8 character corruption in fast-utf8-stream#61745
mcollina wants to merge 1 commit intonodejs:mainfrom
mcollina:fix-utf8-stream-partial-write

Conversation

@mcollina
Copy link
Member

@mcollina mcollina commented Feb 8, 2026

Summary

Fix releaseWritingBuf() to correctly handle partial writes that split multi-byte UTF-8 characters.

The previous implementation incorrectly converted byte counts to character counts by using:

n = Buffer.from(writingBuf).subarray(0, n).toString().length;

When n bytes cuts through a multi-byte character, the incomplete UTF-8 sequence becomes U+FFFD (replacement character) via .toString(), which has a different .length than the original character. This caused:

  • 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

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: nodejs#61744
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. labels Feb 8, 2026
@codecov
Copy link

codecov bot commented Feb 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.74%. Comparing base (f77a709) to head (4f5cf65).
⚠️ Report is 208 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #61745      +/-   ##
==========================================
- Coverage   89.74%   89.74%   -0.01%     
==========================================
  Files         674      675       +1     
  Lines      204360   204537     +177     
  Branches    39265    39308      +43     
==========================================
+ Hits       183412   183569     +157     
- Misses      13251    13257       +6     
- Partials     7697     7711      +14     
Files with missing lines Coverage Δ
lib/internal/streams/fast-utf8-stream.js 74.57% <100.00%> (+0.33%) ⬆️

... and 44 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 9, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 9, 2026
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 10, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 10, 2026
@addaleax addaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 11, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 11, 2026
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 13, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 13, 2026
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 13, 2026
@mcollina
Copy link
Member Author

Manual CI: https://ci.nodejs.org/job/node-test-pull-request/71336/

(no idea why request-ci did not work here)

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 13, 2026
@mcollina
Copy link
Member Author

@nodejs/build can someone help to see why CI is not starting?

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 17, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 18, 2026
@ShogunPanda ShogunPanda added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Feb 26, 2026
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Feb 26, 2026
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/61745
✔  Done loading data for nodejs/node/pull/61745
----------------------------------- PR info ------------------------------------
Title      stream: fix UTF-8 character corruption in fast-utf8-stream (#61745)
Author     Matteo Collina <matteo.collina@gmail.com> (@mcollina)
Branch     mcollina:fix-utf8-stream-partial-write -> nodejs:main
Labels     stream, needs-ci, commit-queue-squash
Commits    1
 - stream: fix UTF-8 character corruption in fast-utf8-stream
Committers 1
 - Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/61745
Fixes: https://github.com/nodejs/node/issues/61744
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>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/61745
Fixes: https://github.com/nodejs/node/issues/61744
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>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sun, 08 Feb 2026 20:39:59 GMT
   ✔  Approvals: 4
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/61745#pullrequestreview-3770760306
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/61745#pullrequestreview-3774539778
   ✔  - Colin Ihrig (@cjihrig): https://github.com/nodejs/node/pull/61745#pullrequestreview-3778727542
   ✔  - Paolo Insogna (@ShogunPanda) (TSC): https://github.com/nodejs/node/pull/61745#pullrequestreview-3859183492
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-02-13T22:03:07Z: https://ci.nodejs.org/job/node-test-pull-request/71336/
- Querying data for job/node-test-pull-request/71336/
✔  Build data downloaded
   ✘  1 failure(s) on the last Jenkins CI run
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/22433955375

@mcollina mcollina added request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Feb 26, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 26, 2026
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 1, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 2, 2026
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Mar 3, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 3, 2026
@aduh95
Copy link
Contributor

aduh95 commented Mar 3, 2026

fatal: couldn't find remote ref refs/pull/61745/head 🤔

@mcollina
Copy link
Member Author

mcollina commented Mar 3, 2026

Should I open a fresh PR?

@anonrig
Copy link
Member

anonrig commented Mar 3, 2026

Rebasing and force pushing might fix the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UTF-8 character corruption in fast-utf8-stream.js via releaseWritingBuf() leads to silent data loss on partial writes

8 participants