Repository: cloudwego/hertz
Pull request #1102 (2024-04-28 09:33:48)

fix: resp set trailer will panic
#### What type of PR is this?
<!--
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
-->
fix
#### 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:
-->
- [x] This PR title match the format: \<type\>(optional scope): \<description\>
- [x] The description of this PR title is user-oriented and clear enough for others to understand.
- [x] 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.
修复当使用 ResponseHeader.Set/Add 设置 Trailer 时，有可能会 panic 的问题

#### (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: `ResponseHeader.Set/Add` method uses `bytesconv.S2b` method to reduce string copying, while `SetTrailer` modifies data in place using `utils.NormalizeHeaderKey`. However, if the string used for setting the header comes from RODATA, it can cause a segmentation fault.
Solution: make a copy of the header value when calling SetTrailer, to avoid modifying the data in place.

zh(optional): `ResponseHeader.Set/Add` 使用了 `bytesconv.S2b` 方法来减少 string 的拷贝，而 `SetTrailer` 会使用 `utils.NormalizeHeaderKey` 来原地修改数据，这是如果设置 Header 使用的 string 来自 RODATA，那么会造成段错误
解决方案：在 SetTrailer 时拷贝一次 header value

#### (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/1102?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 82.81%. Comparing base [(`b7cbc9d`)](https://app.codecov.io/gh/cloudwego/hertz/commit/b7cbc9de1778367e157c1509818da08f5e16592c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) to head [(`7a9b04a`)](https://app.codecov.io/gh/cloudwego/hertz/pull/1102?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego).
> Report is 8 commits behind head on develop.


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


```diff
@@             Coverage Diff             @@
##           develop    #1102      +/-   ##
===========================================
+ Coverage    82.79%   82.81%   +0.02%     
===========================================
  Files           98       98              
  Lines        10032    10033       +1     
===========================================
+ Hits          8306     8309       +3     
+ Misses        1236     1235       -1     
+ Partials       490      489       -1     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/cloudwego/hertz/pull/1102?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).


Patch:
diff --git a/pkg/protocol/header.go b/pkg/protocol/header.go
index 278b137..8888548 100644
--- a/pkg/protocol/header.go
+++ b/pkg/protocol/header.go
@@ -1766,6 +1766,8 @@ func (h *ResponseHeader) setSpecialHeader(key, value []byte) bool {
 			// Transfer-Encoding is managed automatically.
 			return true
 		} else if utils.CaseInsensitiveCompare(bytestr.StrTrailer, key) {
+			// copy value to avoid panic
+			value = append(h.bufKV.value[:0], value...)
 			h.Trailer().SetTrailers(value)
 			return true
 		}


Test patch:
diff --git a/pkg/protocol/header_test.go b/pkg/protocol/header_test.go
index a147040..ba2bd69 100644
--- a/pkg/protocol/header_test.go
+++ b/pkg/protocol/header_test.go
@@ -806,3 +806,13 @@ func TestResponseHeaderDateEmpty(t *testing.T) {
 		t.Fatalf("ResponseDateNoDefaultNotEmpty fail, response: \n%+v\noutcome: \n%q\n", h, headers) //nolint:govet
 	}
 }
+
+func TestSetTrailerWithROString(t *testing.T) {
+	h := &RequestHeader{}
+	h.Add(consts.HeaderTrailer, "foo,bar,hertz")
+	assert.DeepEqual(t, "Foo, Bar, Hertz", h.Get(consts.HeaderTrailer))
+
+	h1 := &ResponseHeader{}
+	h1.Add(consts.HeaderTrailer, "foo,bar,hertz")
+	assert.DeepEqual(t, "Foo, Bar, Hertz", h1.Get(consts.HeaderTrailer))
+}