Commit 69b662e
TextDecoder: accept all WHATWG utf-8 encoding labels (#198)
## What
`TextDecoder`'s constructor only accepted the exact labels `"utf-8"` and
`"UTF-8"`, throwing for every other spelling. This PR makes it accept
all WHATWG-spec UTF-8 labels.
## Why
Per the [WHATWG Encoding
Standard](https://encoding.spec.whatwg.org/#concept-encoding-get), an
encoding label is matched after stripping leading/trailing ASCII
whitespace and ASCII-lowercasing, and several labels all decode as
UTF-8: `utf-8`, `utf8`, `unicode-1-1-utf-8`, `unicode11utf8`,
`unicode20utf8`, `x-unicode20utf8`.
Real consumers rely on this. The Babylon.js glTF/Draco loader constructs
`new TextDecoder("utf8")` (no hyphen). With the old check that threw,
decoding aborted mid-load. In **Babylon Native** the aborted load left
the loader in a state that drove a **native out-of-bounds write**, which
surfaced as non-deterministic **heap corruption**
(`STATUS_HEAP_CORRUPTION`, `0xC0000374`) on the Draco mesh-compression
validation tests.
## Fix
Normalize the label per the spec (trim ASCII whitespace +
ASCII-lowercase) and accept the full set of UTF-8 labels; still throw
for genuinely unsupported encodings.
## Verification
- Two BabylonNative Playground Draco validation tests that previously
crashed with heap corruption (`GLTF Serializer KHR draco mesh
compression`, `GLTF Buggy with Draco Mesh Compression`) now **pass**
(3/3 runs each) with this fix vendored in; a third (`GLTF Box with bad
Draco normalized flag`) no longer crashes.
- Adds unit tests covering the `utf8` label, case/whitespace variants,
the other UTF-8 aliases, and a still-rejected encoding (`utf-16`).
---
## Update: also fixes a latent ChakraCore N-API heap-corruption bug
CI surfaced that the new "throws for an unsupported encoding" test
reliably **heap-corrupts on Chakra** (Win32 x64/x86) —
`STATUS_HEAP_CORRUPTION` (0xC0000374) / access violations, crashing
later in unrelated tests. This is a **pre-existing latent bug** that
this PR's first constructor-throw test merely exposed (it does not
reproduce on `main` only because no existing test throws from a wrapped
constructor; V8/JSC/JSI and the sanitizer jobs are unaffected since the
bug is in the Chakra N-API shim).
**Root cause:** when a napi `ObjectWrap` constructor throws, the C++
instance is destroyed during stack unwinding, but the wrap finalizer
registered by `napi_wrap()` stays attached to `this`. A later GC then
runs the finalizer on the freed instance → use-after-free / double-free.
The addon-api `~ObjectWrap()` is meant to detach the wrap on failure,
but it obtains `this` via `napi_get_reference_value()` on the wrap's
weak (refcount-0) reference — and the Chakra backend returns `null` for
**every** refcount-0 reference (the in-box `jsrt` API can't track
weak-reference liveness), so the detach is skipped.
**Fix** (`Core/Node-API/Source/js_native_api_chakra.cc`):
- In the Chakra construct-call trampoline, if the callback leaves a
pending JS exception, detach any wrap left on `this` via
`napi_remove_wrap()` (preserving the pending exception) so the finalizer
can't run on the freed instance.
- Guard `napi_remove_wrap()` against a null `result` out-param (now
called with `nullptr`, matching upstream).
**Verification:** built the Chakra UnitTests locally; the
constructor-throw scenario went from **heap corruption every run** to
**clean (8/8 runs, all tests passing)**. Added a regression test that
throws from a wrapped constructor 100× then exercises the heap.
> Note: the N-API engine fix is logically independent of the TextDecoder
label change and could be split into its own PR if preferred — it is
bundled here because this PR's constructor-throw test is what exposes it
and is its regression test.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>1 parent 2ff325e commit 69b662e
2 files changed
Lines changed: 65 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
9 | 39 | | |
10 | 40 | | |
11 | 41 | | |
| |||
33 | 63 | | |
34 | 64 | | |
35 | 65 | | |
36 | | - | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
37 | 76 | | |
38 | | - | |
| 77 | + | |
39 | 78 | | |
40 | 79 | | |
41 | 80 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1504 | 1504 | | |
1505 | 1505 | | |
1506 | 1506 | | |
| 1507 | + | |
| 1508 | + | |
| 1509 | + | |
| 1510 | + | |
| 1511 | + | |
| 1512 | + | |
| 1513 | + | |
| 1514 | + | |
| 1515 | + | |
| 1516 | + | |
| 1517 | + | |
| 1518 | + | |
| 1519 | + | |
| 1520 | + | |
| 1521 | + | |
| 1522 | + | |
| 1523 | + | |
| 1524 | + | |
| 1525 | + | |
| 1526 | + | |
| 1527 | + | |
| 1528 | + | |
| 1529 | + | |
| 1530 | + | |
1507 | 1531 | | |
1508 | 1532 | | |
1509 | 1533 | | |
| |||
0 commit comments