Skip to content
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

Array collection format in form binding #2750

Closed
wants to merge 2 commits into from
+106 −7
Closed

Array collection format in form binding #2750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ddcc2ed
Array collection format in form binding
Simon-Saaw Jun 13, 2021
e3f19a5
Merge branch 'master' of https://github.com/gin-gonic/gin
Simon-Saaw Oct 28, 2021
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
23 changes: 16 additions & 7 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -878,6 +878,15 @@ func startPage(c *gin.Context) {

See the [detail information](https://github.com/gin-gonic/gin/issues/742#issuecomment-264681292).

#### Collection format for arrays

| Format | Description | Example |
| --------------- | --------------------------------------------------------- | ----------------------- |
| multi (default) | Multiple parameter instances rather than multiple values. | key=foo&key=bar&key=baz |
| csv | Comma-separated values. | foo,bar,baz |
| ssv | Space-separated values. | foo bar baz |
| pipes | Pipe-separated values. | foo\|bar\|baz |

```go
package main

Expand All @@ -889,11 +898,11 @@ import (
)

type Person struct {
Name string `form:"name"`
Address string `form:"address"`
Birthday time.Time `form:"birthday" time_format:"2006-01-02" time_utc:"1"`
CreateTime time.Time `form:"createTime" time_format:"unixNano"`
UnixTime time.Time `form:"unixTime" time_format:"unix"`
Name string `form:"name"`
Addresses []string `form:"addresses" collection_format:"csv"`
Birthday time.Time `form:"birthday" time_format:"2006-01-02" time_utc:"1"`
CreateTime time.Time `form:"createTime" time_format:"unixNano"`
UnixTime time.Time `form:"unixTime" time_format:"unix"`
}

func main() {
Expand All @@ -909,7 +918,7 @@ func startPage(c *gin.Context) {
// See more at https://github.com/gin-gonic/gin/blob/master/binding/binding.go#L48
if c.ShouldBind(&person) == nil {
log.Println(person.Name)
log.Println(person.Address)
log.Println(person.Addresses)
log.Println(person.Birthday)
log.Println(person.CreateTime)
log.Println(person.UnixTime)
Expand All @@ -921,7 +930,7 @@ func startPage(c *gin.Context) {

Test it with:
```sh
$ curl -X GET "localhost:8085/testing?name=appleboy&address=xyz&birthday=1992-03-15&createTime=1562400033000000123&unixTime=1562400033"
$ curl -X GET "localhost:8085/testing?name=appleboy&addresses=foo,bar&birthday=1992-03-15&createTime=1562400033000000123&unixTime=1562400033"
```

### Bind Uri
Expand Down
34 changes: 34 additions & 0 deletions binding/binding_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,13 @@ type FooStructForSliceType struct {
SliceFoo []int `form:"slice_foo"`
}

type FooStructForCollectionFormatTag struct {
SliceMulti []int `form:"slice_multi" collection_format:"multi"`
SliceCsv []int `form:"slice_csv" collection_format:"csv"`
SliceSsv []int `form:"slice_ssv" collection_format:"ssv"`
SlicePipes []int `form:"slice_pipes" collection_format:"pipes"`
}

type FooStructForStructType struct {
StructFoo struct {
Idx int `form:"idx"`
Expand Down Expand Up @@ -311,6 +318,15 @@ func TestBindingFormInvalidName2(t *testing.T) {
"map_foo=bar", "bar2=foo")
}

func TestBindingFormCollectionFormat(t *testing.T) {
testFormBindingForCollectionFormat(t, "POST",
Copy link

Choose a reason for hiding this comment

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

Example in README.md shows curl with GET but test only tests POST.

"/?slice_multi=1&slice_multi=2&slice_csv=1,2&slice_ssv=1 2&slice_pipes=1|2", "/",
Copy link

Choose a reason for hiding this comment

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

Missing fuzzy tests here. Should try it with string type and see what happens if we feed pipes to csv param, csv to pipe param, csv to multi param etc…

"", "")
testFormBindingForCollectionFormat(t, "POST",
"/", "/",
"slice_multi=1&slice_multi=2&slice_csv=1,2&slice_ssv=1 2&slice_pipes=1|2", "")
}

func TestBindingFormForType(t *testing.T) {
testFormBindingForType(t, "POST",
"/", "/",
Expand Down Expand Up @@ -1064,6 +1080,24 @@ func testFormBindingInvalidName2(t *testing.T, method, path, badPath, body, badB
assert.Error(t, err)
}

func testFormBindingForCollectionFormat(t *testing.T, method, path, badPath, body, badBody string) {
b := Form
assert.Equal(t, "form", b.Name())

obj := FooStructForCollectionFormatTag{}
req := requestWithBody(method, path, body)
if method == "POST" {
req.Header.Add("Content-Type", MIMEPOSTForm)
}
err := b.Bind(req, &obj)
assert.NoError(t, err)

assert.Equal(t, []int{1, 2}, obj.SliceMulti)
Copy link

Choose a reason for hiding this comment

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

I'd refrain from using values 1 and 2 for each one of them. It can potentially conceal misbehaviour.

assert.Equal(t, []int{1, 2}, obj.SliceCsv)
assert.Equal(t, []int{1, 2}, obj.SliceCsv)
assert.Equal(t, []int{1, 2}, obj.SlicePipes)
}

func testFormBindingForType(t *testing.T, method, path, badPath, body, badBody string, typ string) {
b := Form
assert.Equal(t, "form", b.Name())
Expand Down
23 changes: 23 additions & 0 deletions binding/form_mapping.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,15 @@ func setByForm(value reflect.Value, field reflect.StructField, form map[string][
case reflect.Slice:
if !ok {
vs = []string{opt.defaultValue}
} else {
vs = split(vs, field)
}
return true, setSlice(vs, value, field)
case reflect.Array:
if !ok {
vs = []string{opt.defaultValue}
} else {
vs = split(vs, field)
}
if len(vs) != value.Len() {
return false, fmt.Errorf("%q is not valid value for %s", vs, value.Type().String())
Expand Down Expand Up @@ -376,6 +380,25 @@ func head(str, sep string) (head string, tail string) {
return str[:idx], str[idx+len(sep):]
}

func split(vals []string, field reflect.StructField) []string {
if cfTag := field.Tag.Get("collection_format"); cfTag != "" {
sep := "multi"
Copy link

Choose a reason for hiding this comment

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

It will stay as sep = "multi" when invalid separator is defined e.g. collection_format:"foo"
Should it fail / warn instead silently doing fallback?
Should also add test case for it.

switch cfTag {
case "csv":
sep = ","
case "ssv":
sep = " "
case "pipes":
sep = "|"
}

if sep != "multi" && len(vals) == 1 {
return strings.Split(vals[0], sep)
}
}
return vals
}

func setFormMap(ptr interface{}, form map[string][]string) error {
el := reflect.TypeOf(ptr).Elem()

Expand Down
33 changes: 33 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 @@ -74,6 +74,39 @@ func TestMappingDefault(t *testing.T) {
assert.Equal(t, [1]int{9}, s.Array)
}

func TestMappingCollectionFormat(t *testing.T) {
var s struct {
SliceMulti []int `form:"slice_multi" collection_format:"multi"`
SliceCsv []int `form:"slice_csv" collection_format:"csv"`
SliceSsv []int `form:"slice_ssv" collection_format:"ssv"`
SlicePipes []int `form:"slice_pipes" collection_format:"pipes"`
ArrayMulti [2]int `form:"array_multi" collection_format:"multi"`
ArrayCsv [2]int `form:"array_csv" collection_format:"csv"`
ArraySsv [2]int `form:"array_ssv" collection_format:"ssv"`
ArrayPipes [2]int `form:"array_pipes" collection_format:"pipes"`
}
err := mappingByPtr(&s, formSource{
"slice_multi": {"1", "2"},
"slice_csv": {"1,2"},
"slice_ssv": {"1 2"},
"slice_pipes": {"1|2"},
"array_multi": {"1", "2"},
"array_csv": {"1,2"},
"array_ssv": {"1 2"},
"array_pipes": {"1|2"},
}, "form")
assert.NoError(t, err)

assert.Equal(t, []int{1, 2}, s.SliceMulti)
assert.Equal(t, []int{1, 2}, s.SliceCsv)
assert.Equal(t, []int{1, 2}, s.SliceSsv)
assert.Equal(t, []int{1, 2}, s.SlicePipes)
assert.Equal(t, [2]int{1, 2}, s.ArrayMulti)
assert.Equal(t, [2]int{1, 2}, s.ArrayCsv)
assert.Equal(t, [2]int{1, 2}, s.ArraySsv)
assert.Equal(t, [2]int{1, 2}, s.ArrayPipes)
}

func TestMappingSkipField(t *testing.T) {
var s struct {
A int
Expand Down