`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>
simpleUtfEncodingLength() in src/encoding_binding.cc uses the wrong boundary for the two byte range:
UTF-8 encodes U+0080 through U+07FF in two bytes. The comment on findBestFit() a few lines below already states the correct rule:
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=100The 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:
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.