fix(file): report the bytes actually read on invalid magic - #9300
Open
crossthebridgetpa wants to merge 1 commit into
Open
crossthebridgetpa wants to merge 1 commit into
crossthebridgetpa wants to merge 1 commit into
Conversation
decode_footer compares the trailing 4 bytes of a Lance file against MAGIC, but on mismatch it formats the error with MAGIC (the expected value) instead of the bytes actually read. Every such error therefore printed "invalid magic: [76, 65, 78, 67]", which looks like the check passed and sends readers diagnosing a corrupt file down the wrong path. Field report: bennyhodl/centinel#69 Interpolate the actual magic bytes read from the file, and also show the expected value for context. Adds a unit test that decode_footer surfaces the wrong bytes on a mismatched footer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
A-encoding
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The patch corrects the misleading invalid-magic diagnostic at the narrow footer-validation boundary by reporting the actual bytes alongside the expected magic, with a focused regression test. Successful reads and the on-disk format are unchanged.
lance-gatekeeper
Bot
added
the
K-approved
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.
Bug
decode_footerinrust/lance-file/src/reader.rschecks the trailing 4 bytes of a file againstMAGICand, on mismatch, formats the error withMAGIC(the expected value) instead of the bytes actually read. Every invalid-magic error therefore printsinvalid magic: [76, 65, 78, 67](b"LANC", the correct value), which looks like the check passed and sends readers diagnosing a genuinely corrupt file down the wrong path.Field report: bennyhodl/centinel#69, where a user hit this while diagnosing a corrupt fragment file.
Change
magic_bytes.as_ref()) instead ofMAGIC, and also show the expected value for context:invalid magic: {actual}, expected {expected}.Not included
The file path / object-store location is not threaded through this error, since
decode_footeronly receives the raw footer bytes and its callers (read_raw_metadata_for_dispatch, etc.) would need a signature change to pass it in. Left out to keep this PR minimal; flagging as a possible follow-up if it's wanted.cargo check -p lance-fileand the new test pass locally.rustfmt/cargo fmtwere not available in this environment (no sudo to install); the diff was hand-matched to the surrounding style.🤖 Generated with Claude Code