Merge pull request #755 from gopcua/issue-678

Issue #678: Fix Variant to handle nil slices

go.mod

@@ -5,11 +5,17 @@ go 1.22.0
 require (
 	github.com/google/uuid v1.3.0
 	github.com/pascaldekloe/goe v0.1.1
+	github.com/stretchr/testify v1.10.0
 	golang.org/x/exp v0.0.0-20241204233417-43b7b7cde48d
 	golang.org/x/term v0.8.0
 )
 
-require golang.org/x/sys v0.8.0 // indirect
+require (
+	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
+	golang.org/x/sys v0.8.0 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
+)
 
 retract (
 	v0.2.5 // https://github.com/gopcua/opcua/issues/538

go.sum

@@ -1,10 +1,20 @@
+github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
+github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
 github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/pascaldekloe/goe v0.1.1 h1:Ah6WQ56rZONR3RW3qWa2NCZ6JAVvSpUcoLBaOmYFt9Q=
 github.com/pascaldekloe/goe v0.1.1/go.mod h1:KSyfaxQOh0HZPjDP1FL/kFtbqYqrALJTaMafFUIccqU=
+github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
+github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
+github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
 golang.org/x/exp v0.0.0-20241204233417-43b7b7cde48d h1:0olWaB5pg3+oychR51GUVCEsGkeCU/2JxjBgIo4f3M0=
 golang.org/x/exp v0.0.0-20241204233417-43b7b7cde48d/go.mod h1:qj5a5QZpwLU2NLQudwIN5koi3beDhSAlJwa67PuM98c=
 golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU=
 golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.8.0 h1:n5xxQn2i3PC0yLAbjTpNT85q/Kgzcr2gIoX9OrJUols=
 golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
+gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

ua/codec_test.go

@@ -10,7 +10,7 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/pascaldekloe/goe/verify"
+	"github.com/stretchr/testify/require"
 )
 
 // CodecTestCase describes a test case for a encoding and decoding an
@@ -49,15 +49,15 @@ func RunCodecTest(t *testing.T, cases []CodecTestCase) {
 				if typ.Kind() == reflect.Slice {
 					v = v.Elem()
 				}
-				verify.Values(t, "", v.Interface(), c.Struct)
+				require.Equal(t, c.Struct, v.Interface())
 			})
 
 			t.Run("encode", func(t *testing.T) {
 				b, err := Encode(c.Struct)
 				if err != nil {
 					t.Fatal(err)
 				}
-				verify.Values(t, "", b, c.Bytes)
+				require.Equal(t, c.Bytes, b)
 			})
 		})
 	}

ua/datatypes_test.go

@@ -73,6 +73,26 @@ func TestDataValue(t *testing.T) {
 				0x80, 0x3b, 0xe8, 0xb3, 0x92, 0x4e, 0xd4, 0x01,
 			},
 		},
+		{
+			Name: "value with nil slice, source timestamp, server timestamp",
+			Struct: &DataValue{
+				EncodingMask:    0x0d,
+				Value:           MustVariant([]string(nil)),
+				SourceTimestamp: time.Date(2018, time.September, 17, 14, 28, 29, 112000000, time.UTC),
+				ServerTimestamp: time.Date(2018, time.September, 17, 14, 28, 29, 112000000, time.UTC),
+			},
+			Bytes: []byte{
+				// EncodingMask
+				0x0d,
+				// Value
+				0x8c,                   // type
+				0xff, 0xff, 0xff, 0xff, // value
+				// SourceTimestamp
+				0x80, 0x3b, 0xe8, 0xb3, 0x92, 0x4e, 0xd4, 0x01,
+				// SeverTimestamp
+				0x80, 0x3b, 0xe8, 0xb3, 0x92, 0x4e, 0xd4, 0x01,
+			},
+		},
 	}
 	RunCodecTest(t, cases)
 }

ua/variant.go

