🧹 chore: Handle nil map targets in Binder (#3839)

Author: gaby

binder/binder.go

@@ -9,6 +9,7 @@ import (
 var (
 	ErrSuitableContentNotFound = errors.New("binder: suitable content not found to parse body")
 	ErrMapNotConvertible       = errors.New("binder: map is not convertible to map[string]string or map[string][]string")
+	ErrMapNilDestination       = errors.New("binder: map destination is nil and cannot be initialized")
 )
 
 var HeaderBinderPool = sync.Pool{

binder/binder_test.go

@@ -72,14 +72,19 @@ func Test_GetFieldCache_Panic(t *testing.T) {
 func Test_parseToMap_defaultCase(t *testing.T) {
 	t.Parallel()
 	m := map[string]int{}
-	err := parseToMap(m, map[string][]string{"a": {"1"}})
+	err := parseToMap(reflect.ValueOf(m), map[string][]string{"a": {"1"}})
 	require.NoError(t, err)
 	require.Empty(t, m)
 
 	m2 := map[string]string{}
-	err = parseToMap(m2, map[string][]string{"empty": {}})
+	err = parseToMap(reflect.ValueOf(m2), map[string][]string{"empty": {}})
 	require.NoError(t, err)
 	require.Empty(t, m2["empty"])
+
+	var zeroStringMap map[string]string
+	err = parseToMap(reflect.ValueOf(&zeroStringMap).Elem(), map[string][]string{"name": {"john"}})
+	require.NoError(t, err)
+	require.Equal(t, "john", zeroStringMap["name"])
 }
 
 func Test_parse_function_maps(t *testing.T) {
@@ -94,6 +99,16 @@ func Test_parse_function_maps(t *testing.T) {
 	err = parse("query", &m2, map[string][]string{"a": {"b"}})
 	require.NoError(t, err)
 	require.Equal(t, "b", m2["a"])
+
+	var zeroStringMap map[string]string
+	err = parse("query", &zeroStringMap, map[string][]string{"foo": {"bar", "baz"}})
+	require.NoError(t, err)
+	require.Equal(t, "baz", zeroStringMap["foo"])
+
+	var zeroSliceMap map[string][]string
+	err = parse("query", &zeroSliceMap, map[string][]string{"foo": {"bar", "baz"}})
+	require.NoError(t, err)
+	require.Equal(t, []string{"bar", "baz"}, zeroSliceMap["foo"])
 }
 
 func Test_SetParserDecoder_UnknownKeys(t *testing.T) {

binder/mapping.go

@@ -81,7 +81,7 @@ func parse(aliasTag string, out any, data map[string][]string, files ...map[stri
 
 	// Parse into the map
 	if ptrVal.Kind() == reflect.Map && ptrVal.Type().Key().Kind() == reflect.String {
-		return parseToMap(ptrVal.Interface(), data)
+		return parseToMap(ptrVal, data)
 	}
 
 	// Parse into the struct
@@ -106,19 +106,36 @@ func parseToStruct(aliasTag string, out any, data map[string][]string, files ...
 
 // Parse data into the map
 // thanks to https://github.com/gin-gonic/gin/blob/master/binding/binding.go
-func parseToMap(ptr any, data map[string][]string) error {
-	elem := reflect.TypeOf(ptr).Elem()
+func parseToMap(target reflect.Value, data map[string][]string) error {
+	if !target.IsValid() {
+		return errors.New("binder: invalid destination value")
+	}
+
+	if target.Kind() == reflect.Interface && !target.IsNil() {
+		target = target.Elem()
+	}
+
+	if target.Kind() != reflect.Map || target.Type().Key().Kind() != reflect.String {
+		return nil // nothing to do for non-map destinations
+	}
+
+	if target.IsNil() {
+		if !target.CanSet() {
+			return ErrMapNilDestination
+		}
+		target.Set(reflect.MakeMap(target.Type()))
+	}
 
-	switch elem.Kind() {
+	switch target.Type().Elem().Kind() {
 	case reflect.Slice:
-		newMap, ok := ptr.(map[string][]string)
+		newMap, ok := target.Interface().(map[string][]string)
 		if !ok {
 			return ErrMapNotConvertible
 		}
 
 		maps.Copy(newMap, data)
-	case reflect.String, reflect.Interface:
-		newMap, ok := ptr.(map[string]string)
+	case reflect.String:
+		newMap, ok := target.Interface().(map[string]string)
 		if !ok {
 			return ErrMapNotConvertible
 		}
@@ -131,6 +148,10 @@ func parseToMap(ptr any, data map[string][]string) error {
 			newMap[k] = v[len(v)-1]
 		}
 	default:
+		// Interface element maps (e.g. map[string]any) are left untouched because
+		// the binder cannot safely infer element conversions without mutating
+		// caller-provided values. These destinations therefore see a successful
+		// no-op parse.
 		return nil // it's not necessary to check all types
 	}
 

binder/mapping_test.go

@@ -133,7 +133,7 @@ func Test_parseToMap(t *testing.T) {
 
 	// Test map[string]string
 	m := make(map[string]string)
-	err := parseToMap(m, inputMap)
+	err := parseToMap(reflect.ValueOf(m), inputMap)
 	require.NoError(t, err)
 
 	require.Equal(t, "value2", m["key1"])
@@ -142,7 +142,7 @@ func Test_parseToMap(t *testing.T) {
 
 	// Test map[string][]string
 	m2 := make(map[string][]string)
-	err = parseToMap(m2, inputMap)
+	err = parseToMap(reflect.ValueOf(m2), inputMap)
 	require.NoError(t, err)
 
 	require.Len(t, m2["key1"], 2)
@@ -153,8 +153,22 @@ func Test_parseToMap(t *testing.T) {
 
 	// Test map[string]any
 	m3 := make(map[string]any)
-	err = parseToMap(m3, inputMap)
-	require.ErrorIs(t, err, ErrMapNotConvertible)
+	err = parseToMap(reflect.ValueOf(m3), inputMap)
+	require.NoError(t, err)
+	require.Empty(t, m3)
+
+	var zeroStringMap map[string]string
+	err = parseToMap(reflect.ValueOf(&zeroStringMap).Elem(), inputMap)
+	require.NoError(t, err)
+	require.Equal(t, "value2", zeroStringMap["key1"])
+
+	var zeroSliceMap map[string][]string
+	err = parseToMap(reflect.ValueOf(&zeroSliceMap).Elem(), inputMap)
+	require.NoError(t, err)
+	require.Len(t, zeroSliceMap["key1"], 2)
+
+	err = parseToMap(reflect.ValueOf(map[string]string(nil)), inputMap)
+	require.ErrorIs(t, err, ErrMapNilDestination)
 }
 
 func Test_FilterFlags(t *testing.T) {
@@ -384,16 +398,16 @@ func Test_parseToMap_Extended(t *testing.T) {
 	}
 
 	m := make(map[string]string)
-	err := parseToMap(m, data)
+	err := parseToMap(reflect.ValueOf(m), data)
 	require.NoError(t, err)
 	require.Empty(t, m["empty"])
 
 	m2 := make(map[string][]int)
-	err = parseToMap(m2, data)
+	err = parseToMap(reflect.ValueOf(m2), data)
 	require.ErrorIs(t, err, ErrMapNotConvertible)
 
 	m3 := make(map[string]int)
-	err = parseToMap(m3, data)
+	err = parseToMap(reflect.ValueOf(m3), data)
 	require.NoError(t, err)
 }