From cdc534f7224add4be08528c01d4b6e8db096e30a Mon Sep 17 00:00:00 2001 From: Dmitry Kutakov Date: Tue, 26 Mar 2019 20:55:30 +0300 Subject: [PATCH 1/4] refactor(form_mapping.go): mapping multipart request --- binding/form.go | 31 +++++++++++++--- binding/form_mapping.go | 80 +++++++++++++++++++---------------------- 2 files changed, 63 insertions(+), 48 deletions(-) diff --git a/binding/form.go b/binding/form.go index f1f89195d9..eef0cc1381 100644 --- a/binding/form.go +++ b/binding/form.go @@ -4,7 +4,11 @@ package binding -import "net/http" +import ( + "mime/multipart" + "net/http" + "reflect" +) const defaultMemory = 32 * 1024 * 1024 @@ -53,13 +57,30 @@ func (formMultipartBinding) Bind(req *http.Request, obj interface{}) error { if err := req.ParseMultipartForm(defaultMemory); err != nil { return err } - if err := mapForm(obj, req.MultipartForm.Value); err != nil { + if err := mappingByPtr(obj, (*multipartRequest)(req), "form"); err != nil { return err } - if err := mapFiles(obj, req); err != nil { - return err + return validate(obj) +} + +type multipartRequest http.Request + +var ( + multipartFileHeaderStructType = reflect.TypeOf(multipart.FileHeader{}) +) + +func (r *multipartRequest) Set(value reflect.Value, field reflect.StructField, key string, opt setOptions) (isSetted bool, err error) { + if value.Type() == multipartFileHeaderStructType { + _, file, err := (*http.Request)(r).FormFile(key) + if err != nil { + return false, err + } + if file != nil { + value.Set(reflect.ValueOf(*file)) + return true, nil + } } - return validate(obj) + return setByForm(value, field, r.MultipartForm.Value, key, opt) } diff --git a/binding/form_mapping.go b/binding/form_mapping.go index fc33b1df00..85eb4ce578 100644 --- a/binding/form_mapping.go +++ b/binding/form_mapping.go @@ -7,7 +7,6 @@ package binding import ( "errors" "fmt" - "net/http" "reflect" "strconv" "strings" @@ -16,34 +15,6 @@ import ( "github.com/gin-gonic/gin/internal/json" ) -func mapFiles(ptr interface{}, req *http.Request) error { - typ := reflect.TypeOf(ptr).Elem() - val := reflect.ValueOf(ptr).Elem() - for i := 0; i < typ.NumField(); i++ { - typeField := typ.Field(i) - structField := val.Field(i) - - t := fmt.Sprintf("%s", typeField.Type) - if string(t) != "*multipart.FileHeader" { - continue - } - - inputFieldName := typeField.Tag.Get("form") - if inputFieldName == "" { - inputFieldName = typeField.Name - } - - _, fileHeader, err := req.FormFile(inputFieldName) - if err != nil { - return err - } - - structField.Set(reflect.ValueOf(fileHeader)) - - } - return nil -} - var errUnknownType = errors.New("Unknown type") func mapUri(ptr interface{}, m map[string][]string) error { @@ -57,11 +28,25 @@ func mapForm(ptr interface{}, form map[string][]string) error { var emptyField = reflect.StructField{} func mapFormByTag(ptr interface{}, form map[string][]string, tag string) error { - _, err := mapping(reflect.ValueOf(ptr), emptyField, form, tag) + return mappingByPtr(ptr, formSource(form), tag) +} + +type setter interface { + Set(value reflect.Value, field reflect.StructField, key string, opt setOptions) (isSetted bool, err error) +} + +type formSource map[string][]string + +func (form formSource) Set(value reflect.Value, field reflect.StructField, tagValue string, opt setOptions) (isSetted bool, err error) { + return setByForm(value, field, form, tagValue, opt) +} + +func mappingByPtr(ptr interface{}, setter setter, tag string) error { + _, err := mapping(reflect.ValueOf(ptr), emptyField, setter, tag) return err } -func mapping(value reflect.Value, field reflect.StructField, form map[string][]string, tag string) (bool, error) { +func mapping(value reflect.Value, field reflect.StructField, setter setter, tag string) (bool, error) { var vKind = value.Kind() if vKind == reflect.Ptr { @@ -71,7 +56,7 @@ func mapping(value reflect.Value, field reflect.StructField, form map[string][]s isNew = true vPtr = reflect.New(value.Type().Elem()) } - isSetted, err := mapping(vPtr.Elem(), field, form, tag) + isSetted, err := mapping(vPtr.Elem(), field, setter, tag) if err != nil { return false, err } @@ -81,7 +66,7 @@ func mapping(value reflect.Value, field reflect.StructField, form map[string][]s return isSetted, nil } - ok, err := tryToSetValue(value, field, form, tag) + ok, err := tryToSetValue(value, field, setter, tag) if err != nil { return false, err } @@ -97,7 +82,7 @@ func mapping(value reflect.Value, field reflect.StructField, form map[string][]s if !value.Field(i).CanSet() { continue } - ok, err := mapping(value.Field(i), tValue.Field(i), form, tag) + ok, err := mapping(value.Field(i), tValue.Field(i), setter, tag) if err != nil { return false, err } @@ -108,9 +93,14 @@ func mapping(value reflect.Value, field reflect.StructField, form map[string][]s return false, nil } -func tryToSetValue(value reflect.Value, field reflect.StructField, form map[string][]string, tag string) (bool, error) { - var tagValue, defaultValue string - var isDefaultExists bool +type setOptions struct { + isDefaultExists bool + defaultValue string +} + +func tryToSetValue(value reflect.Value, field reflect.StructField, setter setter, tag string) (bool, error) { + var tagValue string + var setOpt setOptions tagValue = field.Tag.Get(tag) tagValue, opts := head(tagValue, ",") @@ -132,25 +122,29 @@ func tryToSetValue(value reflect.Value, field reflect.StructField, form map[stri k, v := head(opt, "=") switch k { case "default": - isDefaultExists = true - defaultValue = v + setOpt.isDefaultExists = true + setOpt.defaultValue = v } } + return setter.Set(value, field, tagValue, setOpt) +} + +func setByForm(value reflect.Value, field reflect.StructField, form map[string][]string, tagValue string, opt setOptions) (isSetted bool, err error) { vs, ok := form[tagValue] - if !ok && !isDefaultExists { + if !ok && !opt.isDefaultExists { return false, nil } switch value.Kind() { case reflect.Slice: if !ok { - vs = []string{defaultValue} + vs = []string{opt.defaultValue} } return true, setSlice(vs, value, field) case reflect.Array: if !ok { - vs = []string{defaultValue} + vs = []string{opt.defaultValue} } if len(vs) != value.Len() { return false, fmt.Errorf("%q is not valid value for %s", vs, value.Type().String()) @@ -159,7 +153,7 @@ func tryToSetValue(value reflect.Value, field reflect.StructField, form map[stri default: var val string if !ok { - val = defaultValue + val = opt.defaultValue } if len(vs) > 0 { From adff101d264e0ea57f70fa972c9b38a06d7d6e53 Mon Sep 17 00:00:00 2001 From: Dmitry Kutakov Date: Sat, 30 Mar 2019 16:59:42 +0300 Subject: [PATCH 2/4] add checkers for a types to match with the setter interface --- binding/form.go | 2 ++ binding/form_mapping.go | 3 +++ 2 files changed, 5 insertions(+) diff --git a/binding/form.go b/binding/form.go index eef0cc1381..09087ae02e 100644 --- a/binding/form.go +++ b/binding/form.go @@ -66,6 +66,8 @@ func (formMultipartBinding) Bind(req *http.Request, obj interface{}) error { type multipartRequest http.Request +var _ setter = (*multipartRequest)(nil) + var ( multipartFileHeaderStructType = reflect.TypeOf(multipart.FileHeader{}) ) diff --git a/binding/form_mapping.go b/binding/form_mapping.go index 85eb4ce578..615e89ffb1 100644 --- a/binding/form_mapping.go +++ b/binding/form_mapping.go @@ -31,12 +31,15 @@ func mapFormByTag(ptr interface{}, form map[string][]string, tag string) error { return mappingByPtr(ptr, formSource(form), tag) } +// setter - try to set value on a walking by fields of a struct type setter interface { Set(value reflect.Value, field reflect.StructField, key string, opt setOptions) (isSetted bool, err error) } type formSource map[string][]string +var _ setter = formSource(nil) + func (form formSource) Set(value reflect.Value, field reflect.StructField, tagValue string, opt setOptions) (isSetted bool, err error) { return setByForm(value, field, form, tagValue, opt) } From 6c5e1da5ad23ff804699cc7e4ddcaeca94067f00 Mon Sep 17 00:00:00 2001 From: Dmitry Kutakov Date: Mon, 1 Apr 2019 10:47:20 +0300 Subject: [PATCH 3/4] form_mapping.go: rename method name on setter interface, add comments --- binding/form.go | 3 ++- binding/form_mapping.go | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/binding/form.go b/binding/form.go index 09087ae02e..843a989364 100644 --- a/binding/form.go +++ b/binding/form.go @@ -72,7 +72,8 @@ var ( multipartFileHeaderStructType = reflect.TypeOf(multipart.FileHeader{}) ) -func (r *multipartRequest) Set(value reflect.Value, field reflect.StructField, key string, opt setOptions) (isSetted bool, err error) { +// TrySet - try to set a value by the multipart request with the binding a form file +func (r *multipartRequest) TrySet(value reflect.Value, field reflect.StructField, key string, opt setOptions) (isSetted bool, err error) { if value.Type() == multipartFileHeaderStructType { _, file, err := (*http.Request)(r).FormFile(key) if err != nil { diff --git a/binding/form_mapping.go b/binding/form_mapping.go index 615e89ffb1..0596618acc 100644 --- a/binding/form_mapping.go +++ b/binding/form_mapping.go @@ -33,14 +33,15 @@ func mapFormByTag(ptr interface{}, form map[string][]string, tag string) error { // setter - try to set value on a walking by fields of a struct type setter interface { - Set(value reflect.Value, field reflect.StructField, key string, opt setOptions) (isSetted bool, err error) + TrySet(value reflect.Value, field reflect.StructField, key string, opt setOptions) (isSetted bool, err error) } type formSource map[string][]string var _ setter = formSource(nil) -func (form formSource) Set(value reflect.Value, field reflect.StructField, tagValue string, opt setOptions) (isSetted bool, err error) { +// TrySet - try to set a value by request's form source (like map[string][]string) +func (form formSource) TrySet(value reflect.Value, field reflect.StructField, tagValue string, opt setOptions) (isSetted bool, err error) { return setByForm(value, field, form, tagValue, opt) } @@ -130,7 +131,7 @@ func tryToSetValue(value reflect.Value, field reflect.StructField, setter setter } } - return setter.Set(value, field, tagValue, setOpt) + return setter.TrySet(value, field, tagValue, setOpt) } func setByForm(value reflect.Value, field reflect.StructField, form map[string][]string, tagValue string, opt setOptions) (isSetted bool, err error) { From fe96cf6a0d0d1eef205accec1e634d3a0f651dd1 Mon Sep 17 00:00:00 2001 From: Dmitry Kutakov Date: Mon, 1 Apr 2019 12:43:28 +0300 Subject: [PATCH 4/4] fix style of comments --- binding/form.go | 2 +- binding/form_mapping.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/binding/form.go b/binding/form.go index 843a989364..0b28aa8a80 100644 --- a/binding/form.go +++ b/binding/form.go @@ -72,7 +72,7 @@ var ( multipartFileHeaderStructType = reflect.TypeOf(multipart.FileHeader{}) ) -// TrySet - try to set a value by the multipart request with the binding a form file +// TrySet tries to set a value by the multipart request with the binding a form file func (r *multipartRequest) TrySet(value reflect.Value, field reflect.StructField, key string, opt setOptions) (isSetted bool, err error) { if value.Type() == multipartFileHeaderStructType { _, file, err := (*http.Request)(r).FormFile(key) diff --git a/binding/form_mapping.go b/binding/form_mapping.go index 0596618acc..aaacf6c535 100644 --- a/binding/form_mapping.go +++ b/binding/form_mapping.go @@ -31,7 +31,7 @@ func mapFormByTag(ptr interface{}, form map[string][]string, tag string) error { return mappingByPtr(ptr, formSource(form), tag) } -// setter - try to set value on a walking by fields of a struct +// setter tries to set value on a walking by fields of a struct type setter interface { TrySet(value reflect.Value, field reflect.StructField, key string, opt setOptions) (isSetted bool, err error) } @@ -40,7 +40,7 @@ type formSource map[string][]string var _ setter = formSource(nil) -// TrySet - try to set a value by request's form source (like map[string][]string) +// TrySet tries to set a value by request's form source (like map[string][]string) func (form formSource) TrySet(value reflect.Value, field reflect.StructField, tagValue string, opt setOptions) (isSetted bool, err error) { return setByForm(value, field, form, tagValue, opt) }