Closed sunziping2016 closed 4 months ago
@sunziping2016 can you offer the raw request curl for the header? I will try it
@sunziping2016 can you offer the raw request curl for the header? I will try it
I tried to reproducing the issue. And I found my origin description of this issue inaccurate.
package main
import (
"context"
"github.com/cloudwego/hertz/pkg/app"
"github.com/cloudwego/hertz/pkg/app/server"
"github.com/cloudwego/hertz/pkg/common/utils"
"github.com/cloudwego/hertz/pkg/protocol/consts"
)
type Request struct {
XSingleValue string `header:"X-SingleHead-Value"`
XMultipleValue []string `header:"X-MultipleHead-Value"`
}
func main() {
h := server.Default()
h.GET("/", func(c context.Context, ctx *app.RequestContext) {
var req Request
err := ctx.BindAndValidate(&req)
if err != nil {
ctx.AbortWithStatusJSON(consts.StatusInternalServerError, utils.H{
"message": "failed to decode",
})
} else {
ctx.JSON(consts.StatusOK, utils.H{
"data": req,
})
}
})
h.Spin()
}
Compile it with the latest version of Hertz.
Then, run curl http://127.0.0.1:8888 -H "X-MultipleHead-Value: hello"
. You should be able to see {"data":{"XSingleValue":"","XMultipleValue":null}}
, whereas the expected output shall be {"data":{"XSingleValue":"","XMultipleValue":["hello"]}}
.
However to trigger this issue (or unexpected behavior for me), the struct field tag must contains an uppercase letter in the middle of a word. Change "X-MultipleHead-Value" to "X-Multiplehead-Value" will make hertz behave as expected. Anyway, to trigger this issue, here are some requirements:
[]string
not string
HTTP defines that header names are case-insensitive. The request parser implements this by using CanonicalHeaderKey, making the first character and any characters following a hyphen uppercase and the rest lowercase.
So the uppercase letter in the middle is useless.
If it must be used this way, perhaps the conversion can be disabled using
ctx.Request.Header.DisableNormalizing()
HTTP defines that header names are case-insensitive. The request parser implements this by using CanonicalHeaderKey, making the first character and any characters following a hyphen uppercase and the rest lowercase.
So the uppercase letter in the middle is useless.
Thanks! I see. I think this issue's title is no longer accurate, and the behavior of Hertz looks good to me. I'll avoid any uppercase letter in the middle.
However, the inconsistent behavior between []string
and string
may be confusing to users. Anyway, that's not a big deal.
❯ curl http://127.0.0.1:8888 -H "X-MultipleHeader-Value: hello"
{"data":{"XSingleValue":"","XMultipleValue":null}}⏎
❯ curl http://127.0.0.1:8888 -H "X-SingleHeader-Value: hello"
{"data":{"XSingleValue":"hello","XMultipleValue":null}}⏎
Thanks for your reply. You can close the issue.
@sunziping2016 @rogerogers 这个问题我说明一下吧,在重构的时候就已经考虑到了。 重构前:
重构后:
@sunziping2016 如果有兴趣,可以提个 pr 再讨论下。代码修改位置: https://github.com/cloudwego/hertz/blob/develop/pkg/app/server/binding/internal/decoder/slice_getter.go#L126
Describe the bug
Hertz header binding becomes case sensitve since v0.7
To Reproduce
Use the following struct definition
Then bind to a request header which use
*-Websocket-*
instead of*-WebSocket-*
.Only
SecWebSocketKey
andSecWebSocketVersion
(whose type isstring
) can be successfully bound.SecWebSocketExtensions
andSecWebSocketProtocol
(whose type is[]string
) cannot be bound correctly and will be empty no matter what the request sets.Expected behavior
Successfully binding all the header.
Screenshots
N/A
Hertz version:
v0.7.3 affected. v0.6.* seems not affected.
Environment:
N/A
Additional context
To make it worse, the standard way to spell it is
WebSocket
notWebsocket
. But there is a legacy bug with Chromium. See wrong headers for websockets draft-hixie-76 protocol [41183445] - Chromium. If a user decides to support chromium using the binding facilities provided by hertz, other browsers may get broken.