Repository: cloudwego/hertz
Pull request #1321 (2025-04-03 10:41:15)

fix(protocol): header safe for concurrent reading
```
b := []byte(key) // <--- ([]byte)(key) does not escape
utils.NormalizeHeaderKey(b, false)
return h.peek(bytesconv.B2s(b))
```

actually there is no need to reuse a buf for normalizing key ...

#### What type of PR is this?
fix

<!--
Add one of the following kinds:

build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
docs: Documentation only changes
feat: A new feature
optimize: A new optimization
fix: A bug fix
perf: A code change that improves performance
refactor: A code change that neither fixes a bug nor adds a feature
style: Changes that do not affect the meaning of the code (white space, formatting, missing semi-colons, etc)
test: Adding missing tests or correcting existing tests
chore: Changes to the build process or auxiliary tools and libraries such as documentation generation
-->

#### Check the PR title.
<!--
The description of the title will be attached in Release Notes, 
so please describe it from user-oriented, what this PR does / why we need it.
Please check your PR title with the below requirements:
-->
- [ ] This PR title match the format: \<type\>(optional scope): \<description\>
- [ ] The description of this PR title is user-oriented and clear enough for others to understand.
- [ ] Attach the PR updating the user documentation if the current PR requires user awareness at the usage level. [User docs repo](https://github.com/cloudwego/cloudwego.github.io)


#### (Optional) Translate the PR title into Chinese.
修复 Header 对于 Peek 方法不是并发安全的问题

#### (Optional) More detailed description for this PR(en: English/zh: Chinese).
<!--
Provide more detailed info for review(e.g., it's recommended to provide perf data if this is a perf type PR).
-->
en:
zh(optional):

#### (Optional) Which issue(s) this PR fixes:
<!--
Automatically closes linked issue when PR is merged.
Eg: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->

#### (Optional) The PR that updates user documentation:
<!--
If the current PR requires user awareness at the usage level, please submit a PR to update user docs. [User docs repo](https://github.com/cloudwego/cloudwego.github.io)
-->

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/cloudwego/hertz/pull/1321?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) Report
Attention: Patch coverage is `89.13043%` with `5 lines` in your changes missing coverage. Please review.
> Project coverage is 72.75%. Comparing base [(`95440fa`)](https://app.codecov.io/gh/cloudwego/hertz/commit/95440fa69ce9ffdd79b19b822f9b06749e100180?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) to head [(`64dcb4c`)](https://app.codecov.io/gh/cloudwego/hertz/commit/64dcb4c66444fc923c24e2dd695d203418b13fb6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego).

| [Files with missing lines](https://app.codecov.io/gh/cloudwego/hertz/pull/1321?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) | Patch % | Lines |
|---|---|---|
| [pkg/protocol/header.go](https://app.codecov.io/gh/cloudwego/hertz/pull/1321?src=pr&el=tree&filepath=pkg%2Fprotocol%2Fheader.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego#diff-cGtnL3Byb3RvY29sL2hlYWRlci5nbw==) | 86.48% | [5 Missing :warning: ](https://app.codecov.io/gh/cloudwego/hertz/pull/1321?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) |

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##           develop    #1321      +/-   ##
===========================================
- Coverage    72.78%   72.75%   -0.03%     
===========================================
  Files          139      139              
  Lines        16242    16244       +2     
===========================================
- Hits         11821    11819       -2     
- Misses        3824     3827       +3     
- Partials       597      598       +1     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/cloudwego/hertz/pull/1321?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego).

<details><summary> :rocket: New features to boost your workflow: </summary>

- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>

Patch:
diff --git a/internal/bytesconv/bytesconv.go b/internal/bytesconv/bytesconv.go
index fd355ec..e2d1ca5 100644
--- a/internal/bytesconv/bytesconv.go
+++ b/internal/bytesconv/bytesconv.go
@@ -43,7 +43,6 @@ package bytesconv
 
 import (
 	"net/http"
-	"reflect"
 	"sync"
 	"time"
 	"unsafe"
@@ -71,23 +70,23 @@ func LowercaseBytes(b []byte) {
 // Note it may break if string and/or slice header will change
 // in the future go versions.
 func B2s(b []byte) string {
-	/* #nosec G103 */
 	return *(*string)(unsafe.Pointer(&b))
 }
 
+type sliceHeader struct {
+	Data unsafe.Pointer
+	Len  int
+	Cap  int
+}
+
 // S2b converts string to a byte slice without memory allocation.
 //
 // Note it may break if string and/or slice header will change
 // in the future go versions.
 func S2b(s string) (b []byte) {
-	/* #nosec G103 */
-	bh := (*reflect.SliceHeader)(unsafe.Pointer(&b))
-	/* #nosec G103 */
-	sh := (*reflect.StringHeader)(unsafe.Pointer(&s))
-	bh.Data = sh.Data
-	bh.Len = sh.Len
-	bh.Cap = sh.Len
-	return b
+	*(*string)(unsafe.Pointer(&b)) = s
+	(*sliceHeader)(unsafe.Pointer(&b)).Cap = len(s)
+	return
 }
 
 func WriteHexInt(w network.Writer, n int) error {
diff --git a/pkg/protocol/header.go b/pkg/protocol/header.go
index 8888548..e68ec5c 100644
--- a/pkg/protocol/header.go
+++ b/pkg/protocol/header.go
@@ -258,8 +258,8 @@ func (h *RequestHeader) Add(key, value string) {
 	if h.setSpecialHeader(bytesconv.S2b(key), bytesconv.S2b(value)) {
 		return
 	}
-
-	k := getHeaderKeyBytes(&h.bufKV, key, h.disableNormalizing)
+	k := []byte(key)
+	utils.NormalizeHeaderKey(k, h.disableNormalizing)
 	h.h = appendArg(h.h, bytesconv.B2s(k), value, ArgsHasValue)
 }
 
@@ -336,7 +336,7 @@ func (h *RequestHeader) GetBufValue() []byte {
 // HasAcceptEncodingBytes returns true if the header contains
 // the given Accept-Encoding value.
 func (h *RequestHeader) HasAcceptEncodingBytes(acceptEncoding []byte) bool {
-	ae := h.peek(bytestr.StrAcceptEncoding)
+	ae := h.peek(consts.HeaderAcceptEncoding)
 	n := bytes.Index(ae, acceptEncoding)
 	if n < 0 {
 		return false
@@ -352,7 +352,7 @@ func (h *RequestHeader) HasAcceptEncodingBytes(acceptEncoding []byte) bool {
 }
 
 func (h *RequestHeader) PeekIfModifiedSinceBytes() []byte {
-	return h.peek(bytestr.StrIfModifiedSince)
+	return h.peek(consts.HeaderIfModifiedSince)
 }
 
 // RequestURI returns RequestURI from the first HTTP request line.
@@ -551,8 +551,8 @@ func (h *ResponseHeader) Add(key, value string) {
 	if h.setSpecialHeader(bytesconv.S2b(key), bytesconv.S2b(value)) {
 		return
 	}
-
-	k := getHeaderKeyBytes(&h.bufKV, key, h.disableNormalizing)
+	k := []byte(key)
+	utils.NormalizeHeaderKey(k, h.disableNormalizing)
 	h.h = appendArg(h.h, bytesconv.B2s(k), value, ArgsHasValue)
 }
 
@@ -622,9 +622,9 @@ func (h *ResponseHeader) DelCookieBytes(key []byte) {
 
 // DelBytes deletes header with the given key.
 func (h *ResponseHeader) DelBytes(key []byte) {
-	h.bufKV.key = append(h.bufKV.key[:0], key...)
-	utils.NormalizeHeaderKey(h.bufKV.key, h.disableNormalizing)
-	h.del(h.bufKV.key)
+	k := []byte(string(key))
+	utils.NormalizeHeaderKey(k, h.disableNormalizing)
+	h.del(k)
 }
 
 // Header returns response header representation.
@@ -636,7 +636,7 @@ func (h *ResponseHeader) Header() []byte {
 }
 
 func (h *ResponseHeader) PeekLocation() []byte {
-	return h.peek(bytestr.StrLocation)
+	return h.peek(consts.HeaderLocation)
 }
 
 // DelClientCookie instructs the client to remove the given cookie.
@@ -684,8 +684,9 @@ func (h *ResponseHeader) DelClientCookieBytes(key []byte) {
 // Returned value is valid until the next call to ResponseHeader.
 // Do not store references to returned value. Make copies instead.
 func (h *ResponseHeader) Peek(key string) []byte {
-	k := getHeaderKeyBytes(&h.bufKV, key, h.disableNormalizing)
-	return h.peek(k)
+	k := []byte(key)
+	utils.NormalizeHeaderKey(k, h.disableNormalizing)
+	return h.peek(bytesconv.B2s(k))
 }
 
 func (h *ResponseHeader) IsDisableNormalizing() bool {
@@ -699,8 +700,8 @@ func (h *ResponseHeader) ParseSetCookie(value []byte) {
 	kv.value = append(kv.value[:0], value...)
 }
 
-func (h *ResponseHeader) peek(key []byte) []byte {
-	switch string(key) {
+func (h *ResponseHeader) peek(key string) []byte {
+	switch key {
 	case consts.HeaderContentType:
 		return h.ContentType()
 	case consts.HeaderContentEncoding:
@@ -711,7 +712,7 @@ func (h *ResponseHeader) peek(key []byte) []byte {
 		if h.ConnectionClose() {
 			return bytestr.StrClose
 		}
-		return peekArgBytes(h.h, key)
+		return peekArgStr(h.h, key)
 	case consts.HeaderContentLength:
 		return h.contentLengthBytes
 	case consts.HeaderSetCookie:
@@ -719,7 +720,7 @@ func (h *ResponseHeader) peek(key []byte) []byte {
 	case consts.HeaderTrailer:
 		return h.Trailer().GetBytes()
 	default:
-		return peekArgBytes(h.h, key)
+		return peekArgStr(h.h, key)
 	}
 }
 
@@ -730,7 +731,8 @@ func (h *ResponseHeader) peek(key []byte) []byte {
 // Any future calls to the Peek* will modify the returned value.
 // Do not store references to returned value. Use ResponseHeader.GetAll(key) instead.
 func (h *ResponseHeader) PeekAll(key string) [][]byte {
-	k := getHeaderKeyBytes(&h.bufKV, key, h.disableNormalizing)
+	k := []byte(key)
+	utils.NormalizeHeaderKey(k, h.disableNormalizing)
 	return h.peekAll(k)
 }
 
@@ -772,7 +774,8 @@ func (h *ResponseHeader) peekAll(key []byte) [][]byte {
 // Any future calls to the Peek* will modify the returned value.
 // Do not store references to returned value. Use RequestHeader.GetAll(key) instead.
 func (h *RequestHeader) PeekAll(key string) [][]byte {
-	k := getHeaderKeyBytes(&h.bufKV, key, h.disableNormalizing)
+	k := []byte(key)
+	utils.NormalizeHeaderKey(k, h.disableNormalizing)
 	return h.peekAll(k)
 }
 
@@ -984,7 +987,8 @@ func (h *ResponseHeader) StatusCode() int {
 
 // Del deletes header with the given key.
 func (h *ResponseHeader) Del(key string) {
-	k := getHeaderKeyBytes(&h.bufKV, key, h.disableNormalizing)
+	k := []byte(key)
+	utils.NormalizeHeaderKey(k, h.disableNormalizing)
 	h.del(k)
 }
 
@@ -1013,7 +1017,8 @@ func (h *ResponseHeader) del(key []byte) {
 //
 // Use AddBytesV for setting multiple header values under the same key.
 func (h *ResponseHeader) SetBytesV(key string, value []byte) {
-	k := getHeaderKeyBytes(&h.bufKV, key, h.disableNormalizing)
+	k := []byte(key)
+	utils.NormalizeHeaderKey(k, h.disableNormalizing)
 	h.SetCanonical(k, value)
 }
 
@@ -1064,14 +1069,15 @@ func (h *RequestHeader) SetByteRange(startPos, endPos int) {
 
 // DelBytes deletes header with the given key.
 func (h *RequestHeader) DelBytes(key []byte) {
-	h.bufKV.key = append(h.bufKV.key[:0], key...)
-	utils.NormalizeHeaderKey(h.bufKV.key, h.disableNormalizing)
-	h.del(h.bufKV.key)
+	k := []byte(string(key))
+	utils.NormalizeHeaderKey(k, h.disableNormalizing)
+	h.del(k)
 }
 
 // Del deletes header with the given key.
 func (h *RequestHeader) Del(key string) {
-	k := getHeaderKeyBytes(&h.bufKV, key, h.disableNormalizing)
+	k := []byte(key)
+	utils.NormalizeHeaderKey(k, h.disableNormalizing)
 	h.del(k)
 }
 
@@ -1128,8 +1134,9 @@ func (h *RequestHeader) CopyTo(dst *RequestHeader) {
 // Returned value is valid until the next call to RequestHeader.
 // Do not store references to returned value. Make copies instead.
 func (h *RequestHeader) Peek(key string) []byte {
-	k := getHeaderKeyBytes(&h.bufKV, key, h.disableNormalizing)
-	return h.peek(k)
+	k := []byte(key)
+	utils.NormalizeHeaderKey(k, h.disableNormalizing)
+	return h.peek(bytesconv.B2s(k))
 }
 
 // SetMultipartFormBoundary sets the following Content-Type:
@@ -1277,9 +1284,9 @@ func (h *RequestHeader) SetRequestURIBytes(requestURI []byte) {
 //
 // Use AddBytesKV for setting multiple header values under the same key.
 func (h *RequestHeader) SetBytesKV(key, value []byte) {
-	h.bufKV.key = append(h.bufKV.key[:0], key...)
-	utils.NormalizeHeaderKey(h.bufKV.key, h.disableNormalizing)
-	h.SetCanonical(h.bufKV.key, value)
+	k := []byte(string(key))
+	utils.NormalizeHeaderKey(k, h.disableNormalizing)
+	h.SetCanonical(k, value)
 }
 
 func (h *RequestHeader) AddArgBytes(key, value []byte, noValue bool) {
@@ -1324,11 +1331,11 @@ func (h *RequestHeader) Cookies() []*Cookie {
 }
 
 func (h *RequestHeader) PeekRange() []byte {
-	return h.peek(bytestr.StrRange)
+	return h.peek(consts.HeaderRange)
 }
 
 func (h *RequestHeader) PeekContentEncoding() []byte {
-	return h.peek(bytestr.StrContentEncoding)
+	return h.peek(consts.HeaderContentEncoding)
 }
 
 // FullCookie returns complete cookie bytes
@@ -1409,7 +1416,8 @@ func (h *RequestHeader) Set(key, value string) {
 }
 
 func initHeaderKV(kv *argsKV, key, value string, disableNormalizing bool) {
-	kv.key = getHeaderKeyBytes(kv, key, disableNormalizing)
+	kv.key = append(kv.key[:0], key...)
+	utils.NormalizeHeaderKey(kv.key, disableNormalizing)
 	kv.value = append(kv.value[:0], value...)
 }
 
@@ -1598,8 +1606,8 @@ func appendArg(args []argsKV, key, value string, noValue bool) []argsKV {
 	return args
 }
 
-func (h *RequestHeader) peek(key []byte) []byte {
-	switch string(key) {
+func (h *RequestHeader) peek(key string) []byte {
+	switch key {
 	case consts.HeaderHost:
 		return h.Host()
 	case consts.HeaderContentType:
@@ -1610,18 +1618,18 @@ func (h *RequestHeader) peek(key []byte) []byte {
 		if h.ConnectionClose() {
 			return bytestr.StrClose
 		}
-		return peekArgBytes(h.h, key)
+		return peekArgStr(h.h, key)
 	case consts.HeaderContentLength:
 		return h.contentLengthBytes
 	case consts.HeaderCookie:
 		if h.cookiesCollected {
 			return appendRequestCookieBytes(nil, h.cookies)
 		}
-		return peekArgBytes(h.h, key)
+		return peekArgStr(h.h, key)
 	case consts.HeaderTrailer:
 		return h.Trailer().GetBytes()
 	default:
-		return peekArgBytes(h.h, key)
+		return peekArgStr(h.h, key)
 	}
 }
 
@@ -1693,12 +1701,6 @@ func refreshServerDate() {
 	ServerDate.Store(b)
 }
 
-func getHeaderKeyBytes(kv *argsKV, key string, disableNormalizing bool) []byte {
-	kv.key = append(kv.key[:0], key...)
-	utils.NormalizeHeaderKey(kv.key, disableNormalizing)
-	return kv.key
-}
-
 // SetMethodBytes sets HTTP request method.
 func (h *RequestHeader) SetMethodBytes(method []byte) {
 	h.method = append(h.method[:0], method...)
diff --git a/pkg/protocol/request.go b/pkg/protocol/request.go
index 0c1c3f4..1bec09f 100644
--- a/pkg/protocol/request.go
+++ b/pkg/protocol/request.go
@@ -172,7 +172,7 @@ func (req *Request) BodyBuffer() *bytebufferpool.ByteBuffer {
 //     with ContinueReadBody.
 //   - Or close the connection.
 func (req *Request) MayContinue() bool {
-	return bytes.Equal(req.Header.peek(bytestr.StrExpect), bytestr.Str100Continue)
+	return bytes.Equal(req.Header.peek(consts.HeaderExpect), bytestr.Str100Continue)
 }
 
 // Scheme returns the scheme of the request.
@@ -238,7 +238,7 @@ func (req *Request) MultipartForm() (*multipart.Form, error) {
 		return nil, errors.ErrNoMultipartForm
 	}
 
-	ce := req.Header.peek(bytestr.StrContentEncoding)
+	ce := req.Header.peek(consts.HeaderContentEncoding)
 	var err error
 	var f *multipart.Form
 
diff --git a/pkg/protocol/trailer.go b/pkg/protocol/trailer.go
index a701cf4..ff90e26 100644
--- a/pkg/protocol/trailer.go
+++ b/pkg/protocol/trailer.go
@@ -41,13 +41,15 @@ func (t *Trailer) Get(key string) string {
 // Returned value is valid until the next call to Trailer.
 // Do not store references to returned value. Make copies instead.
 func (t *Trailer) Peek(key string) []byte {
-	k := getHeaderKeyBytes(&t.bufKV, key, t.disableNormalizing)
+	k := []byte(key)
+	utils.NormalizeHeaderKey(k, t.disableNormalizing)
 	return peekArgBytes(t.h, k)
 }
 
 // Del deletes trailer with the given key.
 func (t *Trailer) Del(key string) {
-	k := getHeaderKeyBytes(&t.bufKV, key, t.disableNormalizing)
+	k := []byte(key)
+	utils.NormalizeHeaderKey(k, t.disableNormalizing)
 	t.h = delAllArgsBytes(t.h, k)
 }
 


Test patch:
diff --git a/pkg/protocol/header_test.go b/pkg/protocol/header_test.go
index ba2bd69..e9c7403 100644
--- a/pkg/protocol/header_test.go
+++ b/pkg/protocol/header_test.go
@@ -816,3 +816,15 @@ func TestSetTrailerWithROString(t *testing.T) {
 	h1.Add(consts.HeaderTrailer, "foo,bar,hertz")
 	assert.DeepEqual(t, "Foo, Bar, Hertz", h1.Get(consts.HeaderTrailer))
 }
+
+func Benchmark_RequestHeader_Peek(b *testing.B) {
+	h := &RequestHeader{}
+	h.Add("hello", "world")
+	if s := string(h.Peek("hello")); s != "world" {
+		b.Fatal(s)
+	}
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		h.Peek("hello")
+	}
+}
diff --git a/pkg/protocol/request_test.go b/pkg/protocol/request_test.go
index 48ef4be..8902b48 100644
--- a/pkg/protocol/request_test.go
+++ b/pkg/protocol/request_test.go
@@ -718,6 +718,7 @@ func TestRequestBodyReuse(t *testing.T) {
 		B: make([]byte, 0, req.maxKeepBodySize+1),
 	}
 	// set a small body
+	buf = req.BodyBuffer()
 	buf.Write(make([]byte, 1))
 	req.ResetBody()
 	assert.Nil(t, req.body)