From 4a23c4f7b9ced6b8e4476f2e021a61165153b71d Mon Sep 17 00:00:00 2001 From: Sai Date: Mon, 11 Mar 2019 11:52:47 +0900 Subject: [PATCH] fix #1804 which is caused by calling middleware twice. (#1805) Fix: https://github.com/gin-gonic/gin/issues/1804 `allNoRoute` contains middlewares such as `gin.Logger`, `gin.Recovery`, so on. The correct code is to use `noRoute`. cc: @MetalBreaker --- routergroup.go | 2 +- routes_test.go | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/routergroup.go b/routergroup.go index 297d3574e9..a1e6c928b2 100644 --- a/routergroup.go +++ b/routergroup.go @@ -195,7 +195,7 @@ func (group *RouterGroup) createStaticHandler(relativePath string, fs http.FileS // Check if file exists and/or if we have permission to access it if _, err := fs.Open(file); err != nil { c.Writer.WriteHeader(http.StatusNotFound) - c.handlers = group.engine.allNoRoute + c.handlers = group.engine.noRoute // Reset index c.index = -1 return diff --git a/routes_test.go b/routes_test.go index a842704f5a..de363a8ca4 100644 --- a/routes_test.go +++ b/routes_test.go @@ -429,7 +429,6 @@ func TestRouterNotFound(t *testing.T) { func TestRouterStaticFSNotFound(t *testing.T) { router := New() - router.StaticFS("/", http.FileSystem(http.Dir("/thisreallydoesntexist/"))) router.NoRoute(func(c *Context) { c.String(404, "non existent") @@ -452,6 +451,27 @@ func TestRouterStaticFSFileNotFound(t *testing.T) { }) } +// Reproduction test for the bug of issue #1805 +func TestMiddlewareCalledOnceByRouterStaticFSNotFound(t *testing.T) { + router := New() + + // Middleware must be called just only once by per request. + middlewareCalledNum := 0 + router.Use(func(c *Context) { + middlewareCalledNum += 1 + }) + + router.StaticFS("/", http.FileSystem(http.Dir("/thisreallydoesntexist/"))) + + // First access + performRequest(router, "GET", "/nonexistent") + assert.Equal(t, 1, middlewareCalledNum) + + // Second access + performRequest(router, "HEAD", "/nonexistent") + assert.Equal(t, 2, middlewareCalledNum) +} + func TestRouteRawPath(t *testing.T) { route := New() route.UseRawPath = true