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 7cf023352 fix(literals): reject oversized fixed-width values (#1585)
7cf023352 is described below
commit 7cf02335229fc54636f3e2a7ba6b86563f19593f
Author: Minh Vu <[email protected]>
AuthorDate: Fri Jul 31 16:20:33 2026 +0200
fix(literals): reject oversized fixed-width values (#1585)
## What changed
Continue padding shortened fixed-width bounds for compatibility, but
reject inputs larger than the declared fixed width with
`ErrInvalidBinSerialization`.
## Why
The previous `len != width` branch copied every mismatched value into a
width-sized buffer. Oversized values were silently truncated into a
different valid value, which could corrupt bounds used for pruning.
Regression coverage includes exact, shortened, empty, one-byte
oversized, and substantially oversized inputs, with expected and actual
lengths asserted for oversize errors.
## Testing
- `go test .`
- `go vet .`
---------
Signed-off-by: Minh Vu <[email protected]>
---
errors.go | 1 +
literals.go | 5 ++-
literals_test.go | 37 ++++++++++++++++
table/evaluators.go | 29 +++++++++++--
table/evaluators_invalid_bounds_test.go | 77 +++++++++++++++++++++++++++++++++
5 files changed, 144 insertions(+), 5 deletions(-)
diff --git a/errors.go b/errors.go
index 45e03770e..1d031c6fe 100644
--- a/errors.go
+++ b/errors.go
@@ -34,6 +34,7 @@ var (
ErrBadCast = errors.New("could not cast value")
ErrBadLiteral = errors.New("invalid literal value")
ErrInvalidBinSerialization = errors.New("invalid binary serialization")
+ ErrInvalidFixedLength = errors.New("invalid fixed literal length")
ErrResolve = errors.New("cannot resolve type")
// ErrBBoxNotSerializable is returned when marshaling a geospatial bbox
// predicate to REST expression JSON: bbox predicates exist only for
local
diff --git a/literals.go b/literals.go
index 5a0a068de..4ee361425 100644
--- a/literals.go
+++ b/literals.go
@@ -193,13 +193,16 @@ func LiteralFromBytes(typ Type, data []byte) (Literal,
error) {
return GeoLiteral{val: data, typ: typ}, nil
case FixedType:
- if len(data) != t.Len() {
+ if len(data) < t.Len() {
// looks like some writers will write a prefix of the
fixed length
// for lower/upper bounds instead of the full length.
so let's pad
// it out to the full length if unpacking a fixed
length literal
padded := make([]byte, t.Len())
copy(padded, data)
data = padded
+ } else if len(data) > t.Len() {
+ return nil, fmt.Errorf("%w: %w: fixed[%d] value has %d
bytes",
+ ErrInvalidBinSerialization,
ErrInvalidFixedLength, t.Len(), len(data))
}
var v FixedLiteral
err := v.UnmarshalBinary(data)
diff --git a/literals_test.go b/literals_test.go
index e0030092f..acccd31ab 100644
--- a/literals_test.go
+++ b/literals_test.go
@@ -1231,6 +1231,43 @@ func TestUnmarshalBinary(t *testing.T) {
}
}
+func TestLiteralFromBytesFixedWidth(t *testing.T) {
+ t.Parallel()
+
+ tests := []struct {
+ name string
+ data []byte
+ want iceberg.FixedLiteral
+ errContains string
+ oversized bool
+ }{
+ {name: "exact", data: []byte{1, 2, 3, 4}, want:
iceberg.FixedLiteral{1, 2, 3, 4}},
+ {name: "short", data: []byte{1, 2, 3}, want:
iceberg.FixedLiteral{1, 2, 3, 0}},
+ {name: "zero length", data: []byte{}, want:
iceberg.FixedLiteral{0, 0, 0, 0}},
+ {name: "nil", data: nil, errContains: "invalid binary
serialization"},
+ {name: "one byte oversized", data: []byte{1, 2, 3, 4, 5},
errContains: "fixed[4] value has 5 bytes", oversized: true},
+ {name: "large oversized", data: make([]byte, 100), errContains:
"fixed[4] value has 100 bytes", oversized: true},
+ }
+
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ literal, err :=
iceberg.LiteralFromBytes(iceberg.FixedTypeOf(4), test.data)
+ if test.errContains != "" {
+ require.ErrorIs(t, err,
iceberg.ErrInvalidBinSerialization)
+ if test.oversized {
+ require.ErrorIs(t, err,
iceberg.ErrInvalidFixedLength)
+ }
+ require.ErrorContains(t, err, test.errContains)
+
+ return
+ }
+
+ require.NoError(t, err)
+ assert.Equal(t, test.want, literal)
+ })
+ }
+}
+
func TestRoundTripLiteralBinary(t *testing.T) {
tests := []struct {
typ iceberg.Type
diff --git a/table/evaluators.go b/table/evaluators.go
index 03a13dbb8..da34bae67 100644
--- a/table/evaluators.go
+++ b/table/evaluators.go
@@ -19,6 +19,7 @@ package table
import (
"encoding"
+ "errors"
"fmt"
"math"
"slices"
@@ -74,7 +75,12 @@ func (m *manifestEvalVisitor) Eval(manifest
iceberg.ManifestFile) (bool, error)
partitionFilter: m.partitionFilter,
}
- return iceberg.VisitExpr(ev.partitionFilter, &ev)
+ result, err := iceberg.VisitExpr(ev.partitionFilter, &ev)
+ if errors.Is(err, iceberg.ErrInvalidFixedLength) {
+ return rowsMightMatch, nil
+ }
+
+ return result, err
}
func removeBoundCmp[T iceberg.LiteralType](bound iceberg.Literal, vals
[]iceberg.Literal, cmpToDelete int) []iceberg.Literal {
@@ -785,7 +791,12 @@ func (m *inclusiveMetricsEval) TestRowGroup(rgmeta
*metadata.RowGroupMetaData, c
}
}
- return iceberg.VisitExpr(m.expr, m)
+ result, err := iceberg.VisitExpr(m.expr, m)
+ if errors.Is(err, iceberg.ErrInvalidFixedLength) {
+ return rowsMightMatch, nil
+ }
+
+ return result, err
}
func (m *inclusiveMetricsEval) Eval(file iceberg.DataFile) (bool, error) {
@@ -803,7 +814,12 @@ func (m *inclusiveMetricsEval) Eval(file iceberg.DataFile)
(bool, error) {
ev.nanCounts = file.NaNValueCounts()
ev.lowerBounds, ev.upperBounds = file.LowerBoundValues(),
file.UpperBoundValues()
- return iceberg.VisitExpr(m.expr, &ev)
+ result, err := iceberg.VisitExpr(m.expr, &ev)
+ if errors.Is(err, iceberg.ErrInvalidFixedLength) {
+ return rowsMightMatch, nil
+ }
+
+ return result, err
}
func (m *inclusiveMetricsEval) mayContainNull(fieldID int) bool {
@@ -1311,7 +1327,12 @@ func (m *strictMetricsEval) Eval(file iceberg.DataFile)
(bool, error) {
ev.nanCounts = file.NaNValueCounts()
ev.lowerBounds, ev.upperBounds = file.LowerBoundValues(),
file.UpperBoundValues()
- return iceberg.VisitExpr(m.expr, &ev)
+ result, err := iceberg.VisitExpr(m.expr, &ev)
+ if errors.Is(err, iceberg.ErrInvalidFixedLength) {
+ return rowsMightNotMatch, nil
+ }
+
+ return result, err
}
func (m *strictMetricsEval) VisitUnbound(iceberg.UnboundPredicate) bool {
diff --git a/table/evaluators_invalid_bounds_test.go
b/table/evaluators_invalid_bounds_test.go
new file mode 100644
index 000000000..bcd121dd6
--- /dev/null
+++ b/table/evaluators_invalid_bounds_test.go
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package table
+
+import (
+ "testing"
+
+ "github.com/apache/iceberg-go"
+ "github.com/stretchr/testify/require"
+)
+
+func TestMalformedFixedBoundsAreConservative(t *testing.T) {
+ schema := iceberg.NewSchema(1, iceberg.NestedField{
+ ID: 1, Name: "value", Type: iceberg.FixedTypeOf(4),
+ })
+ expr := iceberg.EqualTo(iceberg.Reference("value"), []byte{1, 2, 3, 4})
+ malformed := []byte{1, 2, 3, 4, 5}
+
+ t.Run("manifest evaluator keeps the manifest", func(t *testing.T) {
+ spec := iceberg.NewPartitionSpec(iceberg.PartitionField{
+ SourceIDs: []int{1}, FieldID: 1000, Name: "value",
Transform: iceberg.IdentityTransform{},
+ })
+ eval, err := newManifestEvaluator(spec, schema, expr, true)
+ require.NoError(t, err)
+
+ manifest := iceberg.NewManifestFile(2, "manifest.avro", 1, 0,
1).
+ AddedFiles(1).
+ Partitions([]iceberg.FieldSummary{{LowerBound:
&malformed, UpperBound: &malformed}}).
+ Build()
+ matches, err := eval(manifest)
+ require.NoError(t, err)
+ require.True(t, matches)
+ })
+
+ builder, err := iceberg.NewDataFileBuilder(
+ iceberg.NewPartitionSpec(), iceberg.EntryContentData,
"file.parquet",
+ iceberg.ParquetFile, nil, nil, nil, 1, 1,
+ )
+ require.NoError(t, err)
+ dataFile := builder.
+ ValueCounts(map[int]int64{1: 1}).
+ NullValueCounts(map[int]int64{1: 0}).
+ LowerBoundValues(map[int][]byte{1: malformed}).
+ UpperBoundValues(map[int][]byte{1: malformed}).
+ Build()
+
+ t.Run("inclusive evaluator keeps the file", func(t *testing.T) {
+ eval, err := newInclusiveMetricsEvaluator(schema, expr, true,
false)
+ require.NoError(t, err)
+ matches, err := eval(dataFile)
+ require.NoError(t, err)
+ require.True(t, matches)
+ })
+
+ t.Run("strict evaluator does not prove a match", func(t *testing.T) {
+ eval, err := newStrictMetricsEvaluator(schema, expr, true,
false)
+ require.NoError(t, err)
+ matches, err := eval(dataFile)
+ require.NoError(t, err)
+ require.False(t, matches)
+ })
+}
