fix: don't panic on malformed struct field dtypes - #9742
Open
jackylee-ch wants to merge 1 commit into
Open
Conversation
Opening a crafted file whose struct has a field with an undecodable dtype panicked instead of erroring. Field dtypes are decoded lazily, so the top-level dtype segment parsed cleanly and the panic fired later, in StructFields::fields() while reading the file-statistics footer. The accessors that decode a field dtype return a plain DType, and the PartialEq and Hash impls on FieldDType are trait methods, so no signature there can carry a failure. The check therefore belongs where the fields are built from a flatbuffer: try_from_fields decodes each field dtype, and deserialization goes through it. UnionVariants had the identical shape and gets the same check. This costs one decode per field at open time, which is the price of the accessors staying infallible. Signed-off-by: jackylee <qcsd2011@gmail.com>
Contributor
|
these comments are very odd? |
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
2 participants
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.
Opening a crafted file whose struct has a field with an undecodable dtype panicked instead of
erroring. Field dtypes decode lazily, so the top-level dtype segment parsed cleanly and the panic
fired later, in
StructFields::fields()while reading the file-statistics footer.The decoding accessors return a plain
DType, andFieldDType'sPartialEq/Hashare traitmethods — no signature there can carry a failure. So the check goes where the fields are built from a
flatbuffer:
try_from_fieldsdecodes each one, and deserialization now goes through it.UnionVariantshad the identical shape and gets the same check. The cost is one decode per field atopen time, which buys the accessors staying infallible.
StructFields::fields()on malformed file: unwraps a fallible DType decode #8848Tests
Both new tests build a child dtype that passes the flatbuffer verifier but cannot decode; removing
either check makes both fail.
cargo test -p vortex-arraygoes 3442 → 3444, same two pre-existingarrays::listviewfailures.AI assistance
Written with agentic AI assistance; I confirmed the checks fail when removed.