From 1092232f448c0dfef9d2f211c7b0ca25a1d86af8 Mon Sep 17 00:00:00 2001 From: pawannn Date: Sat, 1 Nov 2025 01:40:35 +0530 Subject: [PATCH 1/7] fix: call updateRouteTrees in ServeHTTP using sync.Once to support literal colon routes in all usage scenarios (#4413) --- gin.go | 9 ++++ literal_colon_test.go | 111 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+) create mode 100644 literal_colon_test.go diff --git a/gin.go b/gin.go index 1965a429a9..217f86588e 100644 --- a/gin.go +++ b/gin.go @@ -94,6 +94,11 @@ const ( type Engine struct { RouterGroup + // routeTreesUpdated ensures that the initialization or update of the route trees + // (used for routing HTTP requests) happens only once, even if called multiple times + // concurrently. It helps prevent race conditions and redundant setup operations. + routeTreesUpdated sync.Once + // RedirectTrailingSlash enables automatic redirection if the current route can't be matched but a // handler for the path with (without) the trailing slash exists. // For example if /foo/ is requested but a route only exists for /foo, the @@ -635,6 +640,10 @@ func (engine *Engine) RunListener(listener net.Listener) (err error) { // ServeHTTP conforms to the http.Handler interface. func (engine *Engine) ServeHTTP(w http.ResponseWriter, req *http.Request) { + engine.routeTreesUpdated.Do(func() { + engine.updateRouteTrees() + }) + c := engine.pool.Get().(*Context) c.writermem.reset(w) c.Request = req diff --git a/literal_colon_test.go b/literal_colon_test.go new file mode 100644 index 0000000000..4a4985a719 --- /dev/null +++ b/literal_colon_test.go @@ -0,0 +1,111 @@ +package gin + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestLiteralColonWithRun(t *testing.T) { + SetMode(TestMode) + router := New() + + router.GET(`/test\:action`, func(c *Context) { + c.JSON(http.StatusOK, H{"path": "literal_colon"}) + }) + + router.updateRouteTrees() + + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/test:action", nil) + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Contains(t, w.Body.String(), "literal_colon") +} + +func TestLiteralColonWithDirectServeHTTP(t *testing.T) { + SetMode(TestMode) + router := New() + + router.GET(`/test\:action`, func(c *Context) { + c.JSON(http.StatusOK, H{"path": "literal_colon"}) + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/test:action", nil) + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Contains(t, w.Body.String(), "literal_colon") +} + +func TestLiteralColonWithHandler(t *testing.T) { + + SetMode(TestMode) + router := New() + + router.GET(`/test\:action`, func(c *Context) { + c.JSON(http.StatusOK, H{"path": "literal_colon"}) + }) + + handler := router.Handler() + + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/test:action", nil) + handler.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Contains(t, w.Body.String(), "literal_colon") +} + +func TestLiteralColonWithHTTPServer(t *testing.T) { + SetMode(TestMode) + router := New() + + router.GET(`/test\:action`, func(c *Context) { + c.JSON(http.StatusOK, H{"path": "literal_colon"}) + }) + + router.GET("/test/:param", func(c *Context) { + c.JSON(http.StatusOK, H{"param": c.Param("param")}) + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/test:action", nil) + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Contains(t, w.Body.String(), "literal_colon") + + w2 := httptest.NewRecorder() + req2, _ := http.NewRequest("GET", "/test/foo", nil) + router.ServeHTTP(w2, req2) + + assert.Equal(t, http.StatusOK, w2.Code) + assert.Contains(t, w2.Body.String(), "foo") +} + +// Test that updateRouteTrees is called only once +func TestUpdateRouteTreesCalledOnce(t *testing.T) { + SetMode(TestMode) + router := New() + + callCount := 0 + originalUpdate := router.updateRouteTrees + + router.GET(`/test\:action`, func(c *Context) { + c.JSON(http.StatusOK, H{"call": callCount}) + }) + + for i := 0; i < 5; i++ { + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/test:action", nil) + router.ServeHTTP(w, req) + assert.Equal(t, http.StatusOK, w.Code) + } + + _ = originalUpdate +} From 22d8cd65709f2618476c86a5bd26c5e8e527b0f7 Mon Sep 17 00:00:00 2001 From: pawannn Date: Mon, 3 Nov 2025 10:14:48 +0530 Subject: [PATCH 2/7] chore: fixed golangci-lint issue in test cases for literal colon --- literal_colon_test.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/literal_colon_test.go b/literal_colon_test.go index 4a4985a719..f3663d7b18 100644 --- a/literal_colon_test.go +++ b/literal_colon_test.go @@ -19,7 +19,8 @@ func TestLiteralColonWithRun(t *testing.T) { router.updateRouteTrees() w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", "/test:action", nil) + + req, _ := http.NewRequest(http.MethodGet, "/test:action", nil) router.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) @@ -35,7 +36,7 @@ func TestLiteralColonWithDirectServeHTTP(t *testing.T) { }) w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", "/test:action", nil) + req, _ := http.NewRequest(http.MethodGet, "/test:action", nil) router.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) @@ -54,7 +55,7 @@ func TestLiteralColonWithHandler(t *testing.T) { handler := router.Handler() w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", "/test:action", nil) + req, _ := http.NewRequest(http.MethodGet, "/test:action", nil) handler.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) @@ -74,14 +75,14 @@ func TestLiteralColonWithHTTPServer(t *testing.T) { }) w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", "/test:action", nil) + req, _ := http.NewRequest(http.MethodGet, "/test:action", nil) router.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) assert.Contains(t, w.Body.String(), "literal_colon") w2 := httptest.NewRecorder() - req2, _ := http.NewRequest("GET", "/test/foo", nil) + req2, _ := http.NewRequest(http.MethodGet, "/test/foo", nil) router.ServeHTTP(w2, req2) assert.Equal(t, http.StatusOK, w2.Code) @@ -102,7 +103,7 @@ func TestUpdateRouteTreesCalledOnce(t *testing.T) { for i := 0; i < 5; i++ { w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", "/test:action", nil) + req, _ := http.NewRequest(http.MethodGet, "/test:action", nil) router.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) } From 19e33ded6cef5457867bc3866d3ad19c4cff4106 Mon Sep 17 00:00:00 2001 From: pawannn Date: Thu, 6 Nov 2025 12:04:13 +0530 Subject: [PATCH 3/7] fix: gofumpt formatting issue --- literal_colon_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/literal_colon_test.go b/literal_colon_test.go index f3663d7b18..33eddf8152 100644 --- a/literal_colon_test.go +++ b/literal_colon_test.go @@ -44,7 +44,6 @@ func TestLiteralColonWithDirectServeHTTP(t *testing.T) { } func TestLiteralColonWithHandler(t *testing.T) { - SetMode(TestMode) router := New() From 0771735cd79c9d0badfd91b854c7579c8072976d Mon Sep 17 00:00:00 2001 From: pawannn Date: Mon, 10 Nov 2025 00:09:46 +0530 Subject: [PATCH 4/7] fix: gofumpt issue in gin.go --- gin.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/gin.go b/gin.go index 217f86588e..ff90a246a9 100644 --- a/gin.go +++ b/gin.go @@ -23,10 +23,12 @@ import ( "golang.org/x/net/http2/h2c" ) -const defaultMultipartMemory = 32 << 20 // 32 MB -const escapedColon = "\\:" -const colon = ":" -const backslash = "\\" +const ( + defaultMultipartMemory = 32 << 20 // 32 MB + escapedColon = "\\:" + colon = ":" + backslash = "\\" +) var ( default404Body = []byte("404 page not found") @@ -46,8 +48,10 @@ var defaultTrustedCIDRs = []*net.IPNet{ }, } -var regSafePrefix = regexp.MustCompile("[^a-zA-Z0-9/-]+") -var regRemoveRepeatedChar = regexp.MustCompile("/{2,}") +var ( + regSafePrefix = regexp.MustCompile("[^a-zA-Z0-9/-]+") + regRemoveRepeatedChar = regexp.MustCompile("/{2,}") +) // HandlerFunc defines the handler used by gin middleware as return value. type HandlerFunc func(*Context) From 12600156ae0b908a731d31e43554e6fdb9da2148 Mon Sep 17 00:00:00 2001 From: pawannn Date: Mon, 10 Nov 2025 00:16:32 +0530 Subject: [PATCH 5/7] chore: updated routeTreesUpdated comments --- gin.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gin.go b/gin.go index ff90a246a9..56ac689bfb 100644 --- a/gin.go +++ b/gin.go @@ -99,8 +99,7 @@ type Engine struct { RouterGroup // routeTreesUpdated ensures that the initialization or update of the route trees - // (used for routing HTTP requests) happens only once, even if called multiple times - // concurrently. It helps prevent race conditions and redundant setup operations. + // (used for routing HTTP requests) happens only once, even if called multiple times concurrently. routeTreesUpdated sync.Once // RedirectTrailingSlash enables automatic redirection if the current route can't be matched but a From 232172b7e4237f70561e404992267788f33f906d Mon Sep 17 00:00:00 2001 From: pawannn Date: Mon, 10 Nov 2025 00:17:13 +0530 Subject: [PATCH 6/7] chore: removed unused variable and updated TestUpdateRouteTreesCalledOnce testcase --- literal_colon_test.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/literal_colon_test.go b/literal_colon_test.go index 33eddf8152..f6cfba2884 100644 --- a/literal_colon_test.go +++ b/literal_colon_test.go @@ -93,19 +93,15 @@ func TestUpdateRouteTreesCalledOnce(t *testing.T) { SetMode(TestMode) router := New() - callCount := 0 - originalUpdate := router.updateRouteTrees - router.GET(`/test\:action`, func(c *Context) { - c.JSON(http.StatusOK, H{"call": callCount}) + c.String(http.StatusOK, "ok") }) - for i := 0; i < 5; i++ { + for range 5 { w := httptest.NewRecorder() req, _ := http.NewRequest(http.MethodGet, "/test:action", nil) router.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "ok", w.Body.String()) } - - _ = originalUpdate } From e013e64cae56ed4a1d9dcd62cf1b536eaa08187d Mon Sep 17 00:00:00 2001 From: pawannn Date: Sat, 15 Nov 2025 21:01:57 +0530 Subject: [PATCH 7/7] chore: moved tests from literal_colon_test.go into gin_test.go --- gin_test.go | 99 ++++++++++++++++++++++++++++++++++++++ literal_colon_test.go | 107 ------------------------------------------ 2 files changed, 99 insertions(+), 107 deletions(-) delete mode 100644 literal_colon_test.go diff --git a/gin_test.go b/gin_test.go index be07653788..cee1f3cc74 100644 --- a/gin_test.go +++ b/gin_test.go @@ -913,3 +913,102 @@ func TestMethodNotAllowedNoRoute(t *testing.T) { assert.NotPanics(t, func() { g.ServeHTTP(resp, req) }) assert.Equal(t, http.StatusNotFound, resp.Code) } + +// Test the fix for https://github.com/gin-gonic/gin/pull/4415 +func TestLiteralColonWithRun(t *testing.T) { + SetMode(TestMode) + router := New() + + router.GET(`/test\:action`, func(c *Context) { + c.JSON(http.StatusOK, H{"path": "literal_colon"}) + }) + + router.updateRouteTrees() + + w := httptest.NewRecorder() + + req, _ := http.NewRequest(http.MethodGet, "/test:action", nil) + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Contains(t, w.Body.String(), "literal_colon") +} + +func TestLiteralColonWithDirectServeHTTP(t *testing.T) { + SetMode(TestMode) + router := New() + + router.GET(`/test\:action`, func(c *Context) { + c.JSON(http.StatusOK, H{"path": "literal_colon"}) + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/test:action", nil) + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Contains(t, w.Body.String(), "literal_colon") +} + +func TestLiteralColonWithHandler(t *testing.T) { + SetMode(TestMode) + router := New() + + router.GET(`/test\:action`, func(c *Context) { + c.JSON(http.StatusOK, H{"path": "literal_colon"}) + }) + + handler := router.Handler() + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/test:action", nil) + handler.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Contains(t, w.Body.String(), "literal_colon") +} + +func TestLiteralColonWithHTTPServer(t *testing.T) { + SetMode(TestMode) + router := New() + + router.GET(`/test\:action`, func(c *Context) { + c.JSON(http.StatusOK, H{"path": "literal_colon"}) + }) + + router.GET("/test/:param", func(c *Context) { + c.JSON(http.StatusOK, H{"param": c.Param("param")}) + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/test:action", nil) + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Contains(t, w.Body.String(), "literal_colon") + + w2 := httptest.NewRecorder() + req2, _ := http.NewRequest(http.MethodGet, "/test/foo", nil) + router.ServeHTTP(w2, req2) + + assert.Equal(t, http.StatusOK, w2.Code) + assert.Contains(t, w2.Body.String(), "foo") +} + +// Test that updateRouteTrees is called only once +func TestUpdateRouteTreesCalledOnce(t *testing.T) { + SetMode(TestMode) + router := New() + + router.GET(`/test\:action`, func(c *Context) { + c.String(http.StatusOK, "ok") + }) + + for range 5 { + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/test:action", nil) + router.ServeHTTP(w, req) + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "ok", w.Body.String()) + } +} diff --git a/literal_colon_test.go b/literal_colon_test.go deleted file mode 100644 index f6cfba2884..0000000000 --- a/literal_colon_test.go +++ /dev/null @@ -1,107 +0,0 @@ -package gin - -import ( - "net/http" - "net/http/httptest" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestLiteralColonWithRun(t *testing.T) { - SetMode(TestMode) - router := New() - - router.GET(`/test\:action`, func(c *Context) { - c.JSON(http.StatusOK, H{"path": "literal_colon"}) - }) - - router.updateRouteTrees() - - w := httptest.NewRecorder() - - req, _ := http.NewRequest(http.MethodGet, "/test:action", nil) - router.ServeHTTP(w, req) - - assert.Equal(t, http.StatusOK, w.Code) - assert.Contains(t, w.Body.String(), "literal_colon") -} - -func TestLiteralColonWithDirectServeHTTP(t *testing.T) { - SetMode(TestMode) - router := New() - - router.GET(`/test\:action`, func(c *Context) { - c.JSON(http.StatusOK, H{"path": "literal_colon"}) - }) - - w := httptest.NewRecorder() - req, _ := http.NewRequest(http.MethodGet, "/test:action", nil) - router.ServeHTTP(w, req) - - assert.Equal(t, http.StatusOK, w.Code) - assert.Contains(t, w.Body.String(), "literal_colon") -} - -func TestLiteralColonWithHandler(t *testing.T) { - SetMode(TestMode) - router := New() - - router.GET(`/test\:action`, func(c *Context) { - c.JSON(http.StatusOK, H{"path": "literal_colon"}) - }) - - handler := router.Handler() - - w := httptest.NewRecorder() - req, _ := http.NewRequest(http.MethodGet, "/test:action", nil) - handler.ServeHTTP(w, req) - - assert.Equal(t, http.StatusOK, w.Code) - assert.Contains(t, w.Body.String(), "literal_colon") -} - -func TestLiteralColonWithHTTPServer(t *testing.T) { - SetMode(TestMode) - router := New() - - router.GET(`/test\:action`, func(c *Context) { - c.JSON(http.StatusOK, H{"path": "literal_colon"}) - }) - - router.GET("/test/:param", func(c *Context) { - c.JSON(http.StatusOK, H{"param": c.Param("param")}) - }) - - w := httptest.NewRecorder() - req, _ := http.NewRequest(http.MethodGet, "/test:action", nil) - router.ServeHTTP(w, req) - - assert.Equal(t, http.StatusOK, w.Code) - assert.Contains(t, w.Body.String(), "literal_colon") - - w2 := httptest.NewRecorder() - req2, _ := http.NewRequest(http.MethodGet, "/test/foo", nil) - router.ServeHTTP(w2, req2) - - assert.Equal(t, http.StatusOK, w2.Code) - assert.Contains(t, w2.Body.String(), "foo") -} - -// Test that updateRouteTrees is called only once -func TestUpdateRouteTreesCalledOnce(t *testing.T) { - SetMode(TestMode) - router := New() - - router.GET(`/test\:action`, func(c *Context) { - c.String(http.StatusOK, "ok") - }) - - for range 5 { - w := httptest.NewRecorder() - req, _ := http.NewRequest(http.MethodGet, "/test:action", nil) - router.ServeHTTP(w, req) - assert.Equal(t, http.StatusOK, w.Code) - assert.Equal(t, "ok", w.Body.String()) - } -}