nodejs/node

TextDecoder does not error incorrectly for legacy byte sequences

Open

#40,091 opened on Sep 12, 2021

View on GitHub
 (12 comments) (4 reactions) (0 assignees)JavaScript (35,535 forks)batch import
encodinggood first issue

Repository metrics

Stars
 (117,218 stars)
PR merge metrics
 (Avg merge 13d 4h) (233 merged PRs in 30d)

Description

Version

v16.9.1

Platform

Microsoft Windows NT 10.0.19043.0 x64

Subsystem

encoding

What steps will reproduce the bug?

Enter the following in the REPL:

new TextDecoder("Big5").decode(new Uint8Array([0x83, 0x5C])).charCodeAt(0).toString(16)

as well as

new TextDecoder("Big5").decode(new Uint8Array([0x83, 0x5C])).charCodeAt(1).toString(16)

How often does it reproduce? Is there a required condition?

Every time

What is the expected behavior?

fffd for the first, and 5c for the second (as in Firefox and Chrome, and per the WHATWG Encoding Standard)

What do you see instead?

f00e and NaN

Additional information

I suspect this has to do with you using ICU as-is, instead of properly patching it to match the Encoding Standard. There are probably more bugs like this.

@inexorabletash may be able to point to where in the Chromium source tree we keep our ICU encoding patches.

Contributor guide

Research direction
Compare the ICU encoding patches used in Node.js with the WHATWG Encoding Standard for Big5, focusing on the byte sequence 0x83 0x5C.
Tech stack
javascript
Domain
backend
Issue type
Bug
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
3
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
1-3 hours
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
Active
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
Clear
Prerequisites
JavaScriptNode.js
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
40