From d868810539557c189315fb5b349d824cce39ff67 Mon Sep 17 00:00:00 2001 From: iksars <211250186@smail.nju.edu.cn> Date: Thu, 1 Feb 2024 18:19:47 +0800 Subject: [PATCH 1/3] fix:Add required check for customed type binding \n Required tag assocated with customed type is invalid in previous version. This pr solves this problem by adding required check for customed type binding. What's more, I find that custom type should not only use json tag, because it's useless. This should be pointed out in the document. \n Closes #1007 --- pkg/app/server/binding/binder_test.go | 68 +++++++++++++++++++ .../decoder/customized_type_decoder.go | 9 +++ 2 files changed, 77 insertions(+) diff --git a/pkg/app/server/binding/binder_test.go b/pkg/app/server/binding/binder_test.go index 9071dbc85..c1f973130 100644 --- a/pkg/app/server/binding/binder_test.go +++ b/pkg/app/server/binding/binder_test.go @@ -580,6 +580,74 @@ func TestBind_CustomizedTypeDecodeForPanic(t *testing.T) { }) } +func TestBind_Required_CustomizedTypeDecode1(t *testing.T) { + type Foo struct { + F ***CustomizedDecode `query:"a,required"` + } + + bindConfig := &BindConfig{} + err := bindConfig.RegTypeUnmarshal(reflect.TypeOf(CustomizedDecode{}), func(req *protocol.Request, params param.Params, text string) (reflect.Value, error) { + q1 := req.URI().QueryArgs().Peek("a") + if len(q1) == 0 { + return reflect.Value{}, fmt.Errorf("can be nil") + } + val := CustomizedDecode{ + A: string(q1), + } + return reflect.ValueOf(val), nil + }) + if err != nil { + t.Fatal(err) + } + binder := NewDefaultBinder(bindConfig) + + req := newMockRequest(). + SetRequestURI("http://foobar.com?b=2") + result := Foo{} + err = binder.Bind(req.Req, &result, nil) + if err == nil { + t.Fatal("expect an required error, but get nil") + } +} + +// TestBind_Required_CustomizedTypeDecode2 is a test case for customized type decoder by json. +// this case tells us that never only use json tag for customized type. There are two reasons: +// 1. your customized decoder function will never be called. +// 2. your customized type must meet json unmarshaler format or you will get an unmarshal error. Since it meet json unmarshaler format, you can use json tag directly. +func TestBind_Required_CustomizedTypeDecode2(t *testing.T) { + type Foo struct { + S int `json:"s"` + F CustomizedDecode `json:"f,required"` + D string `json:"d"` + Q string `query:"q"` + } + + bindConfig := &BindConfig{} + err := bindConfig.RegTypeUnmarshal(reflect.TypeOf(CustomizedDecode{}), func(req *protocol.Request, params param.Params, text string) (reflect.Value, error) { + t.Log("CustomizedDecode") + q1 := req.URI().QueryArgs().Peek("a") + if len(q1) == 0 { + return reflect.Value{}, fmt.Errorf("can be nil") + } + val := CustomizedDecode{ + A: string(q1), + } + return reflect.ValueOf(val), nil + }) + if err != nil { + t.Fatal(err) + } + binder := NewDefaultBinder(bindConfig) + + req := newMockRequest(). + SetRequestURI("http://foobar.com?q=dummy").SetJSONContentType().SetBody([]byte(`{"f":54, "d":"d", "s":1}`)) + result := Foo{} + err = binder.Bind(req.Req, &result, nil) + if err == nil { + t.Fatal("expect an unUnmarshal error, but get nil") + } +} + func TestBind_JSON(t *testing.T) { type Req struct { J1 string `json:"j1"` diff --git a/pkg/app/server/binding/internal/decoder/customized_type_decoder.go b/pkg/app/server/binding/internal/decoder/customized_type_decoder.go index 19efa46ae..1b39aec5f 100644 --- a/pkg/app/server/binding/internal/decoder/customized_type_decoder.go +++ b/pkg/app/server/binding/internal/decoder/customized_type_decoder.go @@ -41,6 +41,7 @@ package decoder import ( + "fmt" "reflect" "github.com/cloudwego/hertz/pkg/protocol" @@ -58,6 +59,7 @@ func (d *customizedFieldTextDecoder) Decode(req *protocol.Request, params param. var text string var exist bool var defaultValue string + var err error for _, tagInfo := range d.tagInfos { if tagInfo.Skip || tagInfo.Key == jsonTag || tagInfo.Key == fileNameTag { defaultValue = tagInfo.Default @@ -66,8 +68,15 @@ func (d *customizedFieldTextDecoder) Decode(req *protocol.Request, params param. text, exist = tagInfo.Getter(req, params, tagInfo.Value) defaultValue = tagInfo.Default if exist { + err = nil break } + if tagInfo.Required { + err = fmt.Errorf("'%s' field is a 'required' parameter, but the request does not have this parameter", d.fieldName) + } + } + if err != nil { + return err } if !exist { return nil From 4601c8511a1d77bc4b7d757ee7b919f3e5a02c61 Mon Sep 17 00:00:00 2001 From: iksars <211250186@smail.nju.edu.cn> Date: Wed, 21 Feb 2024 18:46:32 +0800 Subject: [PATCH 2/3] fmt --- pkg/app/server/binding/binder_test.go | 14 ++++++++------ .../internal/decoder/customized_type_decoder.go | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/pkg/app/server/binding/binder_test.go b/pkg/app/server/binding/binder_test.go index c1f973130..3af43d14a 100644 --- a/pkg/app/server/binding/binder_test.go +++ b/pkg/app/server/binding/binder_test.go @@ -580,7 +580,7 @@ func TestBind_CustomizedTypeDecodeForPanic(t *testing.T) { }) } -func TestBind_Required_CustomizedTypeDecode1(t *testing.T) { +func TestBind_RequiredCustomizedTypeDecode1(t *testing.T) { type Foo struct { F ***CustomizedDecode `query:"a,required"` } @@ -610,11 +610,13 @@ func TestBind_Required_CustomizedTypeDecode1(t *testing.T) { } } -// TestBind_Required_CustomizedTypeDecode2 is a test case for customized type decoder by json. -// this case tells us that never only use json tag for customized type. There are two reasons: -// 1. your customized decoder function will never be called. -// 2. your customized type must meet json unmarshaler format or you will get an unmarshal error. Since it meet json unmarshaler format, you can use json tag directly. -func TestBind_Required_CustomizedTypeDecode2(t *testing.T) { +/* + * TestBind_Required_CustomizedTypeDecode2 is a test case for customized type decoder by json. + * this case tells us that never only use json tag for customized type. There are two reasons: + * 1. your customized decoder function will never be called. + * 2. your customized type must meet json unmarshaler format or you will get an unmarshal error. Since it meet json unmarshaler format, you can use json tag directly. + */ +func TestBind_RequiredCustomizedTypeDecode2(t *testing.T) { type Foo struct { S int `json:"s"` F CustomizedDecode `json:"f,required"` diff --git a/pkg/app/server/binding/internal/decoder/customized_type_decoder.go b/pkg/app/server/binding/internal/decoder/customized_type_decoder.go index 1b39aec5f..5a3d998f0 100644 --- a/pkg/app/server/binding/internal/decoder/customized_type_decoder.go +++ b/pkg/app/server/binding/internal/decoder/customized_type_decoder.go @@ -56,10 +56,10 @@ type customizedFieldTextDecoder struct { } func (d *customizedFieldTextDecoder) Decode(req *protocol.Request, params param.Params, reqValue reflect.Value) error { + var err error var text string var exist bool var defaultValue string - var err error for _, tagInfo := range d.tagInfos { if tagInfo.Skip || tagInfo.Key == jsonTag || tagInfo.Key == fileNameTag { defaultValue = tagInfo.Default From f8594a343f420d27eebe9b6ad5fc85dae8f06c1c Mon Sep 17 00:00:00 2001 From: iksars <211250186@smail.nju.edu.cn> Date: Wed, 21 Feb 2024 19:04:08 +0800 Subject: [PATCH 3/3] gofumpt /pkg/protocol/http1/proxy/proxy.go --- pkg/protocol/http1/proxy/proxy.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/protocol/http1/proxy/proxy.go b/pkg/protocol/http1/proxy/proxy.go index f8bae7608..2b243ff04 100644 --- a/pkg/protocol/http1/proxy/proxy.go +++ b/pkg/protocol/http1/proxy/proxy.go @@ -81,13 +81,11 @@ func SetupProxy(conn network.Conn, addr string, proxyURI *protocol.URI, tlsConfi defer close(didReadResponse) err = reqI.Write(connectReq, conn) - if err != nil { return } err = conn.Flush() - if err != nil { return }