Compare commits

...

3 Commits

Author SHA1 Message Date
S-Sarim
382671bed4
Merge a41520ecbf63e739ed16a2c78b6ec44b96548f89 into d3ffc9985281dcf4d3bef604cce4e662b1a327a6 2026-03-18 05:09:31 +08:00
Abhiyan Khanal
d3ffc99852
test(engine): add regression tests for HandleContext with NoRoute (#4571)
Add two regression tests for GitHub issue #1848 to verify that
Context.handlers are properly isolated in HandleContext when used
from a NoRoute handler with group and engine middleware.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Bo-Yi Wu <appleboy.tw@gmail.com>
2026-03-16 20:49:59 +08:00
Syed Sarim Bin Ahmer
a41520ecbf Fix: make Engine.Routes() concurrency-safe (#4457) 2025-12-02 12:47:27 +05:00
3 changed files with 99 additions and 0 deletions

22
engine_test.go Normal file
View File

@ -0,0 +1,22 @@
package gin
import (
"testing"
)
func TestRoutesConcurrent(t *testing.T) {
r := New()
done := make(chan bool)
// Concurrently read routes
go func() {
_ = r.Routes()
done <- true
}()
// Register a route at the same time
r.GET("/", func(c *Context) { c.String(200, "OK") })
<-done
}

5
gin.go
View File

@ -182,6 +182,7 @@ type Engine struct {
noMethod HandlersChain
pool sync.Pool
trees methodTrees
treesMu sync.RWMutex
maxParams uint16
maxSections uint16
trustedProxies []string
@ -362,6 +363,8 @@ func (engine *Engine) rebuild405Handlers() {
}
func (engine *Engine) addRoute(method, path string, handlers HandlersChain) {
engine.treesMu.Lock()
defer engine.treesMu.Unlock()
assert1(path[0] == '/', "path must begin with '/'")
assert1(method != "", "HTTP method can not be empty")
assert1(len(handlers) > 0, "there must be at least one handler")
@ -388,6 +391,8 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) {
// Routes returns a slice of registered routes, including some useful information, such as:
// the http method, path, and the handler name.
func (engine *Engine) Routes() (routes RoutesInfo) {
engine.treesMu.RLock()
defer engine.treesMu.RUnlock()
for _, tree := range engine.trees {
routes = iterate("", tree.method, routes, tree.root)
}

View File

@ -743,6 +743,78 @@ func TestEngineHandleContextPreventsMiddlewareReEntry(t *testing.T) {
assert.Equal(t, int64(1), handlerCounterV2)
}
func TestEngineHandleContextNoRouteWithGroupMiddleware(t *testing.T) {
// Scenario from issue #1848:
// - Engine with no global middleware (gin.New())
// - A group with middleware
// - A route in that group
// - NoRoute handler that redirects via HandleContext
// The group middleware should run exactly once per HandleContext call,
// not accumulate across redirects.
var middlewareCount, handlerCount int64
r := New()
grp := r.Group("", func(c *Context) {
atomic.AddInt64(&middlewareCount, 1)
c.Next()
})
grp.GET("/target", func(c *Context) {
atomic.AddInt64(&handlerCount, 1)
c.String(http.StatusOK, "ok")
})
r.NoRoute(func(c *Context) {
c.Request.URL.Path = "/target"
r.HandleContext(c)
})
// Access a non-existent route to trigger NoRoute -> HandleContext
w := PerformRequest(r, "GET", "/nonexistent")
assert.Equal(t, http.StatusOK, w.Code)
assert.Equal(t, "ok", w.Body.String())
// Middleware and handler should each run exactly once
assert.Equal(t, int64(1), atomic.LoadInt64(&middlewareCount))
assert.Equal(t, int64(1), atomic.LoadInt64(&handlerCount))
}
func TestEngineHandleContextNoRouteWithEngineMiddleware(t *testing.T) {
// When engine middleware exists and NoRoute redirects via HandleContext,
// verify the handlers run the expected number of times.
var engineMwCount, groupMwCount, handlerCount int64
r := New()
r.Use(func(c *Context) {
atomic.AddInt64(&engineMwCount, 1)
c.Next()
})
grp := r.Group("", func(c *Context) {
atomic.AddInt64(&groupMwCount, 1)
c.Next()
})
grp.GET("/target", func(c *Context) {
atomic.AddInt64(&handlerCount, 1)
c.String(http.StatusOK, "ok")
})
r.NoRoute(func(c *Context) {
c.Request.URL.Path = "/target"
r.HandleContext(c)
})
w := PerformRequest(r, "GET", "/nonexistent")
assert.Equal(t, http.StatusOK, w.Code)
assert.Equal(t, "ok", w.Body.String())
// Handler and group middleware should each run once (from HandleContext)
assert.Equal(t, int64(1), atomic.LoadInt64(&handlerCount))
assert.Equal(t, int64(1), atomic.LoadInt64(&groupMwCount))
// Engine middleware runs twice: once for the NoRoute chain, once for the HandleContext chain
// This is expected behavior since HandleContext re-enters the full handler chain
assert.Equal(t, int64(2), atomic.LoadInt64(&engineMwCount))
}
func TestEngineHandleContextUseEscapedPathPercentEncoded(t *testing.T) {
r := New()
r.UseEscapedPath = true