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 5 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.

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