From 59941167ea94c7fcdc18481fd12cb0fe77befa36 Mon Sep 17 00:00:00 2001 From: John Guo Date: Wed, 26 Mar 2025 21:26:09 +0800 Subject: [PATCH 1/4] fix(net/ghttp): panic when post empty string parameter to *ghttp.UploadFile --- util/gconv/gconv_scan.go | 2 +- util/gconv/gconv_z_unit_map_test.go | 1 + util/gconv/gconv_z_unit_struct_test.go | 1 - .../internal/converter/converter_convert.go | 8 ++++++-- .../internal/converter/converter_float.go | 18 ++++++++++++++++-- util/gconv/internal/converter/converter_map.go | 15 +++++++++++++++ .../internal/converter/converter_struct.go | 10 ++++++---- 7 files changed, 45 insertions(+), 10 deletions(-) diff --git a/util/gconv/gconv_scan.go b/util/gconv/gconv_scan.go index 8d8724208..da3fcc431 100644 --- a/util/gconv/gconv_scan.go +++ b/util/gconv/gconv_scan.go @@ -18,7 +18,7 @@ package gconv // TODO: change `paramKeyToAttrMap` to `ScanOption` to be more scalable; add `DeepCopy` option for `ScanOption`. func Scan(srcValue any, dstPointer any, paramKeyToAttrMap ...map[string]string) (err error) { option := ScanOption{ - ContinueOnError: true, + ContinueOnError: false, } if len(paramKeyToAttrMap) > 0 { option.ParamKeyToAttrMap = paramKeyToAttrMap[0] diff --git a/util/gconv/gconv_z_unit_map_test.go b/util/gconv/gconv_z_unit_map_test.go index ac30b0ae5..9a6477f83 100644 --- a/util/gconv/gconv_z_unit_map_test.go +++ b/util/gconv/gconv_z_unit_map_test.go @@ -164,6 +164,7 @@ var mapTests = []struct { func TestMap(t *testing.T) { gtest.C(t, func(t *gtest.T) { for _, test := range mapTests { + // t.Log(test.value, test.expect) t.Assert(gconv.Map(test.value), test.expect) } }) diff --git a/util/gconv/gconv_z_unit_struct_test.go b/util/gconv/gconv_z_unit_struct_test.go index 1dcb190c2..89cfe4e78 100644 --- a/util/gconv/gconv_z_unit_struct_test.go +++ b/util/gconv/gconv_z_unit_struct_test.go @@ -225,7 +225,6 @@ func TestStructErr(t *testing.T) { type User struct { Score Score } - user := new(User) scores := map[string]interface{}{ "Score": 1, diff --git a/util/gconv/internal/converter/converter_convert.go b/util/gconv/internal/converter/converter_convert.go index 33ea78c28..bd0c19863 100644 --- a/util/gconv/internal/converter/converter_convert.go +++ b/util/gconv/internal/converter/converter_convert.go @@ -395,7 +395,11 @@ func (c *Converter) doConvertForDefault(in doConvertInput, option ConvertOption) } // custom converter. - dstReflectValue, ok, err := c.callCustomConverterWithRefer(fromReflectValue, referReflectValue) + var ( + ok bool + dstReflectValue reflect.Value + ) + dstReflectValue, ok, err = c.callCustomConverterWithRefer(fromReflectValue, referReflectValue) if err != nil { return nil, err } @@ -415,7 +419,7 @@ func (c *Converter) doConvertForDefault(in doConvertInput, option ConvertOption) switch referReflectValue.Kind() { case reflect.Ptr: // Type converting for custom type pointers. - // Eg: + // Example: // type PayMode int // type Req struct{ // Mode *PayMode diff --git a/util/gconv/internal/converter/converter_float.go b/util/gconv/internal/converter/converter_float.go index 21418100d..bd54af2c2 100644 --- a/util/gconv/internal/converter/converter_float.go +++ b/util/gconv/internal/converter/converter_float.go @@ -45,7 +45,11 @@ func (c *Converter) Float32(any any) (float32, error) { } return 0, nil case reflect.String: - f, err := strconv.ParseFloat(rv.String(), 32) + s := rv.String() + if s == "" { + return 0, nil + } + f, err := strconv.ParseFloat(s, 32) if err != nil { return 0, gerror.WrapCodef( gcode.CodeInvalidParameter, err, "converting string to float32 failed for: %v", any, @@ -68,6 +72,9 @@ func (c *Converter) Float32(any any) (float32, error) { if err != nil { return 0, err } + if s == "" { + return 0, nil + } v, err := strconv.ParseFloat(s, 32) if err != nil { return 0, gerror.WrapCodef( @@ -112,7 +119,11 @@ func (c *Converter) Float64(any any) (float64, error) { } return 0, nil case reflect.String: - f, err := strconv.ParseFloat(rv.String(), 64) + s := rv.String() + if s == "" { + return 0, nil + } + f, err := strconv.ParseFloat(s, 64) if err != nil { return 0, gerror.WrapCodef( gcode.CodeInvalidParameter, err, "converting string to float64 failed for: %v", any, @@ -135,6 +146,9 @@ func (c *Converter) Float64(any any) (float64, error) { if err != nil { return 0, err } + if s == "" { + return 0, nil + } v, err := strconv.ParseFloat(s, 64) if err != nil { return 0, gerror.WrapCodef( diff --git a/util/gconv/internal/converter/converter_map.go b/util/gconv/internal/converter/converter_map.go index aff383938..e19101e22 100644 --- a/util/gconv/internal/converter/converter_map.go +++ b/util/gconv/internal/converter/converter_map.go @@ -88,6 +88,9 @@ func (c *Converter) doMapConvert( value any, recursive RecursiveType, mustMapReturn bool, option MapOption, ) (map[string]any, error) { if value == nil { + if mustMapReturn { + return map[string]any{}, nil + } return nil, nil } // It redirects to its underlying value if it has implemented interface iVal. @@ -119,6 +122,9 @@ func (c *Converter) doMapConvert( return nil, err } } else { + if len(r) == 0 && mustMapReturn { + return map[string]any{}, nil + } return nil, nil } case []byte: @@ -128,6 +134,9 @@ func (c *Converter) doMapConvert( return nil, err } } else { + if len(r) == 0 && mustMapReturn { + return map[string]any{}, nil + } return nil, nil } case map[interface{}]interface{}: @@ -327,8 +336,14 @@ func (c *Converter) doMapConvert( if m, ok := convertedValue.(map[string]interface{}); ok { return m, nil } + if mustMapReturn { + return map[string]any{}, nil + } return nil, nil default: + if mustMapReturn { + return map[string]any{}, nil + } return nil, nil } } diff --git a/util/gconv/internal/converter/converter_struct.go b/util/gconv/internal/converter/converter_struct.go index 1a84e41b6..f5b324201 100644 --- a/util/gconv/internal/converter/converter_struct.go +++ b/util/gconv/internal/converter/converter_struct.go @@ -160,10 +160,11 @@ func (c *Converter) Struct(params, pointer any, option ...StructOption) (err err return err } if paramsMap == nil { + // fails converting params to map, it so cannot be converted to struct pointer. return gerror.NewCodef( gcode.CodeInvalidParameter, - `convert params from "%#v" to "map[string]any" failed`, - params, + `convert params "%v" to "%s" failed`, + params, pointerReflectValue.Type(), ) } } @@ -505,8 +506,7 @@ func (c *Converter) bindVarToReflectValue(structFieldValue reflect.Value, value case reflect.Struct: // Recursively converting for struct attribute. if err = c.Struct(value, structFieldValue, option); err != nil { - // Note there's reflect conversion mechanism here. - structFieldValue.Set(reflect.ValueOf(value).Convert(structFieldValue.Type())) + return err } // Note that the slice element might be type of struct, @@ -637,6 +637,8 @@ func (c *Converter) bindVarToReflectValue(structFieldValue reflect.Value, value elem := item.Elem() if err = c.bindVarToReflectValue(elem, value, option); err == nil { structFieldValue.Set(elem.Addr()) + } else { + return err } } else { // Not empty pointer, it assigns values to it. From a9953a472962c9e1988b8a1f879a95721ceef2ef Mon Sep 17 00:00:00 2001 From: John Guo Date: Wed, 26 Mar 2025 21:32:56 +0800 Subject: [PATCH 2/4] up --- net/ghttp/ghttp_z_unit_issue_test.go | 30 +++++++++++++++++++ .../gconv/internal/converter/converter_map.go | 2 ++ 2 files changed, 32 insertions(+) diff --git a/net/ghttp/ghttp_z_unit_issue_test.go b/net/ghttp/ghttp_z_unit_issue_test.go index 92f761ce9..ee21a906e 100644 --- a/net/ghttp/ghttp_z_unit_issue_test.go +++ b/net/ghttp/ghttp_z_unit_issue_test.go @@ -726,3 +726,33 @@ func Test_Issue4093(t *testing.T) { t.Assert(client.PostContent(ctx, "/test"), `{"page":1,"pageSize":10,"pagination":true,"name":"john","number":1}`) }) } + +// https://github.com/gogf/gf/issues/4193 +func Test_Issue4193(t *testing.T) { + type Req struct { + g.Meta `method:"post" mime:"multipart/form-data"` + File *ghttp.UploadFile `v:"required" type:"file"` // File is required + } + type Res struct{} + + s := g.Server(guid.S()) + s.BindMiddlewareDefault(ghttp.MiddlewareHandlerResponse) + s.BindHandler("/upload", func(ctx context.Context, req *Req) (res *Res, err error) { + return + }) + s.SetDumpRouterMap(false) + s.SetAccessLogEnabled(false) + s.SetErrorLogEnabled(false) + s.Start() + defer s.Shutdown() + time.Sleep(100 * time.Millisecond) + + gtest.C(t, func(t *gtest.T) { + client := g.Client() + client.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort())) + content := client.PostContent(ctx, "/upload", g.Map{ + "file": "", + }) + t.Assert(content, `{"code":51,"message":"The File field is required","data":null}`) + }) +} diff --git a/util/gconv/internal/converter/converter_map.go b/util/gconv/internal/converter/converter_map.go index e19101e22..bc0834c7d 100644 --- a/util/gconv/internal/converter/converter_map.go +++ b/util/gconv/internal/converter/converter_map.go @@ -125,6 +125,7 @@ func (c *Converter) doMapConvert( if len(r) == 0 && mustMapReturn { return map[string]any{}, nil } + // if r is not empty, which means it fails converting to map. return nil, nil } case []byte: @@ -137,6 +138,7 @@ func (c *Converter) doMapConvert( if len(r) == 0 && mustMapReturn { return map[string]any{}, nil } + // if r is not empty, which means it fails converting to map. return nil, nil } case map[interface{}]interface{}: From c9939fcf7fb23e70f4539445d00fc6b1ebf805ab Mon Sep 17 00:00:00 2001 From: John Guo Date: Wed, 26 Mar 2025 22:27:48 +0800 Subject: [PATCH 3/4] up --- util/gconv/internal/converter/converter_map.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/util/gconv/internal/converter/converter_map.go b/util/gconv/internal/converter/converter_map.go index bc0834c7d..46690c2a6 100644 --- a/util/gconv/internal/converter/converter_map.go +++ b/util/gconv/internal/converter/converter_map.go @@ -338,14 +338,9 @@ func (c *Converter) doMapConvert( if m, ok := convertedValue.(map[string]interface{}); ok { return m, nil } - if mustMapReturn { - return map[string]any{}, nil - } return nil, nil + default: - if mustMapReturn { - return map[string]any{}, nil - } return nil, nil } } From 53f6697d4b2722a757f0684f23c01fd4fb6a63dd Mon Sep 17 00:00:00 2001 From: hailaz <739476267@qq.com> Date: Thu, 27 Mar 2025 10:41:12 +0800 Subject: [PATCH 4/4] =?UTF-8?q?style:=20=E7=A7=BB=E9=99=A4=E9=9D=9E?= =?UTF-8?q?=E5=BF=85=E8=A6=81=E4=BF=AE=E6=94=B9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- util/gconv/gconv_z_unit_map_test.go | 1 - util/gconv/gconv_z_unit_struct_test.go | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/util/gconv/gconv_z_unit_map_test.go b/util/gconv/gconv_z_unit_map_test.go index 9a6477f83..ac30b0ae5 100644 --- a/util/gconv/gconv_z_unit_map_test.go +++ b/util/gconv/gconv_z_unit_map_test.go @@ -164,7 +164,6 @@ var mapTests = []struct { func TestMap(t *testing.T) { gtest.C(t, func(t *gtest.T) { for _, test := range mapTests { - // t.Log(test.value, test.expect) t.Assert(gconv.Map(test.value), test.expect) } }) diff --git a/util/gconv/gconv_z_unit_struct_test.go b/util/gconv/gconv_z_unit_struct_test.go index 89cfe4e78..1dcb190c2 100644 --- a/util/gconv/gconv_z_unit_struct_test.go +++ b/util/gconv/gconv_z_unit_struct_test.go @@ -225,6 +225,7 @@ func TestStructErr(t *testing.T) { type User struct { Score Score } + user := new(User) scores := map[string]interface{}{ "Score": 1,