Skip to content

fix(binding): improve empty slice/array handling in form binding #4380

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
appleboy merged 1 commit into from
Oct 11, 2025
Merged

fix(binding): improve empty slice/array handling in form binding #4380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions binding/form_mapping.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,12 @@ func setByForm(value reflect.Value, field reflect.StructField, form map[string][

switch value.Kind() {
case reflect.Slice:
if !ok {
vs = []string{opt.defaultValue}
if len(vs) == 0 {
if !opt.isDefaultExists {
return false, nil
}
Comment on lines +233 to +236
Copy link

Copilot AI Sep 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition len(vs) == 0 will be true both when the field is not present in the form (ok == false) and when the field is present but has an empty slice. Consider adding a comment to clarify this intentional behavior change from the previous !ok check.

Copilot uses AI. Check for mistakes.

vs = []string{opt.defaultValue}
// pre-process the default value for multi if present
cfTag := field.Tag.Get("collection_format")
if cfTag == "" || cfTag == "multi" {
Expand All @@ -250,9 +253,12 @@ func setByForm(value reflect.Value, field reflect.StructField, form map[string][

return true, setSlice(vs, value, field)
case reflect.Array:
if !ok {
vs = []string{opt.defaultValue}
if len(vs) == 0 {
if !opt.isDefaultExists {
return false, nil
}
Comment on lines +256 to +259
Copy link

Copilot AI Sep 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition len(vs) == 0 will be true both when the field is not present in the form (ok == false) and when the field is present but has an empty slice. Consider adding a comment to clarify this intentional behavior change from the previous !ok check.

Copilot uses AI. Check for mistakes.

vs = []string{opt.defaultValue}
// pre-process the default value for multi if present
cfTag := field.Tag.Get("collection_format")
if cfTag == "" || cfTag == "multi" {
Expand Down
80 changes: 80 additions & 0 deletions binding/form_mapping_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,3 +635,83 @@ func TestMappingCustomArrayForm(t *testing.T) {
expected, _ := convertTo(val)
assert.Equal(t, expected, s.FileData)
}

func TestMappingEmptyValues(t *testing.T) {
t.Run("slice with default", func(t *testing.T) {
var s struct {
Slice []int `form:"slice,default=5"`
}

// field not present
err := mappingByPtr(&s, formSource{}, "form")
require.NoError(t, err)
assert.Equal(t, []int{5}, s.Slice)

// field present but empty
err = mappingByPtr(&s, formSource{"slice": {}}, "form")
require.NoError(t, err)
assert.Equal(t, []int{5}, s.Slice)

// field present with values
err = mappingByPtr(&s, formSource{"slice": {"1", "2", "3"}}, "form")
require.NoError(t, err)
assert.Equal(t, []int{1, 2, 3}, s.Slice)
})

t.Run("array with default", func(t *testing.T) {
var s struct {
Array [1]int `form:"array,default=5"`
}

// field not present
err := mappingByPtr(&s, formSource{}, "form")
require.NoError(t, err)
assert.Equal(t, [1]int{5}, s.Array)

// field present but empty
err = mappingByPtr(&s, formSource{"array": {}}, "form")
require.NoError(t, err)
assert.Equal(t, [1]int{5}, s.Array)
})

t.Run("slice without default", func(t *testing.T) {
var s struct {
Slice []int `form:"slice"`
}

// field present but empty
err := mappingByPtr(&s, formSource{"slice": {}}, "form")
require.NoError(t, err)
assert.Equal(t, []int(nil), s.Slice)
})

t.Run("array without default", func(t *testing.T) {
var s struct {
Array [1]int `form:"array"`
}

// field present but empty
err := mappingByPtr(&s, formSource{"array": {}}, "form")
require.NoError(t, err)
assert.Equal(t, [1]int{0}, s.Array)
})

t.Run("slice with collection format", func(t *testing.T) {
var s struct {
SliceMulti []int `form:"slice_multi,default=1;2;3" collection_format:"multi"`
SliceCsv []int `form:"slice_csv,default=1;2;3" collection_format:"csv"`
}

// field not present
err := mappingByPtr(&s, formSource{}, "form")
require.NoError(t, err)
assert.Equal(t, []int{1, 2, 3}, s.SliceMulti)
assert.Equal(t, []int{1, 2, 3}, s.SliceCsv)

// field present but empty
err = mappingByPtr(&s, formSource{"slice_multi": {}, "slice_csv": {}}, "form")
require.NoError(t, err)
assert.Equal(t, []int{1, 2, 3}, s.SliceMulti)
assert.Equal(t, []int{1, 2, 3}, s.SliceCsv)
})
}