Skip to content

src: fix UTF-8 two byte threshold in encodeInto - #65067

Open
shani-singh1 wants to merge 1 commit into
nodejs:mainfrom
shani-singh1:encoding-encodeinto-utf8-threshold
Open

src: fix UTF-8 two byte threshold in encodeInto#65067
shani-singh1 wants to merge 1 commit into
nodejs:mainfrom
shani-singh1:encoding-encodeinto-utf8-threshold

Conversation

@shani-singh1

Copy link
Copy Markdown

simpleUtfEncodingLength() in src/encoding_binding.cc uses the wrong boundary for the two byte range:

constexpr size_t simpleUtfEncodingLength(uint16_t c) {
  if (c < 0x80) return 1;
  if (c < 0x400) return 2;   // UTF-8 uses two bytes up to U+07FF, not U+03FF
  return 3;
}

UTF-8 encodes U+0080 through U+07FF in two bytes. The comment on findBestFit() a few lines below already states the correct rule:

// - ASCII (< 0x80): 1 byte
// - Code points < 0x800: 2 bytes
// - Other BMP characters: 3 bytes

So every code point from U+0400 upwards is costed as three bytes when it actually takes two. That covers Cyrillic (U+0400 to U+04FF), Hebrew, Arabic, Syriac, Thaana and NKo, among others.

Why it matters

simpleUtfEncodingLength() is the character-by-character boundary refinement step of findBestFit() (src/encoding_binding.cc:165), and findBestFit() decides how many code units of the input fit in the destination of TextEncoder.prototype.encodeInto() (call sites at :252, :276 and :292). Its return value becomes read, and written is derived from converting exactly that many code units.

Over-costing makes the loop break earlier than it should, so encodeInto() stops short of filling the destination and reports read and written values that are too small. The WHATWG Encoding Standard requires encodeInto() to write as many code points as fit.

This is a correctness bug only. The helper always over-estimates and never under-estimates, so the destination is never overrun.

Measured effect

findBestFit() transcribed faithfully into JavaScript and run with both thresholds, for a string of U+0431 (Cyrillic small letter ve, two UTF-8 bytes per character):

 dest | current (0x400)    | fixed (0x800)      | optimal
 -----+--------------------+--------------------+-------------------
    2 | read=  0 wrote=  0 | read=  1 wrote=  2 | read=  1 wrote=  2
    4 | read=  1 wrote=  2 | read=  2 wrote=  4 | read=  2 wrote=  4
   10 | read=  4 wrote=  8 | read=  5 wrote= 10 | read=  5 wrote= 10
   40 | read= 19 wrote= 38 | read= 20 wrote= 40 | read= 20 wrote= 40
  100 | read= 49 wrote= 98 | read= 50 wrote=100 | read= 50 wrote=100

The two byte destination is the clearest case: a two byte character is currently not written at all into a buffer that has exactly enough room for it.

Two checks on that model, both against node itself:

  • its UTF-8 lengths match Buffer.byteLength(s, 'utf8') for every code point in U+0000 to U+0FFF;
  • with the corrected threshold, simpleUtfEncodingLength(cp) equals the real UTF-8 length for every non-surrogate code point in the BMP, with zero mismatches.

Test

test/parallel/test-text-encoder-encode-into-two-byte.js checks encodeInto() against destinations of 2, 3, 4, 10, 40 and 100 bytes for Cyrillic, Hebrew and U+07FF, asserting read, written and the written bytes. It also keeps a three byte code point (U+0800) costed at three bytes.

The test passes on v24.11.1, which predates findBestFit() and does not have the defect, so the assertions describe behaviour that already held before this code was introduced.

@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 Aug 6, 2026
`simpleUtfEncodingLength()` returned 3 for every code point at or above
U+0400, but UTF-8 encodes U+0080 through U+07FF in two bytes. The
comment on `findBestFit()` a few lines below already states the correct
rule.

The helper is the boundary refinement step of `findBestFit()`, which
decides how many code units of the input fit in the destination of
`TextEncoder.prototype.encodeInto()`. Over-costing Cyrillic, Hebrew,
Arabic and the other two byte blocks made it stop early, so
`encodeInto()` left the destination short and under-reported `read` and
`written`. The helper only ever over-estimates, so the destination is
never overrun.

Signed-off-by: Shani Singh <teamdeveloperworld@gmail.com>
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.

2 participants