Repository navigation
SINGLE_FILE_BINARY_ENCODE: report a non-UTF-8 read clearly under ASSERTIONS - #28066
Open
piandtheradians-glitch wants to merge 2 commits into
Open
piandtheradians-glitch wants to merge 2 commits into
piandtheradians-glitch wants to merge 2 commits into
Conversation
…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
There was a problem hiding this comment.
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.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With
-sSINGLE_FILEand the defaultSINGLE_FILE_BINARY_ENCODE=1, a.jsthat is read in an encoding other than UTF-8 (for example from a page with no<meta charset>loaded fromfile://) mangles the embedded Wasm binary, and fails with an error about the Wasm rather than the encoding, such asCompileError: WebAssembly.instantiate(): unknown type form: 85.As discussed in #28048, under
ASSERTIONSthe expected length of the embedded binary is now passed tobinaryDecode(), 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:Builds without
ASSERTIONSare unchanged (byte-identical output checked for-O0 -sASSERTIONS=0,-O2and-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 aCompileError. Onmainit fails with theCompileError.browser.test_single_file_not_utf8: loads the build from awindows-1252page. Onmainit fails with theCompileError; with this change it reports the message (checked in headless Chromium).other.test_single_file_instantiate_failure: updated for the newbinaryDecodesignature underASSERTIONS.Also run locally, with the emsdk version in
test/emsdk_version.txt: all 43other.test_single_file*tests (including the twod8shell tests, with V8 15.8 from jsvu), all 6browser.test_single_file*tests, andtools/maint/rebaseline_tests.py --check-only(all 65 code size tests, expectations up to date). Checked by hand with-O2,--closure=1,-sMINIMAL_RUNTIMEand-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.