feat: Accept Android assembly store v4 versions - #5574
jamescrosswell wants to merge 5 commits into
Conversation
…eader v4 stores (CoreCLR, .NET 11) add a content_id field to the header, so the index starts 8 bytes later. Header.NativeSize is now derived from the format number rather than being a constant. Refs #5454 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## version7 #5574 +/- ##
===========================================
Coverage ? 74.74%
===========================================
Files ? 515
Lines ? 18896
Branches ? 3686
===========================================
Hits ? 14123
Misses ? 3893
Partials ? 880 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| } | ||
|
|
||
| // Index | ||
| if (is64Bit) |
There was a problem hiding this comment.
Bug: The test helper CreateStore unconditionally writes an ignore byte, but StoreReader.Prepare() only reads it on .NET 10.0_OR_GREATER, causing a stream misalignment on older frameworks.
Severity: LOW
Suggested Fix
The test helper CreateStore should be updated to conditionally write the ignore byte, matching the logic in the production StoreReader.Prepare() method. This can be achieved by using the same #if NET10_0_OR_GREATER preprocessor directive in the test code to ensure the test data format is consistent with what the reader expects on all target frameworks.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: test/Sentry.Android.AssemblyReader.Tests/StoreReaderTests.cs#L60
Potential issue: The test helper method `CreateStore` unconditionally writes an `ignore`
byte when creating a test store. However, the `StoreReader.Prepare()` method, which
reads this store, only consumes this byte when compiled for `.NET 10.0_OR_GREATER` due
to a preprocessor directive. On older target frameworks, this byte is not read, causing
the reader's stream position to be misaligned by one byte. This misalignment will lead
to incorrect data being read for subsequent assembly descriptors, causing tests to fail
or produce garbage data on pre-.NET 10.0 targets. This is a test-only issue but prevents
correct validation across all supported frameworks.
Also affects:
src/Sentry.Android.AssemblyReader/V2/StoreReader.cs:163~167
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
We only target .NET 10 and later for Android.
First step of porting the upstream
dotnet/androidAssemblyStore reader changes (up tof1aecf9) so we can read .NET 11 (CoreCLR) v4 assembly stores.Part of #5454
Summary
StoreReadernow accepts0x_0000004store versions for all four ABIs.content_idto the header, so it's read when the format number is >= 4 andHeader.NativeSizeis derived from the version rather than being a constant. Accepting the version alone isn't enough - the index would be read 8 bytes early andPrepare()would fail.StoreReaderTestsbuild minimal in-memory v3/v4 stores (32- and 64-bit) and check they parse, and that v2/v5 are rejected.Notes
AndroidAssemblyReaderTestsstay skipped onnet11.0: real .NET 11 APKs also need_assembly_storeELF dynamic-symbol payload discovery, which is the next step in Port upstream AssemblyStore reader changes: v4/CoreCLR format, _assembly_store symbol, index-entry sizing (next major) #5454. Those skips come off with that change.🤖 Generated with Claude Code