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.

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

A collison between hertz customed type binding and 'required' tag check #1007

Open
iksars opened this issue Nov 26, 2023 · 1 comment · May be fixed by #1058
Open

A collison between hertz customed type binding and 'required' tag check #1007

iksars opened this issue Nov 26, 2023 · 1 comment · May be fixed by #1058
Assignees
Labels
question Further information is requested

Comments

@iksars
Copy link

iksars commented Nov 26, 2023

Describe the Question
I use hertz thrift code generation for my project and I meet a problem about binding param of customed Type. I mark the customed type with "required" tag and "api.header=' X-Authorization-TokenPayload'" tag in the thrift file. Then I inject the customed bindConfig into hertz server . When there is a filed named 'X-Authorization-TokenPayload' in request header, it works well. However , when there is not a filed named that, I hope that hertz can throw a bind error but actually it doesn't. It seems that "required" tag can't work because of customed bind func. Is it a bug? If not, how can I solve it? (hz version v0.6.5)

idl define:

struct CreateUploadTaskReq {
    1: required base.TokenPayload TokenPayload (api.header="X-Authorization-TokenPayload");   
    2: .....  
}

customed bindconfig:

func bindTokenPayloadConfig() *binding.BindConfig {
	bindConfig := &binding.BindConfig{}
	bindConfig.MustRegTypeUnmarshal(reflect.TypeOf(base.TokenPayload{}), func(req *protocol.Request, params param.Params, text string) (reflect.Value, error) {
		hlog.Info("tokenpayload", "text:", text)
		if text == "" {
			return reflect.ValueOf(base.TokenPayload{}), errors.New("tokenpayload is required but not found")
		}
		val := base.TokenPayload{}
		if err := json.Unmarshal([]byte(text), &val); err != nil {
			return reflect.ValueOf(base.TokenPayload{}), err
		}
		return reflect.ValueOf(val), nil
	})
	return bindConfig
}
@FGYFFFF
Copy link
Contributor

FGYFFFF commented Nov 27, 2023

The previous idea was that a custom binding would allow the user to do all the actions for that field, but it seems like Required shouldn't be done by the user. So I'll put the Required validation into hertz. I'll mention a pr.

@li-jin-gou li-jin-gou added the question Further information is requested label Dec 11, 2023
iksars added a commit to iksars/hertz-develop that referenced this issue Feb 1, 2024
…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
@iksars iksars linked a pull request Feb 1, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Development

Successfully merging a pull request may close this issue.

3 participants