Skip to content

Commit f9ec857

Browse files
committed
(improvement) harden RowData and improve NewWithError test coverage
- Add defensive guard in RowData() to detect actualColCount metadata inconsistency: returns a clear error instead of silently producing trailing zero-values or panicking on index-out-of-bounds. - Set iter.err in column count mismatch guard to match error contract. - Add overflow bounds checks before slice indexing to prevent panics. - Add TypeFloat fast-path for map values in CollectionType.NewWithError() for both string-key and int-key maps, avoiding reflection for map[string]float32 and map[int]float32. - Move TestNewWithErrorConsistentWithGoType from helpers_bench_test.go to marshal_test.go (with unit build tag) where it logically belongs. - Add TestCollectionNewWithErrorConsistentWithGoType to verify that CollectionType.NewWithError() fast-paths stay in sync with goType() for list, set, and map types with common element/key combinations. Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
1 parent 1edee76 commit f9ec857

File tree

4 files changed

+170
-60
lines changed

4 files changed

+170
-60
lines changed

helpers.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,11 @@ func (iter *Iter) RowData() (RowData, error) {
376376
idx := 0
377377
for _, column := range iter.Columns() {
378378
if c, ok := column.TypeInfo.(TupleTypeInfo); !ok {
379+
if idx >= actualSize {
380+
err := fmt.Errorf("gocql: column count overflow in RowData: metadata predicted %d columns but encountered more", actualSize)
381+
iter.err = err
382+
return RowData{}, err
383+
}
379384
val, err := column.TypeInfo.NewWithError()
380385
if err != nil {
381386
iter.err = err
@@ -386,6 +391,11 @@ func (iter *Iter) RowData() (RowData, error) {
386391
idx++
387392
} else {
388393
for i, elem := range c.Elems {
394+
if idx >= actualSize {
395+
err := fmt.Errorf("gocql: column count overflow in RowData: metadata predicted %d columns but encountered more", actualSize)
396+
iter.err = err
397+
return RowData{}, err
398+
}
389399
columns[idx] = TupleColumnName(column.Name, i)
390400
val, err := elem.NewWithError()
391401
if err != nil {
@@ -398,6 +408,12 @@ func (iter *Iter) RowData() (RowData, error) {
398408
}
399409
}
400410

411+
if idx != actualSize {
412+
err := fmt.Errorf("gocql: column count mismatch in RowData: metadata predicted %d columns but got %d", actualSize, idx)
413+
iter.err = err
414+
return RowData{}, err
415+
}
416+
401417
rowData := RowData{
402418
Columns: columns,
403419
Values: values,

helpers_bench_test.go

Lines changed: 0 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ package gocql
2020

2121
import (
2222
"fmt"
23-
"reflect"
2423
"testing"
2524
)
2625

@@ -228,61 +227,3 @@ func BenchmarkRowDataAllocation(b *testing.B) {
228227
})
229228
}
230229
}
231-
232-
// TestNewWithErrorConsistentWithGoType verifies that the fast-path type mapping
233-
// in NativeType.NewWithError() stays consistent with the canonical goType() mapping.
234-
// This guards against future changes to one mapping that forget to update the other.
235-
func TestNewWithErrorConsistentWithGoType(t *testing.T) {
236-
// All NativeType type codes that goType handles (excluding collection/tuple/UDT
237-
// which are separate TypeInfo implementations).
238-
nativeTypes := []Type{
239-
TypeVarchar, TypeAscii, TypeText, TypeInet,
240-
TypeBigInt, TypeCounter,
241-
TypeTime,
242-
TypeTimestamp,
243-
TypeBlob,
244-
TypeBoolean,
245-
TypeFloat,
246-
TypeDouble,
247-
TypeInt,
248-
TypeSmallInt,
249-
TypeTinyInt,
250-
TypeDecimal,
251-
TypeUUID, TypeTimeUUID,
252-
TypeVarint,
253-
TypeDate,
254-
TypeDuration,
255-
}
256-
257-
for _, typ := range nativeTypes {
258-
nt := NativeType{typ: typ, proto: protoVersion4}
259-
260-
// Get the fast-path result from NewWithError
261-
fastVal, err := nt.NewWithError()
262-
if err != nil {
263-
t.Errorf("NewWithError(%s): unexpected error: %v", typ, err)
264-
continue
265-
}
266-
267-
// Get the canonical type from goType
268-
canonicalType, err := goType(nt)
269-
if err != nil {
270-
t.Errorf("goType(%s): unexpected error: %v", typ, err)
271-
continue
272-
}
273-
274-
// NewWithError returns a pointer (reflect.New(typ).Interface()), so the
275-
// underlying type is reflect.TypeOf(val).Elem()
276-
fastType := reflect.TypeOf(fastVal)
277-
if fastType.Kind() != reflect.Ptr {
278-
t.Errorf("NewWithError(%s): expected pointer, got %s", typ, fastType.Kind())
279-
continue
280-
}
281-
fastElemType := fastType.Elem()
282-
283-
if fastElemType != canonicalType {
284-
t.Errorf("NewWithError(%s) fast-path type %s does not match goType() canonical type %s",
285-
typ, fastElemType, canonicalType)
286-
}
287-
}
288-
}

marshal.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1862,6 +1862,8 @@ func (t CollectionType) NewWithError() (interface{}, error) {
18621862
return new(map[string]string), nil
18631863
case TypeBoolean:
18641864
return new(map[string]bool), nil
1865+
case TypeFloat:
1866+
return new(map[string]float32), nil
18651867
case TypeDouble:
18661868
return new(map[string]float64), nil
18671869
case TypeUUID:
@@ -1875,6 +1877,8 @@ func (t CollectionType) NewWithError() (interface{}, error) {
18751877
return new(map[int]string), nil
18761878
case TypeInt:
18771879
return new(map[int]int), nil
1880+
case TypeFloat:
1881+
return new(map[int]float32), nil
18781882
}
18791883
}
18801884
}
@@ -1926,7 +1930,7 @@ func (t TupleTypeInfo) String() string {
19261930
}
19271931

19281932
func (t TupleTypeInfo) NewWithError() (interface{}, error) {
1929-
// Tuples scan into *[]interface{} (pointer to a slice of interface values).
1933+
// Tuples scan into *[]interface{} — no reflection needed.
19301934
return new([]interface{}), nil
19311935
}
19321936

marshal_test.go

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,3 +1035,152 @@ func bytesWithLength(data ...[]byte) []byte {
10351035
}
10361036
return ret
10371037
}
1038+
1039+
// TestNativeNewWithErrorConsistentWithGoType verifies that the fast-path type mapping
1040+
// in NativeType.NewWithError() stays consistent with the canonical goType() mapping.
1041+
// This guards against future changes to one mapping that forget to update the other.
1042+
func TestNativeNewWithErrorConsistentWithGoType(t *testing.T) {
1043+
// All NativeType type codes that goType handles (excluding collection/tuple/UDT
1044+
// which are separate TypeInfo implementations).
1045+
nativeTypes := []Type{
1046+
TypeVarchar, TypeAscii, TypeText, TypeInet,
1047+
TypeBigInt, TypeCounter,
1048+
TypeTime,
1049+
TypeTimestamp,
1050+
TypeBlob,
1051+
TypeBoolean,
1052+
TypeFloat,
1053+
TypeDouble,
1054+
TypeInt,
1055+
TypeSmallInt,
1056+
TypeTinyInt,
1057+
TypeDecimal,
1058+
TypeUUID, TypeTimeUUID,
1059+
TypeVarint,
1060+
TypeDate,
1061+
TypeDuration,
1062+
}
1063+
1064+
for _, typ := range nativeTypes {
1065+
nt := NativeType{typ: typ, proto: protoVersion4}
1066+
1067+
// Get the fast-path result from NewWithError
1068+
fastVal, err := nt.NewWithError()
1069+
if err != nil {
1070+
t.Errorf("NewWithError(%s): unexpected error: %v", typ, err)
1071+
continue
1072+
}
1073+
1074+
// Get the canonical type from goType
1075+
canonicalType, err := goType(nt)
1076+
if err != nil {
1077+
t.Errorf("goType(%s): unexpected error: %v", typ, err)
1078+
continue
1079+
}
1080+
1081+
// NewWithError returns a pointer (reflect.New(typ).Interface()), so the
1082+
// underlying type is reflect.TypeOf(val).Elem()
1083+
fastType := reflect.TypeOf(fastVal)
1084+
if fastType.Kind() != reflect.Ptr {
1085+
t.Errorf("NewWithError(%s): expected pointer, got %s", typ, fastType.Kind())
1086+
continue
1087+
}
1088+
fastElemType := fastType.Elem()
1089+
1090+
if fastElemType != canonicalType {
1091+
t.Errorf("NewWithError(%s) fast-path type %s does not match goType() canonical type %s",
1092+
typ, fastElemType, canonicalType)
1093+
}
1094+
}
1095+
}
1096+
1097+
// TestCollectionNewWithErrorConsistentWithGoType verifies that the fast-path type mapping
1098+
// in CollectionType.NewWithError() stays consistent with the canonical goType() mapping.
1099+
func TestCollectionNewWithErrorConsistentWithGoType(t *testing.T) {
1100+
elemTypes := []Type{
1101+
TypeInt, TypeBigInt, TypeCounter,
1102+
TypeText, TypeVarchar, TypeAscii,
1103+
TypeBoolean,
1104+
TypeFloat, TypeDouble,
1105+
TypeUUID, TypeTimeUUID,
1106+
TypeTimestamp, TypeDate,
1107+
TypeSmallInt, TypeTinyInt,
1108+
TypeBlob,
1109+
}
1110+
1111+
// Test list and set types
1112+
for _, collTyp := range []Type{TypeList, TypeSet} {
1113+
for _, elemTyp := range elemTypes {
1114+
ct := CollectionType{
1115+
NativeType: NativeType{typ: collTyp, proto: protoVersion4},
1116+
Elem: NativeType{typ: elemTyp, proto: protoVersion4},
1117+
}
1118+
1119+
fastVal, err := ct.NewWithError()
1120+
if err != nil {
1121+
t.Errorf("NewWithError(%s<%s>): unexpected error: %v", collTyp, elemTyp, err)
1122+
continue
1123+
}
1124+
1125+
canonicalType, err := goType(ct)
1126+
if err != nil {
1127+
t.Errorf("goType(%s<%s>): unexpected error: %v", collTyp, elemTyp, err)
1128+
continue
1129+
}
1130+
1131+
fastType := reflect.TypeOf(fastVal)
1132+
if fastType.Kind() != reflect.Ptr {
1133+
t.Errorf("NewWithError(%s<%s>): expected pointer, got %s", collTyp, elemTyp, fastType.Kind())
1134+
continue
1135+
}
1136+
1137+
if fastType.Elem() != canonicalType {
1138+
t.Errorf("NewWithError(%s<%s>) fast-path type %s does not match goType() canonical type %s",
1139+
collTyp, elemTyp, fastType.Elem(), canonicalType)
1140+
}
1141+
}
1142+
}
1143+
1144+
// Test map types with common key/value combinations
1145+
keyTypes := []Type{TypeText, TypeVarchar, TypeInt}
1146+
valTypes := []Type{
1147+
TypeInt, TypeBigInt,
1148+
TypeText, TypeVarchar,
1149+
TypeBoolean,
1150+
TypeFloat, TypeDouble,
1151+
TypeUUID,
1152+
}
1153+
1154+
for _, keyTyp := range keyTypes {
1155+
for _, valTyp := range valTypes {
1156+
ct := CollectionType{
1157+
NativeType: NativeType{typ: TypeMap, proto: protoVersion4},
1158+
Key: NativeType{typ: keyTyp, proto: protoVersion4},
1159+
Elem: NativeType{typ: valTyp, proto: protoVersion4},
1160+
}
1161+
1162+
fastVal, err := ct.NewWithError()
1163+
if err != nil {
1164+
t.Errorf("NewWithError(map<%s, %s>): unexpected error: %v", keyTyp, valTyp, err)
1165+
continue
1166+
}
1167+
1168+
canonicalType, err := goType(ct)
1169+
if err != nil {
1170+
t.Errorf("goType(map<%s, %s>): unexpected error: %v", keyTyp, valTyp, err)
1171+
continue
1172+
}
1173+
1174+
fastType := reflect.TypeOf(fastVal)
1175+
if fastType.Kind() != reflect.Ptr {
1176+
t.Errorf("NewWithError(map<%s, %s>): expected pointer, got %s", keyTyp, valTyp, fastType.Kind())
1177+
continue
1178+
}
1179+
1180+
if fastType.Elem() != canonicalType {
1181+
t.Errorf("NewWithError(map<%s, %s>) fast-path type %s does not match goType() canonical type %s",
1182+
keyTyp, valTyp, fastType.Elem(), canonicalType)
1183+
}
1184+
}
1185+
}
1186+
}

0 commit comments

Comments
 (0)