This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-go.git
The following commit(s) were added to refs/heads/main by this push:
new 22da919b fix(arrow/array): validate struct child lengths (#1021)
22da919b is described below
commit 22da919b5f507d89c7ca7c643bf00679c58297a7
Author: Minh Vu <[email protected]>
AuthorDate: Wed Aug 5 17:33:55 2026 +0200
fix(arrow/array): validate struct child lengths (#1021)
A struct child shorter than the parent extent can produce an array that
panics when the missing values are accessed. Because structural validation
in
`arrow/array` is opt-in, report this through `Validate`/`ValidateFull`
rather
than changing the `StructBuilder` construction contract.
- Add `Validate`/`ValidateFull` to `Struct`, checking each raw child against
the full `offset + length` extent (offset-aware, so slicing cannot defeat
it), mirroring the sparse-union and fixed-size-list precedents.
- Clamp constructed child views to the available data so malformed arrays
remain inspectable without substituting values from the wrong offset.
- Guard against `offset + length` overflow.
- Preserve the valid child-only builder pattern (e.g. `MapScalar`) and
normal
struct slicing.
---
arrow/array/struct.go | 38 +++++++++++++++++++++++-
arrow/array/struct_test.go | 73 +++++++++++++++++++++++++++++++---------------
2 files changed, 86 insertions(+), 25 deletions(-)
diff --git a/arrow/array/struct.go b/arrow/array/struct.go
index 47e30071..7a3622bb 100644
--- a/arrow/array/struct.go
+++ b/arrow/array/struct.go
@@ -20,6 +20,7 @@ import (
"bytes"
"errors"
"fmt"
+ "math"
"strings"
"github.com/apache/arrow-go/v18/arrow"
@@ -143,6 +144,22 @@ func NewStructData(data arrow.ArrayData) *Struct {
func (a *Struct) NumField() int { return len(a.fields) }
func (a *Struct) Field(i int) arrow.Array { return a.fields[i] }
+func (a *Struct) Validate() error {
+ if a.data.offset < 0 || a.data.length < 0 || int64(a.data.offset) >
math.MaxInt64-int64(a.data.length) {
+ return fmt.Errorf("%w: arrow/array: struct offset and length
overflow", arrow.ErrInvalid)
+ }
+ expectedLength := a.data.offset + a.data.length
+ for i, child := range a.data.childData {
+ if child.Len() < expectedLength {
+ return fmt.Errorf("%w: arrow/array: struct child array
#%d has length smaller than expected for struct array (%d < %d)",
+ arrow.ErrInvalid, i, child.Len(),
expectedLength)
+ }
+ }
+ return nil
+}
+
+func (a *Struct) ValidateFull() error { return a.Validate() }
+
// ValueStr returns the string representation (as json) of the value at index
i.
func (a *Struct) ValueStr(i int) string {
if a.IsNull(i) {
@@ -222,7 +239,26 @@ func (a *Struct) setData(data *Data) {
a.fields = make([]arrow.Array, len(data.childData))
for i, child := range data.childData {
if data.offset != 0 || child.Len() != data.length {
- sub := NewSliceData(child, int64(data.offset),
int64(data.offset+data.length))
+ childLen := int64(child.Len())
+ start := max(int64(data.offset), int64(0))
+ if start > childLen {
+ start = childLen
+ }
+ var end int64
+ offset, length := int64(data.offset), int64(data.length)
+ switch {
+ case length > 0 && offset > math.MaxInt64-length:
+ end = math.MaxInt64
+ case length < 0 && offset < math.MinInt64-length:
+ end = math.MinInt64
+ default:
+ end = offset + length
+ }
+ end = max(end, start)
+ if end > childLen {
+ end = childLen
+ }
+ sub := NewSliceData(child, start, end)
a.fields[i] = MakeFromData(sub)
sub.Release()
} else {
diff --git a/arrow/array/struct_test.go b/arrow/array/struct_test.go
index a6d4f1c7..74fda503 100644
--- a/arrow/array/struct_test.go
+++ b/arrow/array/struct_test.go
@@ -17,6 +17,7 @@
package array_test
import (
+ "math"
"reflect"
"testing"
@@ -135,6 +136,46 @@ func TestStructArray(t *testing.T) {
}
}
+func TestStructValidateFullRejectsShortField(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ dt := arrow.StructOf(
+ arrow.Field{Name: "a", Type: arrow.PrimitiveTypes.Int32},
+ arrow.Field{Name: "b", Type: arrow.PrimitiveTypes.Int32},
+ )
+ b := array.NewStructBuilder(mem, dt)
+ defer b.Release()
+ b.Append(true)
+ b.Append(true)
+ b.FieldBuilder(0).(*array.Int32Builder).Append(1)
+ b.FieldBuilder(1).(*array.Int32Builder).AppendValues([]int32{2, 3}, nil)
+
+ arr := b.NewStructArray()
+ defer arr.Release()
+ assert.ErrorIs(t, arr.Validate(), arrow.ErrInvalid)
+ assert.ErrorIs(t, arr.ValidateFull(), arrow.ErrInvalid)
+ assert.ErrorIs(t, array.ValidateFull(arr), arrow.ErrInvalid)
+
+ sliced := array.NewSlice(arr, 1, 2).(*array.Struct)
+ defer sliced.Release()
+ assert.Zero(t, sliced.Field(0).Len())
+ assert.ErrorIs(t, sliced.Validate(), arrow.ErrInvalid)
+ assert.ErrorIs(t, sliced.ValidateFull(), arrow.ErrInvalid)
+}
+
+func TestStructValidateRejectsOffsetLengthOverflow(t *testing.T) {
+ childData := array.NewData(arrow.PrimitiveTypes.Int32, 0,
[]*memory.Buffer{nil, nil}, nil, 0, 0)
+ defer childData.Release()
+ data := array.NewData(arrow.StructOf(arrow.Field{Name: "value", Type:
arrow.PrimitiveTypes.Int32}),
+ math.MaxInt, nil, []arrow.ArrayData{childData}, 0, 1)
+ defer data.Release()
+
+ arr := array.NewStructData(data)
+ defer arr.Release()
+ require.ErrorIs(t, arr.Validate(), arrow.ErrInvalid)
+}
+
func TestStructStringRoundTrip(t *testing.T) {
// 1. create array
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
@@ -615,18 +656,18 @@ func TestStructArrayUnmarshalJSONMissingFields(t
*testing.T) {
name string
jsonInput string
want string
- panic bool
+ invalid bool
}{
{
name: "missing required field",
jsonInput: `[{"f2": 3, "f3": {"f3_1": "test"}}]`,
- panic: true,
+ invalid: true,
want: "",
},
{
name: "missing optional fields",
jsonInput: `[{"f2": 3, "f3": {"f3_3": "test"}}]`,
- panic: false,
+ invalid: false,
want: `{[(null)] [3] {[(null)] [(null)]
["test"]}}`,
},
}
@@ -634,30 +675,9 @@ func TestStructArrayUnmarshalJSONMissingFields(t
*testing.T) {
for _, tc := range tests {
t.Run(
tc.name, func(t *testing.T) {
-
- var val bool
-
sb := array.NewStructBuilder(pool, dtype)
defer sb.Release()
- if tc.panic {
- defer func() {
- e := recover()
- if e == nil {
- t.Fatalf("this should
have panicked, but did not; slice value %v", val)
- }
- if got, want := e.(string),
"arrow/array: index out of range"; got != want {
- t.Fatalf("invalid
error. got=%q, want=%q", got, want)
- }
- }()
- } else {
- defer func() {
- if e := recover(); e != nil {
- t.Fatalf("unexpected
panic: %v", e)
- }
- }()
- }
-
err := sb.UnmarshalJSON([]byte(tc.jsonInput))
if err != nil {
t.Fatal(err)
@@ -665,6 +685,11 @@ func TestStructArrayUnmarshalJSONMissingFields(t
*testing.T) {
arr := sb.NewArray().(*array.Struct)
defer arr.Release()
+ if tc.invalid {
+ require.ErrorIs(t,
array.ValidateFull(arr), arrow.ErrInvalid)
+ return
+ }
+ require.NoError(t, array.ValidateFull(arr))
got := arr.String()
if got != tc.want {
