-
Notifications
You must be signed in to change notification settings - Fork 14
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
fix: skip write body when status not allowed #20
fix: skip write body when status not allowed #20
Conversation
Signed-off-by: rogrogers <rogers@rogerogers.com>
@rogerogers it's not just status not modified. also, it's "ok" to include body for custom status code. so i think it's ok to have body everywhere too. |
https://cs.opensource.google/go/x/net/+/master:http2/http2.go;l=304 |
possible to expedite this merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me
cc @wzekin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls just approve else 304 just crash. worst by doing nothing
@rogerogers @welkeyever |
You can use github.com/hertz-contrib/http2@962a08ada7d76a386c4b47a7d65cf75d0275c291 to perform your test. and update it when we release the new version |
What type of PR is this?
fix
What this PR does / why we need it (English/Chinese):
en: don't write body when status not allowed. According to the RFC definition https://www.rfc-editor.org/rfc/rfc9110.html#name-204-no-content, 204 and 304 response is terminated by the end of the header section; it cannot contain content or trailers.
zh: 状态码不允许存在body的时候不写入body。根据rfc定义,204、304还有其他几个状态码,不能设置响应body,这里跳过写入body。
Which issue(s) this PR fixes:
Fixes cloudwego/hertz#788