This is an automated email from the ASF dual-hosted git repository.
laskoviymishka pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git
The following commit(s) were added to refs/heads/main by this push:
new 556b52c3f fix(dv): reject trailing bitmap data (#1625)
556b52c3f is described below
commit 556b52c3f15be9598d2c5e6185fe63e9d56068a3
Author: Minh Vu <[email protected]>
AuthorDate: Tue Aug 4 12:19:47 2026 +0200
fix(dv): reject trailing bitmap data (#1625)
## Summary
Reject trailing bytes after a portable roaring position bitmap.
## Problem
The portable format starts with the number of keyed bitmaps, so that
count frames the complete payload. The decoder reads exactly that many
entries but does not verify that the input is exhausted afterward. As a
result, an otherwise valid bitmap with arbitrary trailing bytes is
accepted and the extra data is silently ignored.
For deletion vectors this weakens corruption detection because malformed
inner bitmap data can pass decoding even when the outer DV length and
CRC are valid.
## Fix
After decoding the declared bitmap entries, require the input reader to
have no bytes remaining.
## Tests
Added framing coverage for empty and multi-key non-empty bitmaps. An
end-to-end DV envelope test also appends bytes to a valid bitmap,
rebuilds the matching length and CRC, and verifies the contextual decode
error.
```
go test ./table/dv -count=1
```
---
table/dv/deletion_vector_test.go | 17 +++++++++++++++++
table/dv/roaring_bitmap.go | 3 +++
table/dv/roaring_bitmap_test.go | 27 +++++++++++++++++++++++++++
3 files changed, 47 insertions(+)
diff --git a/table/dv/deletion_vector_test.go b/table/dv/deletion_vector_test.go
index 945447fb4..8c1084976 100644
--- a/table/dv/deletion_vector_test.go
+++ b/table/dv/deletion_vector_test.go
@@ -331,6 +331,23 @@ func TestDeserializeDVWrapsBitmapDecodeError(t *testing.T)
{
assert.ErrorContains(t, err, "deserialize deletion vector bitmap")
}
+// Why: a valid DV length and CRC must not hide extra bytes after the framed
+// roaring bitmap.
+// Condition: append bytes to a valid inner bitmap, then rebuild the envelope
+// so its length and CRC both match the malformed payload.
+// Assertion: returns a contextual bitmap error containing "trailing data".
+func TestDeserializeDVRejectsTrailingBitmapData(t *testing.T) {
+ bm := NewRoaringPositionBitmap()
+ bm.Set(7)
+ var bitmap bytes.Buffer
+ require.NoError(t, bm.Serialize(&bitmap))
+ bitmap.Write([]byte{0xde, 0xad})
+
+ _, err := DeserializeDV(wrapDVPayloadForTest(bitmap.Bytes()), -1)
+ assert.ErrorContains(t, err, "deserialize deletion vector bitmap")
+ assert.ErrorContains(t, err, "trailing data")
+}
+
// Why: manifest metadata and DV content should agree on the number of deleted
rows.
// Condition: valid DV blob with 5 positions, but expected cardinality is set
to 999.
// Assertion: returns an error containing "cardinality mismatch".
diff --git a/table/dv/roaring_bitmap.go b/table/dv/roaring_bitmap.go
index e606773a9..20cbfc818 100644
--- a/table/dv/roaring_bitmap.go
+++ b/table/dv/roaring_bitmap.go
@@ -179,6 +179,9 @@ func DeserializeRoaringPositionBitmap(data []byte)
(*RoaringPositionBitmap, erro
lastKey = key
hasLastKey = true
}
+ if r.Len() != 0 {
+ return nil, fmt.Errorf("trailing data after bitmaps: %d bytes",
r.Len())
+ }
return b, nil
}
diff --git a/table/dv/roaring_bitmap_test.go b/table/dv/roaring_bitmap_test.go
index bccc7e9c7..c5e93ba3e 100644
--- a/table/dv/roaring_bitmap_test.go
+++ b/table/dv/roaring_bitmap_test.go
@@ -148,6 +148,33 @@ func TestDeserializeRoaringBitmapTruncatedAfterKey(t
*testing.T) {
assert.ErrorContains(t, err, "read bitmap for key 0")
}
+// Why: the outer count fully frames the portable bitmap and extra bytes are
+// malformed rather than a forward-compatible extension.
+// Condition: append bytes after otherwise valid empty and non-empty bitmaps.
+// Assertion: returns an error containing "trailing data".
+func TestDeserializeRoaringBitmapTrailingData(t *testing.T) {
+ for _, tt := range []struct {
+ name string
+ positions []uint64
+ }{
+ {name: "empty"},
+ {name: "non-empty", positions: []uint64{1, (uint64(3) << 32) |
7}},
+ } {
+ t.Run(tt.name, func(t *testing.T) {
+ bm := NewRoaringPositionBitmap()
+ for _, position := range tt.positions {
+ bm.Set(position)
+ }
+ var buf bytes.Buffer
+ require.NoError(t, bm.Serialize(&buf))
+ buf.Write([]byte{0xde, 0xad})
+
+ _, err := DeserializeRoaringPositionBitmap(buf.Bytes())
+ assert.ErrorContains(t, err, "trailing data")
+ })
+ }
+}
+
// TestKeepMaskBytes pins KeepMaskBytes' contract: a bit-packed []byte where
// bit i is 1 iff position i is NOT in the bitmap, sized to ⌈length/8⌉ bytes,
// matching Arrow Boolean buffer layout so it can be wrapped without
re-packing.
