Skip to content

SINGLE_FILE_BINARY_ENCODE: report a non-UTF-8 read clearly under ASSERTIONS - #28066

Open
piandtheradians-glitch wants to merge 2 commits into
emscripten-core:mainfrom
piandtheradians-glitch:single-file-utf8-check
Open

piandtheradians-glitch wants to merge 2 commits into
emscripten-core:mainfrom
piandtheradians-glitch:single-file-utf8-check

Conversation

@piandtheradians-glitch

Copy link
Copy Markdown

With -sSINGLE_FILE and the default SINGLE_FILE_BINARY_ENCODE=1, a .js that is read in an encoding other than UTF-8 (for example from a page with no <meta charset> loaded from file://) mangles the embedded Wasm binary, and fails with an error about the Wasm rather than the encoding, such as CompileError: WebAssembly.instantiate(): unknown type form: 85.

As discussed in #28048, under ASSERTIONS the expected length of the embedded binary is now passed to binaryDecode(), which checks it once before decoding. Read as UTF-8, each byte of the binary is one character of the string, so a different length means the file was read in another encoding. It then aborts with:

The embedded Wasm binary is 16265 characters long, but should be 14895: this file was not read as UTF-8 (SINGLE_FILE_BINARY_ENCODE). Serve it with charset=utf-8, add to the page, or build with -sSINGLE_FILE_BINARY_ENCODE=0

Builds without ASSERTIONS are unchanged (byte-identical output checked for -O0 -sASSERTIONS=0, -O2 and -sMINIMAL_RUNTIME -O2).

Tests:

  • other.test_single_file_not_utf8: re-encodes the output as Latin-1 (what a Windows-1252 read amounts to) and expects the message instead of a CompileError. On main it fails with the CompileError.
  • browser.test_single_file_not_utf8: loads the build from a windows-1252 page. On main it fails with the CompileError; with this change it reports the message (checked in headless Chromium).
  • other.test_single_file_instantiate_failure: updated for the new binaryDecode signature under ASSERTIONS.

Also run locally, with the emsdk version in test/emsdk_version.txt: all 43 other.test_single_file* tests (including the two d8 shell tests, with V8 15.8 from jsvu), all 6 browser.test_single_file* tests, and tools/maint/rebaseline_tests.py --check-only (all 65 code size tests, expectations up to date). Checked by hand with -O2, --closure=1, -sMINIMAL_RUNTIME and -sMODULARIZE -sEXPORT_ES6.

Fixes #28048

Disclosure: researched and written by Claude (Claude Code) at my direction, and reviewed by me. The tests were run as described above.

…RTIONS

With `-sSINGLE_FILE` and the default `SINGLE_FILE_BINARY_ENCODE=1`, a `.js` that is read in an encoding other than UTF-8 (for example from a page with no `<meta charset>` loaded from `file://`) mangles the embedded Wasm binary, and fails with an error about the Wasm rather than the encoding, such as `CompileError: WebAssembly.instantiate(): unknown type form: 85`.

As discussed in emscripten-core#28048, under `ASSERTIONS` the expected length of the embedded binary is now passed to `binaryDecode()`, which checks it once before decoding. Read as UTF-8, each byte of the binary is one character of the string, so a different length means the file was read in another encoding. It then aborts with:

> The embedded Wasm binary is 16265 characters long, but should be 14895: this file was not read as UTF-8 (SINGLE_FILE_BINARY_ENCODE). Serve it with charset=utf-8, add <meta charset="utf-8"> to the page, or build with -sSINGLE_FILE_BINARY_ENCODE=0

Builds without `ASSERTIONS` are unchanged (byte-identical output checked for `-O0 -sASSERTIONS=0`, `-O2` and `-sMINIMAL_RUNTIME -O2`).

Tests:
- `other.test_single_file_not_utf8`: re-encodes the output as Latin-1 (what a Windows-1252 read amounts to) and expects the message instead of a `CompileError`. On `main` it fails with the `CompileError`.
- `browser.test_single_file_not_utf8`: loads the build from a `windows-1252` page. On `main` it fails with the `CompileError`; with this change it reports the message (checked in headless Chromium).
- `other.test_single_file_instantiate_failure`: updated for the new `binaryDecode` signature under `ASSERTIONS`.

Also run locally, with the emsdk version in `test/emsdk_version.txt`: `other.test_single_file*` (all pass except the two `d8` shell tests, as no `d8` here), the browser `test_single_file*` tests, and the code size tests touching `SINGLE_FILE` (`test_codesize_hello_single_file`, `test_minimal_runtime_code_size*`, `test_unoptimized_code_size`, `test_binary_encode_is_smaller_than_base64_encode`). Checked by hand with `-O2`, `--closure=1`, `-sMINIMAL_RUNTIME` and `-sMODULARIZE -sEXPORT_ES6`.

Fixes emscripten-core#28048

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces validation for the UTF-8 encoding of embedded WebAssembly binaries in SINGLE_FILE builds when ASSERTIONS are enabled. It passes the expected binary size to binaryDecode and asserts that the decoded string length matches, adding tests to verify this behavior. Feedback suggests using strict equality (===) instead of loose equality (==) in the assertion to follow JavaScript best practices.

Comment thread src/binaryDecode.js Outdated

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SINGLE_FILE_BINARY_ENCODE: report a non-UTF-8 read clearly instead of a wasm CompileError

1 participant