Skip to content

util: fix TextEncoder.encodeInto underfilling - #65997

Open
XadillaX wants to merge 1 commit into
nodejs:mainfrom
XadillaX:fix-textencoder-encodeinto-underfill
Open

util: fix TextEncoder.encodeInto underfilling#65997
XadillaX wants to merge 1 commit into
nodejs:mainfrom
XadillaX:fix-textencoder-encodeinto-underfill

Conversation

@XadillaX

Copy link
Copy Markdown
Contributor

TextEncoder.encodeInto() could underfill narrow destination buffers on the optimized path because:

  • UTF-16 code units below U+0800 were incorrectly treated as three-byte UTF-8 sequences starting at U+0400.
  • Latin-1 input was exposed as signed char while calculating the scalar tail.
  • Surrogate pairs in the scalar tail were adjusted only after sizing, which could leave usable destination space unfilled.

Keep one-byte input unsigned, use the correct UTF-8 boundary, and account for surrogate pairs atomically. Use replacement-aware UTF-16 sizing and validate during conversion, avoiding a separate validation pass for valid input. A TODO records the preferred future simdutf bounded-conversion API that would return both consumed and written counts.

Fixes: #65994

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Sep 12, 2026
@XadillaX
XadillaX force-pushed the fix-textencoder-encodeinto-underfill branch from 8afb2c6 to 6417dc3 Compare September 12, 2026 06:30
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: nodejs#65994
Signed-off-by: XadillaX <i@2333.moe>
@XadillaX
XadillaX force-pushed the fix-textencoder-encodeinto-underfill branch from 6417dc3 to fe02f30 Compare September 12, 2026 06:47
@codecov

codecov Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.03%. Comparing base (565f69f) to head (fe02f30).
⚠️ Report is 25 commits behind head on main.

Files with missing lines Patch % Lines
src/encoding_binding.cc 93.93% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65997      +/-   ##
==========================================
+ Coverage   89.99%   90.03%   +0.03%     
==========================================
  Files         784      784              
  Lines      268410   268411       +1     
  Branches    51124    51125       +1     
==========================================
+ Hits       241562   241651      +89     
+ Misses      17385    17291      -94     
- Partials     9463     9469       +6     
Files with missing lines Coverage Δ
src/encoding_binding.cc 87.69% <93.93%> (+26.58%) ⬆️

... and 31 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.

@ronag
ronag requested a review from anonrig September 13, 2026 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TextEncoder.encodeInto() underfills the destination for some non-ASCII text

2 participants