Skip to content

Commit

Permalink
fix:Add required check for customed type binding \n Required tag asso…
Browse files Browse the repository at this point in the history
…cated 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 cloudwego#1007
  • Loading branch information
iksars committed Feb 1, 2024
1 parent 0619701 commit d868810
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 0 deletions.
68 changes: 68 additions & 0 deletions pkg/app/server/binding/binder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
package decoder

import (
"fmt"
"reflect"

"github.com/cloudwego/hertz/pkg/protocol"
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit d868810

Please sign in to comment.