fix: reset skippedNodes before each getValue call in HandleMethodNotAllowed loop

With HandleMethodNotAllowed enabled, handleHTTPRequest searches every
method tree to build the Allow response header. It reuses the pooled
Context's skippedNodes slice across all getValue calls in that loop.

getValue extends the slice with a raw reslice:

    *skippedNodes = (*skippedNodes)[:index+1]  // tree.go:435

The slice has a fixed capacity equal to engine.maxSections (allocated
once when the Context is created). Context.reset() zeroes the length at
the start of each request, but nothing zeroes it between getValue calls
inside the HandleMethodNotAllowed loop. With enough wildcard routes,
residual entries from earlier trees fill the slice until the reslice
exceeds capacity and panics:

    panic: runtime error: slice bounds out of range [:7] with capacity 6

Because the panic fires during route matching — before any handler
runs — gin.Recovery() cannot intercept it. net/http's per-connection
recover logs "http: panic serving …" and closes the connection with
no response; callers see an empty reply or a connection reset.

Fix: zero the slice length before each getValue call in the loop.
This matches the same reset Context.reset() performs at request start
and costs only a single pointer write per method tree.

Add TestMethodNotAllowedSkippedNodesPanic reproducing the exact route
set and request from issue #4818 to guard against regression.

Fixes #4818

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
okxint 2026-09-02 12:31:11 +05:30
parent 3498e1cdbc
commit b05d45dbb1
2 changed files with 37 additions and 0 deletions

5
gin.go
View File

@ -743,6 +743,11 @@ func (engine *Engine) handleHTTPRequest(c *Context) {
if tree.method == httpMethod {
continue
}
// Reset skippedNodes before each getValue call so residue from the
// previous tree does not accumulate. The slice is allocated once per
// pooled Context with a fixed capacity (engine.maxSections); getValue
// extends it with a raw reslice that panics if len exceeds cap.
*c.skippedNodes = (*c.skippedNodes)[:0]
if value := tree.root.getValue(rPath, nil, c.skippedNodes, unescape); value.handlers != nil {
allowed = append(allowed, tree.method)
}

View File

@ -1058,6 +1058,38 @@ func TestMethodNotAllowedNoRoute(t *testing.T) {
assert.Equal(t, http.StatusNotFound, resp.Code)
}
// TestMethodNotAllowedSkippedNodesPanic is a regression test for #4818.
// With HandleMethodNotAllowed enabled, handleHTTPRequest calls getValue once
// per method tree to build the Allow header. Each call may push onto
// c.skippedNodes, which has a fixed capacity (engine.maxSections). Without a
// reset between calls the slice fills up and a subsequent reslice panics with
// "slice bounds out of range". Because the panic occurs before any handler
// runs, gin.Recovery() cannot catch it and the client receives an empty reply.
func TestMethodNotAllowedSkippedNodesPanic(t *testing.T) {
SetMode(ReleaseMode)
router := New()
router.HandleMethodNotAllowed = true
h := func(c *Context) {}
router.OPTIONS("/:p0/:p1/a/:p2", h)
router.GET("/:p0/:p1/a/:p2", h)
router.PATCH("/b/:p0/:p1/c", h)
router.DELETE("/b/:p0/:p1/d/:p3", h)
router.GET("/b/:p0/:p1/e/f", h)
router.POST("/b/:p0/:p1/g/:p4/h", h)
router.OPTIONS("/b/:p0/:p1/g/:p4/h", h)
router.DELETE("/b/cache", h)
router.GET("/b/clients/:p1/g", h)
router.POST("/b/clients/:p1/g", h)
router.PATCH("/b/clients/:p1/g/:p4", h)
router.OPTIONS("/b/clients/:p1/g/:p4", h)
req := httptest.NewRequest(http.MethodPost, "/b/clients/42", nil)
w := httptest.NewRecorder()
assert.NotPanics(t, func() { router.ServeHTTP(w, req) })
assert.Equal(t, http.StatusMethodNotAllowed, w.Code)
}
// Test the fix for https://github.com/gin-gonic/gin/pull/4415
func TestLiteralColonWithRun(t *testing.T) {
SetMode(TestMode)