Repository: hamba/avro
Pull request #508 (2025-03-10 18:00:46)

fix: empty and nil slice handling
## Goal of this PR

Fix handling of empty and nil slices.  There are scenarios where empty slices get turned into slices of 1 element, and where nil slices cause a panic.

This PR fixes these scenarios.

## How did I test it?

Added unit tests that demonstrate various scenarios, some of which fail before the fix and pass with the fix.
Notably:

* `TestDecoder_UnionInterfaceArrayEmpty`
  * empty array of ints in union decoded as a slice of int with 1 element instead of empty slice
* `TestDecoder_UnionArrayRecordEmpty`
  * empty array of records in union decoded to non-nil pointer to nil slice as nil slice instead of empty slice
* `TestDecoder_UnionInterfaceArrayRecordEmpty`
  * empty array of records in union decoded as a slice of records with 1 element instead of empty slice
* `TestEncoder_UnionInterfaceUnregisteredArrayNull`
  * encoding a nil array in `map[string]any` panicked instead of encoding an empty slice



Patch:
diff --git a/codec_array.go b/codec_array.go
index 0b412d9..78b0201 100644
--- a/codec_array.go
+++ b/codec_array.go
@@ -42,6 +42,10 @@ func (d *arrayDecoder) Decode(ptr unsafe.Pointer, r *Reader) {
 	var size int
 	sliceType := d.typ
 
+	if sliceType.UnsafeIsNil(ptr) {
+		sliceType.UnsafeSet(ptr, sliceType.UnsafeMakeSlice(0, 0))
+	}
+
 	for {
 		l, _ := r.ReadBlockHeader()
 		if l == 0 {
diff --git a/codec_union.go b/codec_union.go
index 7d80b53..30db5c0 100644
--- a/codec_union.go
+++ b/codec_union.go
@@ -155,6 +155,12 @@ func (e *mapUnionEncoder) Encode(ptr unsafe.Pointer, w *Writer) {
 		return
 	}
 
+	// encode a nil slice as an empty array
+	if schema.Type() == Array && val == nil {
+		// element data type doesn't matter since it skips iterating the slice
+		val = []struct{}{}
+	}
+
 	elemType := reflect2.TypeOf(val)
 	elemPtr := reflect2.PtrOf(val)
 
@@ -366,11 +372,11 @@ func (d *unionResolvedDecoder) Decode(ptr unsafe.Pointer, r *Reader) {
 	switch typ.Kind() {
 	case reflect.Map:
 		mapType := typ.(*reflect2.UnsafeMapType)
-		newPtr = mapType.UnsafeMakeMap(1)
+		newPtr = mapType.UnsafeMakeMap(0)
 
 	case reflect.Slice:
 		mapType := typ.(*reflect2.UnsafeSliceType)
-		newPtr = mapType.UnsafeMakeSlice(1, 1)
+		newPtr = mapType.UnsafeMakeSlice(0, 0)
 
 	case reflect.Ptr:
 		elemType := typ.(*reflect2.UnsafePtrType).Elem()


Test patch:
diff --git a/decoder_array_test.go b/decoder_array_test.go
index 2cb81a1..2b2d2fe 100644
--- a/decoder_array_test.go
+++ b/decoder_array_test.go
@@ -96,7 +96,7 @@ func TestDecoder_ArrayRecursiveStruct(t *testing.T) {
 	err := dec.Decode(&got)
 
 	assert.NoError(t, err)
-	assert.Equal(t, record{A: 1, B: []record{{A: 2}, {A: 3}}}, got)
+	assert.Equal(t, record{A: 1, B: []record{{A: 2, B: []record{}}, {A: 3, B: []record{}}}}, got)
 }
 
 func TestDecoder_ArraySliceError(t *testing.T) {
diff --git a/decoder_union_test.go b/decoder_union_test.go
index 4af546b..cbf87ae 100644
--- a/decoder_union_test.go
+++ b/decoder_union_test.go
@@ -439,6 +439,22 @@ func TestDecoder_UnionInterfaceArray(t *testing.T) {
 	assert.Equal(t, []int{27}, got)
 }
 
+func TestDecoder_UnionInterfaceArrayEmpty(t *testing.T) {
+	defer ConfigTeardown()
+
+	avro.Register("array:int", []int{})
+
+	data := []byte{0x02, 0x00}
+	schema := `["int", {"type": "array", "items": "int"}]`
+	dec, _ := avro.NewDecoder(schema, bytes.NewReader(data))
+
+	var got any
+	err := dec.Decode(&got)
+
+	require.NoError(t, err)
+	assert.Equal(t, []int{}, got)
+}
+
 func TestDecoder_UnionInterfaceArrayNamed(t *testing.T) {
 	defer ConfigTeardown()
 
@@ -523,6 +539,78 @@ func TestDecoder_UnionInterfaceRecordNotReused(t *testing.T) {
 	assert.Equal(t, "foo", rec.B)
 }
 
+func TestDecoder_UnionArrayRecord(t *testing.T) {
+	defer ConfigTeardown()
+
+	data := []byte{0x02, 0x02, 0x36, 0x06, 0x66, 0x6F, 0x6F, 0x00}
+	schema := `["null", {"type": "array", "items": {"type": "record", "name": "test", "fields": [{"name": "a", "type": "long"}, {"name": "b", "type": "string"}]}}]`
+	dec, _ := avro.NewDecoder(schema, bytes.NewReader(data))
+
+	var record []TestRecord
+	got := &record
+	err := dec.Decode(&got)
+
+	require.NoError(t, err)
+	assert.Equal(t, int64(27), (*got)[0].A)
+	assert.Equal(t, "foo", (*got)[0].B)
+}
+
+func TestDecoder_UnionArrayRecordEmpty(t *testing.T) {
+	defer ConfigTeardown()
+
+	data := []byte{0x02, 0x00}
+	schema := `["null", {"type": "array", "items": {"type": "record", "name": "test", "fields": [{"name": "a", "type": "long"}, {"name": "b", "type": "string"}]}}]`
+	dec, _ := avro.NewDecoder(schema, bytes.NewReader(data))
+
+	// decode to non-nil pointer to nil slice
+	var record []TestRecord
+	got := &record
+	assert.NotNil(t, got)
+	assert.Nil(t, *got)
+
+	err := dec.Decode(&got)
+
+	require.NoError(t, err)
+	assert.NotNil(t, *got)
+	assert.Equal(t, &[]TestRecord{}, got)
+}
+
+func TestDecoder_UnionInterfaceArrayRecord(t *testing.T) {
+	defer ConfigTeardown()
+
+	avro.Register("array:test", []TestRecord{})
+
+	data := []byte{0x02, 0x02, 0x36, 0x06, 0x66, 0x6F, 0x6F, 0x00}
+	schema := `["null", {"type": "array", "items": {"type": "record", "name": "test", "fields": [{"name": "a", "type": "long"}, {"name": "b", "type": "string"}]}}]`
+	dec, _ := avro.NewDecoder(schema, bytes.NewReader(data))
+
+	var got any
+	err := dec.Decode(&got)
+
+	require.NoError(t, err)
+	assert.IsType(t, []TestRecord{}, got)
+	assert.Len(t, got, 1)
+	assert.Equal(t, int64(27), got.([]TestRecord)[0].A)
+	assert.Equal(t, "foo", got.([]TestRecord)[0].B)
+}
+
+func TestDecoder_UnionInterfaceArrayRecordEmpty(t *testing.T) {
+	defer ConfigTeardown()
+
+	avro.Register("array:test", []TestRecord{})
+
+	data := []byte{0x02, 0x00}
+	schema := `["null", {"type": "array", "items": {"type": "record", "name": "test", "fields": [{"name": "a", "type": "long"}, {"name": "b", "type": "string"}]}}]`
+	dec, _ := avro.NewDecoder(schema, bytes.NewReader(data))
+
+	var got any
+	err := dec.Decode(&got)
+
+	require.NoError(t, err)
+	assert.IsType(t, []TestRecord{}, got)
+	assert.Equal(t, []TestRecord{}, got)
+}
+
 func TestDecoder_UnionInterfaceUnresolvableType(t *testing.T) {
 	defer ConfigTeardown()
 
@@ -549,7 +637,7 @@ func TestDecoder_UnionInterfaceDualRecords(t *testing.T) {
 
 	data := []byte{0x02, 0x36, 0x06, 0x66, 0x6F, 0x6F}
 	schema := `[
-		"int", 
+		"int",
 		{"type": "record", "name": "test", "fields" : [{"name": "a", "type": "long"}, {"name": "b", "type": "string"}]},
 		{"type": "record", "name": "otherTest", "fields" : [{"name": "a", "type": "long"}, {"name": "b", "type": "string"}]}
 	]`
@@ -572,7 +660,7 @@ func TestDecoder_UnionInterfaceDualRecordsUnresolvableType(t *testing.T) {
 
 	data := []byte{0x02, 0x36, 0x06, 0x66, 0x6F, 0x6F}
 	schema := `[
-		"int", 
+		"int",
 		{"type": "record", "name": "test", "fields" : [{"name": "a", "type": "long"}, {"name": "b", "type": "string"}]},
 		{"type": "record", "name": "otherTest", "fields" : [{"name": "a", "type": "long"}, {"name": "b", "type": "string"}]}
 	]`
@@ -598,7 +686,7 @@ func TestDecoder_UnionInterfaceDualRecordsPartialResolution(t *testing.T) {
 
 	data := []byte{0x02, 0x36, 0x06, 0x66, 0x6F, 0x6F}
 	schema := `[
-		"int", 
+		"int",
 		{"type": "record", "name": "test", "fields" : [{"name": "a", "type": "long"}, {"name": "b", "type": "string"}]},
 		{"type": "record", "name": "otherTest", "fields" : [{"name": "a", "type": "long"}, {"name": "b", "type": "string"}]}
 	]`
diff --git a/encoder_union_test.go b/encoder_union_test.go
index 72c9d6e..6b45f77 100644
--- a/encoder_union_test.go
+++ b/encoder_union_test.go
@@ -383,6 +383,74 @@ func TestEncoder_UnionInterfaceArray(t *testing.T) {
 	assert.Equal(t, []byte{0x02, 0x01, 0x02, 0x36, 0x00}, buf.Bytes())
 }
 
+func TestEncoder_UnionInterfaceArrayEmpty(t *testing.T) {
+	defer ConfigTeardown()
+
+	avro.Register("array:int", []int{})
+
+	schema := `["int", {"type": "array", "items": "int"}]`
+	buf := bytes.NewBuffer([]byte{})
+	enc, err := avro.NewEncoder(schema, buf)
+	require.NoError(t, err)
+
+	var val any = []int{}
+	err = enc.Encode(val)
+
+	require.NoError(t, err)
+	assert.Equal(t, []byte{0x02, 0x00}, buf.Bytes())
+}
+
+func TestEncoder_UnionInterfaceUnregisteredArray(t *testing.T) {
+	defer ConfigTeardown()
+
+	schema := `["int", {"type": "array", "items": "int"}]`
+	buf := bytes.NewBuffer([]byte{})
+	enc, err := avro.NewEncoder(schema, buf)
+	require.NoError(t, err)
+
+	var val any = map[string]any{
+		"array": []int{27},
+	}
+	err = enc.Encode(val)
+
+	require.NoError(t, err)
+	assert.Equal(t, []byte{0x02, 0x01, 0x02, 0x36, 0x00}, buf.Bytes())
+}
+
+func TestEncoder_UnionInterfaceUnregisteredArrayEmpty(t *testing.T) {
+	defer ConfigTeardown()
+
+	schema := `["int", {"type": "array", "items": "int"}]`
+	buf := bytes.NewBuffer([]byte{})
+	enc, err := avro.NewEncoder(schema, buf)
+	require.NoError(t, err)
+
+	var val any = map[string]any{
+		"array": []int{},
+	}
+	err = enc.Encode(val)
+
+	require.NoError(t, err)
+	assert.Equal(t, []byte{0x02, 0x00}, buf.Bytes())
+}
+
+func TestEncoder_UnionInterfaceUnregisteredArrayNull(t *testing.T) {
+	defer ConfigTeardown()
+
+	schema := `["int", {"type": "array", "items": "int"}]`
+	buf := bytes.NewBuffer([]byte{})
+	enc, err := avro.NewEncoder(schema, buf)
+	require.NoError(t, err)
+
+	var val any = map[string]any{
+		"array": nil,
+	}
+	err = enc.Encode(val)
+
+	require.NoError(t, err)
+	assert.Equal(t, []byte{0x02, 0x00}, buf.Bytes())
+}
+
 func TestEncoder_UnionInterfaceNull(t *testing.T) {
 	defer ConfigTeardown()