Repository: cloudwego/hertz
Pull request #1024 (2023-12-12 09:16:00)

chore: update comment about MaxKeepBodySize option
#### What type of PR is this?
chore

#### 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.
- [ ] 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.
更新有关 MaxKeepBodySize 选项的注释

#### (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/1024?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:
> Comparison is base [(`82f9bfa`)](https://app.codecov.io/gh/cloudwego/hertz/commit/82f9bfa7e68d6fd724a39350f3b517ec68ab54fc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) 82.55% compared to head [(`afa2fdc`)](https://app.codecov.io/gh/cloudwego/hertz/pull/1024?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) 82.58%.


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


```diff
@@             Coverage Diff             @@
##           develop    #1024      +/-   ##
===========================================
+ Coverage    82.55%   82.58%   +0.03%     
===========================================
  Files           98       98              
  Lines         9974     9974              
===========================================
+ Hits          8234     8237       +3     
+ Misses        1244     1241       -3     
  Partials       496      496              
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/cloudwego/hertz/pull/1024?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/app/server/option.go b/pkg/app/server/option.go
index 18f1843..73188d1 100644
--- a/pkg/app/server/option.go
+++ b/pkg/app/server/option.go
@@ -188,6 +188,7 @@ func WithMaxRequestBodySize(bs int) config.Option {
 // WithMaxKeepBodySize sets max size of request/response body to keep when recycled. Unit: byte
 //
 // Body buffer which larger than this size will be put back into buffer poll.
+// Note: If memory pressure is high, try setting the value to 0.
 func WithMaxKeepBodySize(bs int) config.Option {
 	return config.Option{F: func(o *config.Options) {
 		o.MaxKeepBodySize = bs
diff --git a/pkg/common/config/option.go b/pkg/common/config/option.go
index 958d9b3..38cbfb3 100644
--- a/pkg/common/config/option.go
+++ b/pkg/common/config/option.go
@@ -220,9 +220,9 @@ func NewOptions(opts []Option) *Options {
 		// an error will be returned
 		MaxRequestBodySize: defaultMaxRequestBodySize,
 
-		// max reserved body buffer size when reset Request & Request
-		// If the body size exceeds this value, then the buffer won't be put to
-		// sync.Pool to prevent OOM
+		// max reserved body buffer size when reset Request & Response
+		// If the body size exceeds this value, then the buffer will be put to
+		// sync.Pool instead of hold by Request/Response directly.
 		MaxKeepBodySize: defaultMaxRequestBodySize,
 
 		// only accept GET request
diff --git a/pkg/protocol/request.go b/pkg/protocol/request.go
index d047318..8e4b40b 100644
--- a/pkg/protocol/request.go
+++ b/pkg/protocol/request.go
@@ -65,7 +65,7 @@ import (
 )
 
 var (
-	errMissingFile = errors.NewPublic("http: no such file")
+	ErrMissingFile = errors.NewPublic("http: no such file")
 
 	responseBodyPool bytebufferpool.Pool
 	requestBodyPool  bytebufferpool.Pool
@@ -313,7 +313,7 @@ func (req *Request) FormFile(name string) (*multipart.FileHeader, error) {
 	}
 	fhh := mf.File[name]
 	if fhh == nil {
-		return nil, errMissingFile
+		return nil, ErrMissingFile
 	}
 	return fhh[0], nil
 }
diff --git a/pkg/route/engine.go b/pkg/route/engine.go
index 30b899d..4881ee9 100644
--- a/pkg/route/engine.go
+++ b/pkg/route/engine.go
@@ -75,6 +75,7 @@ import (
 	"github.com/cloudwego/hertz/pkg/protocol/http1"
 	"github.com/cloudwego/hertz/pkg/protocol/http1/factory"
 	"github.com/cloudwego/hertz/pkg/protocol/suite"
+	"github.com/cloudwego/hertz/pkg/route/param"
 )
 
 const unknownTransporterName = "unknown"
@@ -749,6 +750,12 @@ func (engine *Engine) ServeHTTP(c context.Context, ctx *app.RequestContext) {
 		return
 	}
 
+	// if Params is re-assigned in HandlerFunc and the capacity is not enough we need to realloc
+	maxParams := int(engine.maxParams)
+	if cap(ctx.Params) < maxParams {
+		ctx.Params = make(param.Params, 0, maxParams)
+	}
+
 	// Find root of the tree for the given HTTP method
 	t := engine.trees
 	paramsPointer := &ctx.Params


Test patch:
diff --git a/pkg/common/utils/utils_test.go b/pkg/common/utils/utils_test.go
index 92873b5..225d8e4 100644
--- a/pkg/common/utils/utils_test.go
+++ b/pkg/common/utils/utils_test.go
@@ -49,7 +49,34 @@ import (
 
 // test assert func
 func TestUtilsAssert(t *testing.T) {
-	// nothing to test
+	assertPanic := func() (panicked bool) {
+		defer func() {
+			if r := recover(); r != nil {
+				panicked = true
+			}
+		}()
+		Assert(false, "should panic")
+		return false
+	}
+
+	// Checking if the assertPanic function results in a panic as expected.
+	// We expect a true value because it should panic.
+	assert.DeepEqual(t, true, assertPanic())
+
+	// Checking if a true assertion does not result in a panic.
+	// We create a wrapper around Assert to capture if it panics when it should not.
+	noPanic := func() (panicked bool) {
+		defer func() {
+			if r := recover(); r != nil {
+				panicked = true
+			}
+		}()
+		Assert(true, "should not panic")
+		return false
+	}
+
+	// We expect a false value because it should not panic.
+	assert.DeepEqual(t, false, noPanic())
 }
 
 func TestUtilsIsTrueString(t *testing.T) {
@@ -142,3 +169,18 @@ func TestFilterContentType(t *testing.T) {
 	contentType = FilterContentType(contentType)
 	assert.DeepEqual(t, "text/plain", contentType)
 }
+
+func TestNormalizeHeaderKeyEdgeCases(t *testing.T) {
+	empty := []byte("")
+	NormalizeHeaderKey(empty, false)
+	assert.DeepEqual(t, []byte(""), empty)
+	NormalizeHeaderKey(empty, true)
+	assert.DeepEqual(t, []byte(""), empty)
+}
+
+func TestFilterContentTypeEdgeCases(t *testing.T) {
+	simpleContentType := "text/plain"
+	assert.DeepEqual(t, "text/plain", FilterContentType(simpleContentType))
+	complexContentType := "text/html; charset=utf-8; format=flowed"
+	assert.DeepEqual(t, "text/html", FilterContentType(complexContentType))
+}
diff --git a/pkg/route/engine_test.go b/pkg/route/engine_test.go
index b3e0adb..ea1bc5f 100644
--- a/pkg/route/engine_test.go
+++ b/pkg/route/engine_test.go
@@ -1029,3 +1029,33 @@ func TestAcquireHijackConn(t *testing.T) {
 	assert.DeepEqual(t, engine, hijackConn.e)
 	assert.DeepEqual(t, conn, hijackConn.Conn)
 }
+
+func TestHandleParamsReassignInHandleFunc(t *testing.T) {
+	e := NewEngine(config.NewOptions(nil))
+	routes := []string{
+		"/:a/:b/:c",
+	}
+	for _, r := range routes {
+		e.GET(r, func(c context.Context, ctx *app.RequestContext) {
+			ctx.Params = make([]param.Param, 1)
+			ctx.String(consts.StatusOK, "")
+		})
+	}
+	testRoutes := []string{
+		"/aaa/bbb/ccc",
+		"/asd/alskja/alkdjad",
+		"/asd/alskja/alkdjad",
+		"/asd/alskja/alkdjad",
+		"/asd/alskja/alkdjad",
+		"/alksjdlakjd/ooo/askda",
+		"/alksjdlakjd/ooo/askda",
+		"/alksjdlakjd/ooo/askda",
+	}
+	ctx := e.ctxPool.Get().(*app.RequestContext)
+	for _, tr := range testRoutes {
+		r := protocol.NewRequest(http.MethodGet, tr, nil)
+		r.CopyTo(&ctx.Request)
+		e.ServeHTTP(context.Background(), ctx)
+		ctx.ResetWithoutConn()
+	}
+}