Repository: gocql/gocql
Pull request #1630 (2022-06-21 16:03:35)

Handle nil values when (un)marshalling lists and maps
Handle `nil` values for supported protocol versions when (un)marshalling lists and maps according to the specs:
```
6.11 list

  A [int] n indicating the number of elements in the list, followed by n
  elements.  Each element is [bytes] representing the serialized value.

6.12 map

  A [int] n indicating the number of key/value pairs in the map, followed by
  n entries.  Each entry is composed of two [bytes] representing the key
  and value.
```
```
    [bytes]        A [int] n, followed by n bytes if n >= 0. If n < 0,
                   no byte should follow and the value represented is `null`.
```



Patch:
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 422e79f..75d4d05 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -39,7 +39,7 @@ jobs:
       fail-fast: false
       matrix:
         go: [ '1.17', '1.18' ]
-        cassandra_version: [ '3.0.27', '3.11.13' ]
+        cassandra_version: [ '3.0.24', '3.11.10' ]
         auth: [ "false" ]
         compressor: [ "snappy" ]
         tags: [ "cassandra", "integration", "ccm" ]
@@ -112,7 +112,7 @@ jobs:
       fail-fast: false
       matrix:
         go: [ 1.17, 1.18 ]
-        cassandra_version: [ 3.11.13 ]
+        cassandra_version: [ 3.11.10 ]
         compressor: [ "snappy" ]
         tags: [ "integration" ]
 
diff --git a/AUTHORS b/AUTHORS
index ff78d46..c71553a 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -127,3 +127,4 @@ Yuto Doi <yutodoi.seattle@gmail.com>
 Krishna Vadali <tejavadali@gmail.com>
 Jens-W. Schicke-Uffmann <drahflow@gmx.de>
 Ondrej Polakovič <ondrej.polakovic@kiwi.com>
