From 5cf02b6f7f951ffbe128abf57197d54f6d621532 Mon Sep 17 00:00:00 2001 From: water <672684719@qq.com> Date: Thu, 27 Aug 2026 19:42:00 +0800 Subject: [PATCH 1/2] fix: reset skipped-nodes stack on getValue entry to prevent slice overflow panic [#4818] --- tree.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tree.go b/tree.go index 580abbaf..2f4f7047 100644 --- a/tree.go +++ b/tree.go @@ -418,6 +418,14 @@ type skippedNode struct { func (n *node) getValue(path string, params *Params, skippedNodes *[]skippedNode, unescape bool) (value nodeValue) { var globalParamsCount int16 + // 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. + *skippedNodes = (*skippedNodes)[:0] + walk: // Outer loop for walking the tree for { prefix := n.path From b261e00ec27472df759c3c9c29e8b01fd1b1c27a Mon Sep 17 00:00:00 2001 From: water <672684719@qq.com> Date: Thu, 27 Aug 2026 19:42:02 +0800 Subject: [PATCH 2/2] test: add regression test for getValue skipped-nodes overflow panic [#4818] --- issue4818_test.go | 104 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 issue4818_test.go diff --git a/issue4818_test.go b/issue4818_test.go new file mode 100644 index 00000000..027511c1 --- /dev/null +++ b/issue4818_test.go @@ -0,0 +1,104 @@ +package gin + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +// 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 +// accumulates until the reslice exceeds the capacity (engine.maxSections). +// The request must not panic and must fall through to the normal 404 path +// (the path genuinely matches no route, so 405 is not applicable here). +func TestIssue4818_skippedNodesOverflow_Panic(t *testing.T) { + 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() + router.ServeHTTP(w, req) + + // Must not panic; path matches no route, so 404 is the correct outcome. + if w.Code != http.StatusNotFound { + t.Fatalf("expected 404, got %d (a panic would have aborted the test)", w.Code) + } +} + +// TestIssue4818_MethodNotAllowedStillWorks verifies the HandleMethodNotAllowed +// loop still produces a correct 405 with an Allow header after the fix, i.e. +// the reset does not break the loop across multiple method trees. +func TestIssue4818_MethodNotAllowedStillWorks(t *testing.T) { + 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) + + // PATCH /b/clients/42/g is registered under GET but not under PATCH + // -> HandleMethodNotAllowed should yield 405 with an Allow header. + req := httptest.NewRequest(http.MethodPatch, "/b/clients/42/g", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusMethodNotAllowed { + t.Fatalf("expected 405, got %d", w.Code) + } + allow := w.Header().Get("Allow") + if allow == "" { + t.Fatal("expected Allow header to be set for 405") + } +} + +// TestIssue4818_MethodNotAllowedManyTrees stresses the loop across many +// method trees to make sure the skipped-nodes accumulator can never overflow. +func TestIssue4818_MethodNotAllowedManyTrees(t *testing.T) { + router := New() + router.HandleMethodNotAllowed = true + + h := func(c *Context) {} + for _, m := range []string{ + http.MethodGet, http.MethodPost, http.MethodPut, http.MethodDelete, + http.MethodPatch, http.MethodOptions, http.MethodHead, http.MethodConnect, + http.MethodTrace, + } { + router.Handle(m, "/:p0/:p1/a/:p2", h) + router.Handle(m, "/s/:p0/:p1/:p2/:p3/:p4/:p5/d", h) + } + // GET route so that a POST to a GET-only path yields 405 + router.GET("/only-get", h) + + req := httptest.NewRequest(http.MethodPost, "/only-get", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusMethodNotAllowed { + t.Fatalf("expected 405, got %d", w.Code) + } +}