Repository: cloudwego/hertz
Pull request #1341 (2025-04-24 16:48:33)

feat(server): detect request race
#### What type of PR is this?
feat

<!--
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:
-->
- [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.


#### (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: detect request race when parsing a new request and user is reading the old request in background
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/1341?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 73.45%. Comparing base [(`cf38573`)](https://app.codecov.io/gh/cloudwego/hertz/commit/cf38573ce4d4eb6a6737e19398ad95e5211ef128?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) to head [(`ffcca19`)](https://app.codecov.io/gh/cloudwego/hertz/commit/ffcca19c207fc5374ad6193d1d801240b378b70e?dropdown=coverage&el=desc&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    #1341      +/-   ##
===========================================
+ Coverage    73.43%   73.45%   +0.01%     
===========================================
  Files          144      144              
  Lines        16569    16580      +11     
===========================================
+ Hits         12168    12179      +11     
  Misses        3813     3813              
  Partials       588      588              
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/cloudwego/hertz/pull/1341?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/pkg/protocol/http1/server.go b/pkg/protocol/http1/server.go
index c4b86de..8152018 100644
--- a/pkg/protocol/http1/server.go
+++ b/pkg/protocol/http1/server.go
@@ -31,6 +31,7 @@ import (
 	"github.com/cloudwego/hertz/pkg/app"
 	"github.com/cloudwego/hertz/pkg/app/server/render"
 	errs "github.com/cloudwego/hertz/pkg/common/errors"
+	"github.com/cloudwego/hertz/pkg/common/hlog"
 	"github.com/cloudwego/hertz/pkg/common/tracer/stats"
 	"github.com/cloudwego/hertz/pkg/common/tracer/traceinfo"
 	"github.com/cloudwego/hertz/pkg/common/utils"
@@ -328,6 +329,18 @@ func (s Server) Serve(c context.Context, conn network.Conn) (err error) {
 			statefulConn.DetectConnectionClose()
 		}
 
+		if ctx.Request.IsURIParsed() {
+			// ctx.Request.URI() must not be called before ServeHTTP
+			// The only case is concurrency issue when parsing a new request,
+			// and user is reading the old request in background.
+			hlog.SystemLogger().Warnf("%s\n%s\n%s\n%s",
+				"Race detected.",
+				"Please be aware that the protocol.Request passed to handler is only valid before the handler returns.",
+				"DO NOT attempt to keep and access protocol.Request after the handler returns.",
+				"Try build with -race to check the race issue.")
+			return errors.New("race detected")
+		}
+
 		// Handle the request
 		//
 		// NOTE: All middlewares and business handler will be executed in this. And at this point, the request has been parsed


Test patch:
diff --git a/pkg/protocol/http1/client_test.go b/pkg/protocol/http1/client_test.go
index 715a2ce..f73bea7 100644
--- a/pkg/protocol/http1/client_test.go
+++ b/pkg/protocol/http1/client_test.go
@@ -324,14 +324,11 @@ func TestDoNonNilReqResp(t *testing.T) {
 	assert.DeepEqual(t, resp.Body(), []byte("123456"))
 }
 
-func TestDoNonNilReqResp1(t *testing.T) {
+func TestDoNonNilReqResp_Err(t *testing.T) {
 	c := &HostClient{
 		ClientOptions: &ClientOptions{
 			Dialer: newSlowConnDialer(func(network, addr string, timeout time.Duration) (network.Conn, error) {
-				return &writeErrConn{
-						Conn: mock.NewConn(""),
-					},
-					nil
+				return peekErrConn{writeErrConn{mock.NewConn("")}}, nil
 			}),
 		},
 	}
@@ -341,6 +338,7 @@ func TestDoNonNilReqResp1(t *testing.T) {
 	retry, err := c.doNonNilReqResp(req, resp)
 	assert.True(t, retry)
 	assert.NotNil(t, err)
+	assert.Assert(t, err == errs.ErrConnectionClosed, err) // returned by writeErrConn
 }
 
 func doGET(t *testing.T, addr string, path string) *protocol.Response {
@@ -602,6 +600,14 @@ func (w writeErrConn) WriteBinary(b []byte) (n int, err error) {
 	return 0, errs.ErrConnectionClosed
 }
 
+type peekErrConn struct {
+	network.Conn
+}
+
+func (c peekErrConn) Peek(n int) ([]byte, error) {
+	return nil, errors.New("peek err")
+}
+
 type retryConn struct {
 	network.Conn
 }
diff --git a/pkg/protocol/http1/server_test.go b/pkg/protocol/http1/server_test.go
index a273a51..491c81a 100644
--- a/pkg/protocol/http1/server_test.go
+++ b/pkg/protocol/http1/server_test.go
@@ -260,6 +260,25 @@ func TestServerDisableReqCtxPool(t *testing.T) {
 	assert.Nil(t, err)
 }
 
+func TestServer_RaceDetect(t *testing.T) {
+	c := &app.RequestContext{}
+	_ = c.Request.URI() // parsedURI = true
+	m := &mockCore{
+		ctxPool: &sync.Pool{New: func() interface{} {
+			return c
+		}},
+		mockHandler: func(c context.Context, ctx *app.RequestContext) {
+			panic("must not be called")
+		},
+	}
+	s := &Server{}
+	s.Core = m
+	conn := mock.NewConn("GET / HTTP/1.1\nHost: foobar.com\n\n")
+	err := s.Serve(context.Background(), conn)
+	assert.NotNil(t, err)
+	assert.Assert(t, err.Error() == "race detected")
+}
+
 func TestHijackResponseWriter(t *testing.T) {
 	server := &Server{}
 	reqCtx := &app.RequestContext{}