@@ -141,19 +141,29 @@ func (m *Variant) Decode(b []byte) (int, error) {
 
 	// read flattened array elements
 	n := int(m.arrayLength)
-	if n < 0 || n > MaxVariantArrayLength {
+	if n > MaxVariantArrayLength {
 		return buf.Pos(), StatusBadEncodingLimitsExceeded
 	}
 
+	// get the type for the slice
+	sliceType := reflect.SliceOf(typ)
+	if m.Type() == TypeIDByte {
+		sliceType = reflect.TypeOf(ByteArray{})
+	}
+
 	var vals reflect.Value
-	switch m.Type() {
-	case TypeIDByte:
-		vals = reflect.MakeSlice(reflect.TypeOf(ByteArray{}), n, n)
+	switch {
+	// decode a nil slice
+	case n == -1:
+		vals = reflect.Zero(reflect.MakeSlice(sliceType, 0, 0).Type())
+		m.value = vals.Interface()
+
+	// decode a slice with values
 	default:
-		vals = reflect.MakeSlice(reflect.SliceOf(typ), n, n)
-	}
-	for i := 0; i < n; i++ {
-		vals.Index(i).Set(reflect.ValueOf(m.decodeValue(buf)))
+		vals = reflect.MakeSlice(sliceType, n, n)
+		for i := 0; i < n; i++ {
+			vals.Index(i).Set(reflect.ValueOf(m.decodeValue(buf)))
+		}
 	}
 
 	// check for dimensions of multi-dimensional array
@@ -416,9 +426,11 @@ var errUnbalancedSlice = errors.New("unbalanced multi-dimensional array")
 
 // sliceDim determines the element type, dimensions and the total length
 // of a one or multi-dimensional slice.
-func sliceDim(v reflect.Value) (typ reflect.Type, dim []int32, count int32, err error) {
+//
+// If the value is a nil slice then count is -1.
+func sliceDim(val reflect.Value) (typ reflect.Type, dim []int32, count int32, err error) {
 	// null type
-	if v.Kind() == reflect.Invalid {
+	if val.Kind() == reflect.Invalid {
 		return nil, nil, 0, nil
 	}
 
@@ -430,35 +442,40 @@ func sliceDim(v reflect.Value) (typ reflect.Type, dim []int32, count int32, err
 	// array of Byte.
 	//
 	// https://github.com/gopcua/opcua/issues/463
-	if v.Type() == reflect.TypeOf([]byte{}) && v.Type() != reflect.TypeOf(ByteArray{}) {
-		return v.Type(), nil, 1, nil
+	if val.Type() == reflect.TypeOf([]byte{}) && val.Type() != reflect.TypeOf(ByteArray{}) {
+		return val.Type(), nil, 1, nil
 	}
 
 	// element type
-	if v.Kind() != reflect.Slice {
-		return v.Type(), nil, 1, nil
+	if val.Kind() != reflect.Slice {
+		return val.Type(), nil, 1, nil
+	}
+
+	// nil array
+	if val.IsNil() {
+		return val.Type().Elem(), nil, -1, nil
 	}
 
 	// empty array
-	if v.Len() == 0 {
-		return v.Type().Elem(), append([]int32{0}, dim...), 0, nil
+	if val.Len() == 0 {
+		return val.Type().Elem(), append([]int32{0}, dim...), 0, nil
 	}
 
 	// check that inner slices all have the same length
-	if v.Index(0).Kind() == reflect.Slice {
-		for i := 0; i < v.Len(); i++ {
-			if v.Index(i).Len() != v.Index(0).Len() {
+	if val.Index(0).Kind() == reflect.Slice {
+		for i := 0; i < val.Len(); i++ {
+			if val.Index(i).Len() != val.Index(0).Len() {
 				return nil, nil, 0, errUnbalancedSlice
 			}
 		}
 	}
 
 	// recurse to inner slice or element type
-	typ, dim, count, err = sliceDim(v.Index(0))
+	typ, dim, count, err = sliceDim(val.Index(0))
 	if err != nil {
 		return nil, nil, 0, err
 	}
-	return typ, append([]int32{int32(v.Len())}, dim...), count * int32(v.Len()), nil
+	return typ, append([]int32{int32(val.Len())}, dim...), count * int32(val.Len()), nil
 }
 
 // set sets the value and updates the flags according to the type.
@@ -469,15 +486,18 @@ func (m *Variant) set(v interface{}) error {
 		return err
 	}
 
-	if len(dim) > 0 {
-		m.mask |= VariantArrayValues
+	switch {
+	case len(dim) > 1:
+		m.mask |= VariantArrayValues | VariantArrayDimensions
 		m.arrayLength = count
-	}
-
-	if len(dim) > 1 {
-		m.mask |= VariantArrayDimensions
 		m.arrayDimensionsLength = int32(len(dim))
 		m.arrayDimensions = dim
+
+	case len(dim) > 0 || count == -1:
+		m.mask |= VariantArrayValues
+		m.arrayLength = count
+		m.arrayDimensionsLength = 0
+		m.arrayDimensions = nil
 	}
 
 	typeid, ok := variantTypeToTypeID[et]

ua/variant_test.go

@@ -477,6 +477,16 @@ func TestVariant(t *testing.T) {
 				0x01, 0x00, 0x00, 0x00,
 			},
 		},
+		{
+			Name:   "[]string(nil)",
+			Struct: MustVariant([]string(nil)),
+			Bytes: []byte{
+				// variant encoding mask
+				0x8c,
+				// array length
+				0xff, 0xff, 0xff, 0xff,
+			},
+		},
 	}
 	RunCodecTest(t, cases)
 }
@@ -543,19 +553,6 @@ func TestArray(t *testing.T) {
 			t.Fatalf("got error %#v want %#v", got, want)
 		}
 	})
-	t.Run("length negative", func(t *testing.T) {
-		b := []byte{
-			// variant encoding mask
-			0x87,
-			// array length
-			0xff, 0xff, 0xff, 0xff, // -1
-		}
-
-		_, err := Decode(b, MustVariant([]uint32{0}))
-		if got, want := err, StatusBadEncodingLimitsExceeded; !errors.Equal(got, want) {
-			t.Fatalf("got error %#v want %#v", got, want)
-		}
-	})
 	t.Run("length too big", func(t *testing.T) {
 		b := []byte{
 			// variant encoding mask