mirror of
https://github.com/gin-gonic/gin.git
synced 2026-10-28 11:04:42 +08:00
fix(tree): grow skippedNodes when pooled capacity is stale (#4852)
getValue resliced the backtrack stack to index+1 on the assumption that its capacity equals Engine.maxSections. The pool captures that capacity at allocation. Routes registered afterwards raise maxSections without resizing the pooled slice. The next request that needs one more frame panics inside the router, before Recovery. append grows the slice when the captured capacity is short and preserves the existing backtrack behaviour when it is already sufficient. The reset at entry stays: getValue still runs once per method tree on the same stack. Co-authored-by: hazyhaar <hazyhaarlabs@gmail.com> Co-authored-by: Bo-Yi Wu <appleboy.tw@gmail.com>
This commit is contained in:
2 files changed
+31
-8
No files matched your search
@@ -847,6 +847,30 @@ func TestEngineHandleMethodNotAllowedCornerCase(t *testing.T) {
|
||||
assert.Equal(t, http.StatusNotFound, w.Code)
|
||||
}
|
||||
|
||||
func TestLateRouteRegistrationSkippedNodes(t *testing.T) {
|
||||
router := New()
|
||||
router.HandleMethodNotAllowed = true
|
||||
router.GET("/a", func(c *Context) { c.Status(http.StatusOK) })
|
||||
|
||||
w := PerformRequest(router, http.MethodGet, "/a")
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("warmup status = %d, want 200", w.Code)
|
||||
}
|
||||
|
||||
h := func(c *Context) {}
|
||||
router.GET("/x/y/z/w", h)
|
||||
router.GET("/x/y/:id/w", h)
|
||||
router.GET("/x/:id/z/w", h)
|
||||
router.GET("/:id/y/z/w", h)
|
||||
|
||||
req := httptest.NewRequest(http.MethodPost, "/x/y/z/w/extra", nil)
|
||||
w = httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
if w.Code != http.StatusNotFound {
|
||||
t.Fatalf("status = %d, want 404", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// TestIssue4818_skippedNodesOverflow_Panic reproduces the panic from issue #4818:
|
||||
// with HandleMethodNotAllowed enabled, getValue is called once per method tree
|
||||
// reusing the same c.skippedNodes stack without resetting it, so residue
|
||||
|
||||
@@ -420,10 +420,11 @@ func (n *node) getValue(path string, params *Params, skippedNodes *[]skippedNode
|
||||
|
||||
// Reset the skipped-nodes stack on entry. getValue is called once per
|
||||
// method tree (e.g. in the HandleMethodNotAllowed loop) reusing the same
|
||||
// pooled Context stack, and the walk below grows it via a raw reslice that
|
||||
// cannot exceed engine.maxSections. Without this reset the residue from a
|
||||
// previous tree leaks into the next call and can panic with "slice bounds
|
||||
// out of range" once the accumulated length passes the capacity.
|
||||
// pooled Context stack. Without this reset the residue from a previous
|
||||
// tree leaks into the next call. The walk grows the stack with append:
|
||||
// a context allocated before addRoute raised maxSections must still be
|
||||
// able to record one more backtrack frame. A raw reslice of the capacity
|
||||
// captured at pool allocation panics with "slice bounds out of range".
|
||||
*skippedNodes = (*skippedNodes)[:0]
|
||||
|
||||
walk: // Outer loop for walking the tree
|
||||
@@ -439,9 +440,7 @@ walk: // Outer loop for walking the tree
|
||||
if c == idxc {
|
||||
// strings.HasPrefix(n.children[len(n.children)-1].path, ":") == n.wildChild
|
||||
if n.wildChild {
|
||||
index := len(*skippedNodes)
|
||||
*skippedNodes = (*skippedNodes)[:index+1]
|
||||
(*skippedNodes)[index] = skippedNode{
|
||||
*skippedNodes = append(*skippedNodes, skippedNode{
|
||||
path: prefix + path,
|
||||
node: &node{
|
||||
path: n.path,
|
||||
@@ -453,7 +452,7 @@ walk: // Outer loop for walking the tree
|
||||
fullPath: n.fullPath,
|
||||
},
|
||||
paramsCount: globalParamsCount,
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
n = n.children[i]
|
||||
|
||||
Reference in new issue
Block a user