Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: hertz panic when edit ctx.Params on HandleFunc #1150

Merged
merged 10 commits into from
Jul 17, 2024
Merged

fix: hertz panic when edit ctx.Params on HandleFunc #1150

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f0a7a96
Override user edit of ctx.Params.
3DRX Jul 8, 2024
60dd9a1
Realloc Params array in context when the capacity changed
3DRX Jul 9, 2024
a0af181
Reduce casting overhead in ResetWithoutConn.
3DRX Jul 9, 2024
c6d27a9
Add test
3DRX Jul 9, 2024
dbea427
Update test, use the context created by engine.
3DRX Jul 10, 2024
182aa48
Move the check & realloc logic closer to calling HandlerFunc
3DRX Jul 10, 2024
20e616b
Get maxParams directly from engine, don't need to save a copy in
3DRX Jul 11, 2024
cb6292e
Merge branch 'develop' into fix/user_edit_params_cause_panic
3DRX Jul 11, 2024
3683763
Params's len should be 0 before being used
3DRX Jul 16, 2024
450ec4b
Change variable name
3DRX Jul 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions pkg/route/engine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -749,6 +750,12 @@ func (engine *Engine) ServeHTTP(c context.Context, ctx *app.RequestContext) {
return
}

// if ctx.Params is re-assigned by user in HandlerFunc and the capacity changed we need to realloc
paramsCnt := int(engine.maxParams)
3DRX marked this conversation as resolved.
Show resolved Hide resolved
if cap(ctx.Params) < paramsCnt {
ctx.Params = make(param.Params, paramsCnt)
3DRX marked this conversation as resolved.
Show resolved Hide resolved
}

// Find root of the tree for the given HTTP method
t := engine.trees
paramsPointer := &ctx.Params
Expand Down
30 changes: 30 additions & 0 deletions pkg/route/engine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
Loading