Skip to content

fix(dv): validate Puffin blob metadata against manifest - #1615

Open
fallintoplace wants to merge 2 commits into
apache:mainfrom
fallintoplace:fix/dv-validate-blob-identity
Open

fix(dv): validate Puffin blob metadata against manifest#1615
fallintoplace wants to merge 2 commits into
apache:mainfrom
fallintoplace:fix/dv-validate-blob-identity

Conversation

@fallintoplace

@fallintoplace fallintoplace commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Problem

ReadDV finds a Puffin blob using the manifest entry's content_offset and content_size_in_bytes, then validates its cardinality. It does not verify that the selected footer entry is a deletion-vector-v1 blob for the manifest's referenced_data_file.

That leaves a gap when one Puffin file contains deletion vectors for multiple data files. A malformed manifest entry can point to another blob with the same cardinality, pass the existing checks, and return row positions belonging to a different data file.

Fix

Before reading or decoding the blob payload, ReadDV now:

  • requires the manifest entry to have a non-empty referenced_data_file
  • verifies that the footer entry selected by offset and length has type deletion-vector-v1
  • requires its referenced-data-file property to match the manifest value

The existing cardinality checks and compatibility behavior for a missing Puffin cardinality property are unchanged.

Tests

Added coverage for mismatched and missing data-file references, a non-DV blob at the selected range, and both nil and empty manifest references.

  • go test ./table/dv
  • go test ./...
  • golangci-lint run --timeout=10m

Signed-off-by: Minh Vu <vuhoangminh97@gmail.com>
@fallintoplace
fallintoplace requested a review from zeroshade as a code owner July 30, 2026 19:44
Signed-off-by: Minh Vu <vuhoangminh97@gmail.com>
@fallintoplace fallintoplace changed the title fix(dv): validate Puffin blob identity fix(dv): validate Puffin blob metadata against manifest Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant