Skip to content

Return Serde errors for corrupt RLE and RunEnd metadata - #10852

Merged
robert3005 merged 2 commits into
vortex-data:developfrom
ump45nose:fix/10756-rle-runend-deserialize-panic
Nov 11, 2026
Merged

Return Serde errors for corrupt RLE and RunEnd metadata#10852
robert3005 merged 2 commits into
vortex-data:developfrom
ump45nose:fix/10756-rle-runend-deserialize-panic

Conversation

@ump45nose

@ump45nose ump45nose commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Corrupt or crafted metadata can make a file read panic instead of returning a Serde error. #10760 covers the ALP-RD side of that issue. This PR covers the RLE and RunEnd paths called out there:

  • RLE deserialize called dtype.as_ptype(), which panics when the file dtype is not primitive. It now uses PType::try_from(dtype)?.
  • RunEnd deserialize used vortex_expect on usize::try_from for num_runs and offset, which panics on 32-bit targets when the stored value exceeds usize::MAX. Both conversions now return a Serde error.

Partially addresses #10756.

Tests

  • RLE: regression that deserializes with a non-primitive (utf8) dtype and expects an error (panics on develop).
  • RunEnd: 32-bit-only regressions for num_runs / offset above usize::MAX (cfg(target_pointer_width = "32")). Existing RunEnd deserialize and array tests still pass on this 64-bit host.
  • cargo test -p vortex-fastlanes --lib rle::vtable::tests
  • cargo test -p vortex-array --lib arrays::runend::array::tests

AI assistance

Developed with agentic AI assistance; I reviewed the diff, ran the tests above, and stand behind the change.

@ump45nose
Return Serde errors for corrupt RLE and RunEnd metadata
7bce475 
RLE deserialize called dtype.as_ptype(), which panics for a
non-primitive file dtype. RunEnd deserialize used vortex_expect on
usize::try_from for num_runs and offset, which panics on 32-bit for
values above usize::MAX. Both now return errors so a corrupt file
read fails instead of panicking.

Partially addresses vortex-data#10756 (ALP-RD covered by vortex-data#10760).

Signed-off-by: Vincent <[email protected]>
@connortsui20 connortsui20 mentioned this pull request Oct 30, 2026
Open
20 of 34 tasks
@ump45nose
Merge remote-tracking branch 'upstream/develop' into fix/10756-rle-ru…
620e120 
…nend-deserialize-panic

vortex-data#11285 moved array serde from VTable into ArrayPlugin impls, so carry the
fix over: RLE's plugin deserialize uses PType::try_from(dtype) instead of
dtype.as_ptype(), RunEnd's plugin deserialize returns errors for num_runs /
offset values that do not fit in usize, and the regression tests now call
ArrayPlugin::deserialize with ArrayDeserialization::new.

Signed-off-by: Vincent <[email protected]>
@ump45nose

ump45nose commented Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

Merged current develop (620e120) to clear the conflicts. #11285 moved array serde from VTable into ArrayPlugin impls, so I carried the fix over to the new plugin.rs files:

  • RLE's deserialize uses PType::try_from(dtype)? instead of dtype.as_ptype(), so a non-primitive dtype returns an error instead of panicking.
  • RunEnd's deserialize returns an error when num_runs or offset does not fit in usize, instead of calling vortex_expect.
  • The regression tests now call ArrayPlugin::deserialize with ArrayDeserialization::new, like the existing unknown-ptype tests.

Locally, cargo test -p vortex-array --lib runend (159 passed) and cargo test -p vortex-fastlanes --lib rle (82 passed) both pass. cargo clippy -p vortex-array -p vortex-fastlanes --all-targets -- -D warnings is clean, and cargo +nightly-2026-09-10 fmt --all --check is clean for the touched files.

(AI-assisted reply.)

@robert3005 robert3005 added the changelog/fix A bug fix label Nov 11, 2026
@robert3005
robert3005 enabled auto-merge (squash) November 11, 2026 10:36
@codspeed

codspeed Bot commented Nov 11, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 10.36%

2 warnings

⚠️ Walltime results may be unreliable

Some benchmarks used the Walltime instrument outside CodSpeed Macro Runners, which can make the results inconsistent.

Learn more about Macro Runners

See affected benchmarks

⚠️ 1 benchmark is dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚡ 1 improved benchmark
✅ 1915 untouched benchmarks
⏩ 48 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ Walltime decode_chunked_neon[i64, 524288] 14.7 µs 13.3 µs +10.36%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.

Comparing ump45nose:fix/10756-rle-runend-deserialize-panic (620e120) with develop (49acec0)

Open in CodSpeed

Footnotes

  1. 48 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@robert3005
robert3005 merged commit d455777 into vortex-data:develop Nov 11, 2026
99 checks passed
@robert3005 robert3005 mentioned this pull request Nov 11, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robert3005 robert3005 robert3005 approved these changes

Assignees

No one assigned

Labels

changelog/fix A bug fix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ump45nose @robert3005