fix(v8): accept UTF-8 labels and decode UTF-16LE - #6038
Closed
thisisanubhav wants to merge 1 commit into
Closed
thisisanubhav wants to merge 1 commit into
thisisanubhav wants to merge 1 commit into
Conversation
Contributor
|
I'm going to close this in favor of #6045, though I might nab the test file if that's okay. |
Contributor
|
@thisisanubhav would you please sign the CLA? |
Author
|
@coolreader18 CLA signed |
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.
Description of Changes
Fixes #5994.
The V8 host previously rejected
new TextDecoder('utf8')andnew TextDecoder('utf-16le'). Both are constructed eagerly by the h3-js@4.5.0 browser bundle, so the dependency could not finish module evaluation even though its UTF-16 decoder is unused.Compatibility: this remains a subset of TextDecoder. UTF-16BE and other encodings, streaming, and
ignoreBOM: trueremain unsupported. The existing native UTF-8 decode path (including its existing BOM behavior) is unchanged. UTF-16LE is implemented in JavaScript, without a new native API or dependency. The separate CLI success-exit behavior after module-evaluation errors is outside this fix.API and ABI breaking changes
None. Previously rejected labels now work; the public host ABI is unchanged.
Rollback safety impact
n/a
Expected complexity level and risk
Testing
RUSTUP_TOOLCHAIN=stable rustfmt --check --edition 2024 crates/core/src/host/v8/mod.rs, andgit diff --check.vm.SourceTextModuleharness (node --experimental-vm-modules /tmp/issue-5994-check.mjs). Only the unchanged native UTF-8 primitives were shimmed; UTF-16LE executed the production implementation.latLngToCell(37.775938728915946, -122.41795063018799, 9)returned8928308280fffff;gridDisk(cell, 1)returned seven cells. Master reproduced theutf8failure and, after changing only that label, theutf-16lefailure.iai-callgrindGit dependency was not cached. Disk was critically low (256–416 MiB free; ENOSPC constraint), so no further dependencies were downloaded or large builds attempted. Formatting used the installed stable toolchain (Rust 1.97.1), rather than the pinned 1.96.1 toolchain.cargo test --locked -p spacetimedb-core text_decoder_dependency_construction_and_decoding. No standalone publish/build flow was run locally.