cloudwego / hertz

Go HTTP framework with high-performance and strong-extensibility for building micro-services.
https://www.cloudwego.io
Apache License 2.0
5.28k stars 515 forks source link

fix: resp set trailer will panic #1102

Closed wzekin closed 5 months ago

wzekin commented 6 months ago

What type of PR is this?

fix

Check the PR title.

(Optional) Translate the PR title into Chinese.

修复当使用 ResponseHeader.Set/Add 设置 Trailer 时,有可能会 panic 的问题

(Optional) More detailed description for this PR(en: English/zh: Chinese).

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:

(Optional) The PR that updates user documentation:

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 82.81%. Comparing base (b7cbc9d) to head (7a9b04a). Report is 13 commits behind head on develop.

:exclamation: Current head 7a9b04a differs from pull request most recent head c41a858. Consider uploading reports for the commit c41a858 to get more accurate results

Additional details and impacted files ```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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.