+Sergei Karetnikov <sergei.karetnikov@gmail.com>
diff --git a/marshal.go b/marshal.go
index 6879a79..afdfa94 100644
--- a/marshal.go
+++ b/marshal.go
@@ -1562,7 +1562,12 @@ func marshalList(info TypeInfo, value interface{}) ([]byte, error) {
 			if err != nil {
 				return nil, err
 			}
-			if err := writeCollectionSize(listInfo, len(item), buf); err != nil {
+			itemLen := len(item)
+			// Set the value to null for supported protocols
+			if item == nil && listInfo.proto > protoVersion2 {
+				itemLen = -1
+			}
+			if err := writeCollectionSize(listInfo, itemLen, buf); err != nil {
 				return nil, err
 			}
 			buf.Write(item)
@@ -1587,7 +1592,7 @@ func readCollectionSize(info CollectionType, data []byte) (size, read int, err e
 		if len(data) < 4 {
 			return 0, 0, unmarshalErrorf("unmarshal list: unexpected eof")
 		}
-		size = int(data[0])<<24 | int(data[1])<<16 | int(data[2])<<8 | int(data[3])
+		size = int(int32(data[0])<<24 | int32(data[1])<<16 | int32(data[2])<<8 | int32(data[3]))
 		read = 4
 	} else {
 		if len(data) < 2 {
@@ -1646,10 +1651,16 @@ func unmarshalList(info TypeInfo, data []byte, value interface{}) error {
 			if len(data) < m {
 				return unmarshalErrorf("unmarshal list: unexpected eof")
 			}
-			if err := Unmarshal(listInfo.Elem, data[:m], rv.Index(i).Addr().Interface()); err != nil {
+
+			// In case m < 0, the value is null, and unmarshalData should be nil.
+			var unmarshalData []byte
+			if m >= 0 {
+				unmarshalData = data[:m]
+				data = data[m:]
+			}
+			if err := Unmarshal(listInfo.Elem, unmarshalData, rv.Index(i).Addr().Interface()); err != nil {
 				return err
 			}
-			data = data[m:]
 		}
 		return nil
 	}
@@ -1692,7 +1703,12 @@ func marshalMap(info TypeInfo, value interface{}) ([]byte, error) {
 		if err != nil {
 			return nil, err
 		}
-		if err := writeCollectionSize(mapInfo, len(item), buf); err != nil {
+		itemLen := len(item)
+		// Set the key to null for supported protocols
+		if item == nil && mapInfo.proto > protoVersion2 {
+			itemLen = -1
+		}
+		if err := writeCollectionSize(mapInfo, itemLen, buf); err != nil {
 			return nil, err
 		}
 		buf.Write(item)
@@ -1701,7 +1717,12 @@ func marshalMap(info TypeInfo, value interface{}) ([]byte, error) {
 		if err != nil {
 			return nil, err
 		}
-		if err := writeCollectionSize(mapInfo, len(item), buf); err != nil {
+		itemLen = len(item)
+		// Set the value to null for supported protocols
+		if item == nil && mapInfo.proto > protoVersion2 {
+			itemLen = -1
+		}
+		if err := writeCollectionSize(mapInfo, itemLen, buf); err != nil {
 			return nil, err
 		}
 		buf.Write(item)
@@ -1744,10 +1765,16 @@ func unmarshalMap(info TypeInfo, data []byte, value interface{}) error {
 			return unmarshalErrorf("unmarshal map: unexpected eof")
 		}
 		key := reflect.New(t.Key())
-		if err := Unmarshal(mapInfo.Key, data[:m], key.Interface()); err != nil {
+
+		// In case m < 0, the key is null, and unmarshalData should be nil.
+		var unmarshalData []byte
+		if m >= 0 {
+			unmarshalData = data[:m]
+			data = data[m:]
+		}
+		if err := Unmarshal(mapInfo.Key, unmarshalData, key.Interface()); err != nil {
 			return err
 		}
-		data = data[m:]
 
 		m, p, err = readCollectionSize(mapInfo, data)
 		if err != nil {
@@ -1758,10 +1785,16 @@ func unmarshalMap(info TypeInfo, data []byte, value interface{}) error {
 			return unmarshalErrorf("unmarshal map: unexpected eof")
 		}
 		val := reflect.New(t.Elem())
-		if err := Unmarshal(mapInfo.Elem, data[:m], val.Interface()); err != nil {
+
+		// In case m < 0, the value is null, and unmarshalData should be nil.
+		unmarshalData = nil
+		if m >= 0 {
+			unmarshalData = data[:m]
+			data = data[m:]
+		}
+		if err := Unmarshal(mapInfo.Elem, unmarshalData, val.Interface()); err != nil {
 			return err
 		}
-		data = data[m:]
 
 		rv.SetMapIndex(key.Elem(), val.Elem())
 	}


Test patch:
diff --git a/marshal_test.go b/marshal_test.go
index 99b0a59..6048398 100644
--- a/marshal_test.go
+++ b/marshal_test.go
@@ -446,8 +446,8 @@ var marshalTests = []struct {
 		},
 		bytes.Join([][]byte{
 			[]byte("\x00\x01\xFF\xFF"),
-			bytes.Repeat([]byte("X"), 65535)}, []byte("")),
-		[]string{strings.Repeat("X", 65535)},
+			bytes.Repeat([]byte("X"), math.MaxUint16)}, []byte("")),
+		[]string{strings.Repeat("X", math.MaxUint16)},
 		nil,
 		nil,
 	},
@@ -459,11 +459,11 @@ var marshalTests = []struct {
 		},
 		bytes.Join([][]byte{
 			[]byte("\x00\x01\xFF\xFF"),
-			bytes.Repeat([]byte("X"), 65535),
+			bytes.Repeat([]byte("X"), math.MaxUint16),
 			[]byte("\xFF\xFF"),
-			bytes.Repeat([]byte("Y"), 65535)}, []byte("")),
+			bytes.Repeat([]byte("Y"), math.MaxUint16)}, []byte("")),
 		map[string]string{
-			strings.Repeat("X", 65535): strings.Repeat("Y", 65535),
+			strings.Repeat("X", math.MaxUint16): strings.Repeat("Y", math.MaxUint16),
 		},
 		nil,
 		nil,
@@ -1475,12 +1475,12 @@ func TestMarshalVarint(t *testing.T) {
 	}
 }
 
-func equalStringSlice(leftList, rightList []string) bool {
+func equalStringPointerSlice(leftList, rightList []*string) bool {
 	if len(leftList) != len(rightList) {
 		return false
 	}
 	for index := range leftList {
-		if rightList[index] != leftList[index] {
+		if !reflect.DeepEqual(rightList[index], leftList[index]) {
 			return false
 		}
 	}
@@ -1488,42 +1488,105 @@ func equalStringSlice(leftList, rightList []string) bool {
 }
 
 func TestMarshalList(t *testing.T) {
-	typeInfo := CollectionType{
+	typeInfoV2 := CollectionType{
 		NativeType: NativeType{proto: 2, typ: TypeList},
 		Elem:       NativeType{proto: 2, typ: TypeVarchar},
 	}
+	typeInfoV3 := CollectionType{
+		NativeType: NativeType{proto: 3, typ: TypeList},
+		Elem:       NativeType{proto: 3, typ: TypeVarchar},
+	}
 
-	sourceLists := [][]string{
-		{"valueA"},
-		{"valueA", "valueB"},
-		{"valueB"},
+	type tc struct {
+		typeInfo CollectionType
+		input    []*string
+		expected []*string
 	}
 
-	listDatas := [][]byte{}
+	valueA := "valueA"
+	valueB := "valueB"
+	valueEmpty := ""
+	testCases := []tc{
+		{
+			typeInfo: typeInfoV2,
+			input:    []*string{&valueA},
+			expected: []*string{&valueA},
+		},
+		{
+			typeInfo: typeInfoV2,
+			input:    []*string{&valueA, &valueB},
+			expected: []*string{&valueA, &valueB},
+		},
+		{
+			typeInfo: typeInfoV2,
+			input:    []*string{&valueA, &valueEmpty, &valueB},
+			expected: []*string{&valueA, &valueEmpty, &valueB},
+		},
+		{
+			typeInfo: typeInfoV2,
+			input:    []*string{&valueEmpty},
+			expected: []*string{&valueEmpty},
+		},
+		{
+			// nil values are marshalled to empty values for protocol < 3
+			typeInfo: typeInfoV2,
+			input:    []*string{nil},
+			expected: []*string{&valueEmpty},
+		},
+		{
+			typeInfo: typeInfoV2,
+			input:    []*string{&valueA, nil, &valueB},
+			expected: []*string{&valueA, &valueEmpty, &valueB},
+		},
+		{
+			typeInfo: typeInfoV3,
+			input:    []*string{&valueEmpty},
+			expected: []*string{&valueEmpty},
+		},
+		{
+			typeInfo: typeInfoV3,
+			input:    []*string{nil},
+			expected: []*string{nil},
+		},
+		{
+			typeInfo: typeInfoV3,
+			input:    []*string{&valueA, nil, &valueB},
+			expected: []*string{&valueA, nil, &valueB},
+		},
+	}
 
-	for _, list := range sourceLists {
-		listData, marshalErr := Marshal(typeInfo, list)
+	listDatas := [][]byte{}
+	for _, c := range testCases {
+		listData, marshalErr := Marshal(c.typeInfo, c.input)
 		if nil != marshalErr {
-			t.Errorf("Error marshal %+v of type %+v: %s", list, typeInfo, marshalErr)
+			t.Errorf("Error marshal %+v of type %+v: %s", c.input, c.typeInfo, marshalErr)
 		}
 		listDatas = append(listDatas, listData)
 	}
 
-	outputLists := [][]string{}
+	outputLists := [][]*string{}
 
-	var outputList []string
+	var outputList []*string
 
-	for _, listData := range listDatas {
-		if unmarshalErr := Unmarshal(typeInfo, listData, &outputList); nil != unmarshalErr {
+	for i, listData := range listDatas {
+		if unmarshalErr := Unmarshal(testCases[i].typeInfo, listData, &outputList); nil != unmarshalErr {
 			t.Error(unmarshalErr)
 		}
+		resultList := []interface{}{}
+		for i := range outputList {
+			if outputList[i] != nil {
+				resultList = append(resultList, *outputList[i])
+			} else {
+				resultList = append(resultList, nil)
+			}
+		}
 		outputLists = append(outputLists, outputList)
 	}
 
-	for index, sourceList := range sourceLists {
+	for index, c := range testCases {
 		outputList := outputLists[index]
-		if !equalStringSlice(sourceList, outputList) {
-			t.Errorf("Lists %+v not equal to lists %+v, but should", sourceList, outputList)
+		if !equalStringPointerSlice(c.expected, outputList) {
+			t.Errorf("Lists %+v not equal to lists %+v, but should", c.expected, outputList)
 		}
 	}
 }