Write explicit null terminator in Utf8StringArray#482
Open
tolmachev-pravo wants to merge 1 commit into
Open
Conversation
Utf8StringArray reserved a byte for each string's null terminator but never wrote it, relying on the buffer already being zero. On .NET the buffer is rented from ArrayPool<byte>.Shared, which is not zero-initialized, so once the pool has been used the terminator byte can hold leftover data. Native node then reads each argv entry past its intended end, e.g. "bad option: --disable-wasm-trap-handler<garbage>". Write the terminator explicitly, and add a regression test that reproduces the failure by dirtying the shared pool before marshaling. Fixes microsoft#481
Author
|
@microsoft-github-policy-service agree |
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
1 participant
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.
Fixes #481.
Utf8StringArrayreserved a byte for each string's null terminator (the+ 1in the encoding loop) but never wrote the\0— it relied on the reserved byte already being zero. That holds on theNETFRAMEWORK/NETSTANDARDpath (new byte[]is zeroed), but on .NET the buffer is rented fromArrayPool<byte>.Shared, which is not zero-initialized. Once the pool has been used, the terminator byte can contain leftover data, and native node reads each argv entry past its end — e.g.bad option: --disable-wasm-trap-handler<garbage>. It only reproduces after warm-up (dirty pool), which made it hard to track down.Fix: write the terminator explicitly.
Adds a regression test that dirties
ArrayPool<byte>.Sharedand verifies the marshaled strings round-trip exactly (fails onmain, passes with this change). Testing the internal struct needsInternalsVisibleToon the test assembly.Verified:
dotnet test --framework net8.0 --filter Utf8StringArrayTestspasses with the fix and fails without it.