Skip to content

Commit 2755f15

Browse files
mromaszewiczclaude
andauthored
Fix form binding of Nullables (#133)
Closes: #129 `nullable.Nullable[T]` is defined as `map[bool]T`, but neither `bindFormImpl` nor `BindStringToObjectWithOptions` had a `reflect.Map` branch, so any Nullable field in a form body failed with "error binding string parameter: can not bind to destination of type: map". This change adds Map handling at both layers: - `BindStringToObjectWithOptions` (bindstring.go): a bool-keyed map destination is treated as a Nullable wrapper -- bind `src` into a fresh value of the map's element type, then store it under `map[bool]T{true: value}`. This covers query, path, and header parameters in addition to forms. - `bindFormImpl` (bindform.go): a bool-keyed map field recurses through `bindFormImpl` on the element type and wraps the result, so `Nullable[ComplexStruct]` works alongside scalars. A non-bool-keyed map field routes to a new `bindFormMap` helper that binds entries of the form `name[key]=value` into a generic `map[K]V`. The structural `map[bool]T` check keeps `runtime` decoupled from `github.com/oapi-codegen/nullable`; the data shape is the whole type, so detecting it by reflection is equivalent to importing the package. An absent form field still produces an unspecified (zero) Nullable, matching the pre-fix behavior. Form payloads have no way to encode an explicit null, so `name=` binds as the inner type's zero value (specified), symmetric with non-nullable form binding. Tests cover scalar Nullable, generic `map[K]V`, and the absent-field-stays-unspecified path. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 17de1dd commit 2755f15

5 files changed

Lines changed: 178 additions & 0 deletions

File tree

bindform.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,26 @@ func bindFormImpl(v reflect.Value, form map[string][]string, files map[string][]
194194
hasData = hasData || fieldHasData
195195
}
196196
return hasData, nil
197+
case reflect.Map:
198+
// A bool-keyed map (such as nullable.Nullable[T], which is
199+
// map[bool]T) is treated as a nullable wrapper: bind the inner type
200+
// and store the result under map[true]. An absent field stays as
201+
// the zero (unspecified) map.
202+
if v.Type().Key().Kind() == reflect.Bool {
203+
valuePtr := reflect.New(v.Type().Elem())
204+
valueHasData, err := bindFormImpl(valuePtr.Elem(), form, files, name)
205+
if err != nil {
206+
return false, err
207+
}
208+
if valueHasData {
209+
newMap := reflect.MakeMap(v.Type())
210+
newMap.SetMapIndex(reflect.ValueOf(true), valuePtr.Elem())
211+
v.Set(newMap)
212+
return true, nil
213+
}
214+
return false, nil
215+
}
216+
return bindFormMap(v, form, files, name)
197217
default:
198218
value := form[name]
199219
if len(value) != 0 {
@@ -286,6 +306,66 @@ func bindAdditionalProperties(additionalProperties reflect.Value, parentStruct r
286306
return hasData, nil
287307
}
288308

309+
// bindFormMap binds form (and file) entries of the form `name[key]=value`
310+
// into a generic map[K]V destination. It is reached from bindFormImpl for
311+
// non-bool-keyed maps; the bool-keyed case is handled separately as a
312+
// nullable wrapper.
313+
func bindFormMap(v reflect.Value, form map[string][]string, files map[string][]*multipart.FileHeader, name string) (bool, error) {
314+
keyType := v.Type().Key()
315+
valueType := v.Type().Elem()
316+
result := reflect.MakeMap(v.Type())
317+
hasData := false
318+
prefix := name + "["
319+
seen := map[string]struct{}{}
320+
321+
process := func(formKey string) error {
322+
if !strings.HasPrefix(formKey, prefix) {
323+
return nil
324+
}
325+
inner := strings.TrimPrefix(formKey, prefix)
326+
end := strings.Index(inner, "]")
327+
if end < 0 {
328+
return nil
329+
}
330+
innerKey := inner[:end]
331+
if _, ok := seen[innerKey]; ok {
332+
return nil
333+
}
334+
seen[innerKey] = struct{}{}
335+
336+
keyPtr := reflect.New(keyType)
337+
if err := BindStringToObject(innerKey, keyPtr.Interface()); err != nil {
338+
return err
339+
}
340+
valuePtr := reflect.New(valueType)
341+
innerHasData, err := bindFormImpl(valuePtr.Elem(), form, files, fmt.Sprintf("%s[%s]", name, innerKey))
342+
if err != nil {
343+
return err
344+
}
345+
if innerHasData {
346+
result.SetMapIndex(keyPtr.Elem(), valuePtr.Elem())
347+
hasData = true
348+
}
349+
return nil
350+
}
351+
352+
for k := range form {
353+
if err := process(k); err != nil {
354+
return false, err
355+
}
356+
}
357+
for k := range files {
358+
if err := process(k); err != nil {
359+
return false, err
360+
}
361+
}
362+
363+
if hasData {
364+
v.Set(result)
365+
}
366+
return hasData, nil
367+
}
368+
289369
func marshalFormImpl(v reflect.Value, result url.Values, name string) {
290370
switch v.Kind() {
291371
case reflect.Ptr:

bindform_test.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ import (
66
"net/url"
77
"testing"
88

9+
"github.com/oapi-codegen/nullable"
910
"github.com/oapi-codegen/runtime/types"
1011
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
1113
)
1214

1315
func TestBindURLForm(t *testing.T) {
@@ -65,6 +67,84 @@ func TestBindURLForm(t *testing.T) {
6567
}
6668
}
6769

70+
// TestBindURLFormNullable reproduces oapi-codegen/runtime#129: BindForm fails on
71+
// a nullable.Nullable[T] field because Nullable's underlying type is map[bool]T
72+
// and neither bindFormImpl nor BindStringToObjectWithOptions handles maps.
73+
func TestBindURLFormNullable(t *testing.T) {
74+
type testStruct struct {
75+
Name nullable.Nullable[string] `json:"name"`
76+
Age nullable.Nullable[int] `json:"age"`
77+
}
78+
79+
t.Run("string value is bound", func(t *testing.T) {
80+
values, err := url.ParseQuery("name=alice")
81+
require.NoError(t, err)
82+
var result testStruct
83+
err = BindForm(&result, values, nil, nil)
84+
require.NoError(t, err)
85+
got, err := result.Name.Get()
86+
require.NoError(t, err)
87+
assert.Equal(t, "alice", got)
88+
})
89+
90+
t.Run("int value is bound", func(t *testing.T) {
91+
values, err := url.ParseQuery("age=42")
92+
require.NoError(t, err)
93+
var result testStruct
94+
err = BindForm(&result, values, nil, nil)
95+
require.NoError(t, err)
96+
got, err := result.Age.Get()
97+
require.NoError(t, err)
98+
assert.Equal(t, 42, got)
99+
})
100+
101+
t.Run("missing field stays unspecified", func(t *testing.T) {
102+
var result testStruct
103+
err := BindForm(&result, url.Values{}, nil, nil)
104+
require.NoError(t, err)
105+
assert.False(t, result.Name.IsSpecified())
106+
assert.False(t, result.Age.IsSpecified())
107+
})
108+
}
109+
110+
// TestBindURLFormGenericMap covers binding into a non-Nullable map[K]V field
111+
// using the `name[key]=value` form encoding.
112+
func TestBindURLFormGenericMap(t *testing.T) {
113+
t.Run("string keys, int values", func(t *testing.T) {
114+
type testStruct struct {
115+
Attrs map[string]int `json:"attrs"`
116+
}
117+
values, err := url.ParseQuery("attrs[a]=1&attrs[b]=2")
118+
require.NoError(t, err)
119+
var result testStruct
120+
err = BindForm(&result, values, nil, nil)
121+
require.NoError(t, err)
122+
assert.Equal(t, map[string]int{"a": 1, "b": 2}, result.Attrs)
123+
})
124+
125+
t.Run("int keys, string values", func(t *testing.T) {
126+
type testStruct struct {
127+
Labels map[int]string `json:"labels"`
128+
}
129+
values, err := url.ParseQuery("labels[1]=one&labels[2]=two")
130+
require.NoError(t, err)
131+
var result testStruct
132+
err = BindForm(&result, values, nil, nil)
133+
require.NoError(t, err)
134+
assert.Equal(t, map[int]string{1: "one", 2: "two"}, result.Labels)
135+
})
136+
137+
t.Run("absent map stays nil", func(t *testing.T) {
138+
type testStruct struct {
139+
Attrs map[string]int `json:"attrs"`
140+
}
141+
var result testStruct
142+
err := BindForm(&result, url.Values{}, nil, nil)
143+
require.NoError(t, err)
144+
assert.Nil(t, result.Attrs)
145+
})
146+
}
147+
68148
func TestBindMultipartForm(t *testing.T) {
69149
var testStruct struct {
70150
File types.File `json:"file"`

bindstring.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,21 @@ func BindStringToObjectWithOptions(src string, dst interface{}, opts BindStringT
190190
// We fall through to the error case below if we haven't handled the
191191
// destination type above.
192192
fallthrough
193+
case reflect.Map:
194+
// A bool-keyed map (such as nullable.Nullable[T], which is
195+
// map[bool]T) is treated as a nullable wrapper: bind src into a
196+
// fresh value of the inner type and store it under map[true].
197+
if t.Kind() == reflect.Map && t.Key().Kind() == reflect.Bool {
198+
elemPtr := reflect.New(t.Elem())
199+
if bindErr := BindStringToObjectWithOptions(src, elemPtr.Interface(), opts); bindErr != nil {
200+
return bindErr
201+
}
202+
newMap := reflect.MakeMap(t)
203+
newMap.SetMapIndex(reflect.ValueOf(true), elemPtr.Elem())
204+
v.Set(newMap)
205+
return nil
206+
}
207+
fallthrough
193208
default:
194209
// We've got a bunch of types unimplemented, don't fail silently.
195210
err = fmt.Errorf("can not bind to destination of type: %s", t.Kind())

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ require (
88
github.com/google/uuid v1.6.0
99
github.com/kataras/iris/v12 v12.2.11
1010
github.com/labstack/echo/v4 v4.15.1
11+
github.com/oapi-codegen/nullable v1.1.0
1112
github.com/stretchr/testify v1.11.1
1213
)
1314

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9G
127127
github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk=
128128
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs=
129129
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
130+
github.com/oapi-codegen/nullable v1.1.0 h1:eAh8JVc5430VtYVnq00Hrbpag9PFRGWLjxR1/3KntMs=
131+
github.com/oapi-codegen/nullable v1.1.0/go.mod h1:KUZ3vUzkmEKY90ksAmit2+5juDIhIZhfDl+0PwOQlFY=
130132
github.com/pelletier/go-toml/v2 v2.2.2 h1:aYUidT7k73Pcl9nb2gScu7NSrKCSHIDE89b3+6Wq+LM=
131133
github.com/pelletier/go-toml/v2 v2.2.2/go.mod h1:1t835xjRzz80PqgE6HHgN2JOsmgYu/h4qDAS4n929Rs=
132134
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=

0 commit comments

Comments
 (0)