Skip to content

Commit

Permalink
chore(performance): Improve performance for adding RemoveExtraS… (gin…
Browse files Browse the repository at this point in the history
…-gonic#2159)

* chore: Add RemoveExtraSlash flag

* fix testing

Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
  • Loading branch information
appleboy authored and ThomasObenaus committed Feb 19, 2020
1 parent 95602a0 commit dee55d7
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 9 deletions.
10 changes: 9 additions & 1 deletion gin.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ type Engine struct {
// method call.
MaxMultipartMemory int64

// RemoveExtraSlash a parameter can be parsed from the URL even with extra slashes.
// See the PR #1817 and issue #1644
RemoveExtraSlash bool

delims render.Delims
secureJsonPrefix string
HTMLRender render.HTMLRender
Expand Down Expand Up @@ -134,6 +138,7 @@ func New() *Engine {
ForwardedByClientIP: true,
AppEngine: defaultAppEngine,
UseRawPath: false,
RemoveExtraSlash: false,
UnescapePathValues: true,
MaxMultipartMemory: defaultMultipartMemory,
trees: make(methodTrees, 0, 9),
Expand Down Expand Up @@ -385,7 +390,10 @@ func (engine *Engine) handleHTTPRequest(c *Context) {
rPath = c.Request.URL.RawPath
unescape = engine.UnescapePathValues
}
rPath = cleanPath(rPath)

if engine.RemoveExtraSlash {
rPath = cleanPath(rPath)
}

// Find root of the tree for the given HTTP method
t := engine.trees
Expand Down
40 changes: 32 additions & 8 deletions routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ func TestRouteParamsByNameWithExtraSlash(t *testing.T) {
lastName := ""
wild := ""
router := New()
router.RemoveExtraSlash = true
router.GET("/test/:name/:last_name/*wild", func(c *Context) {
name = c.Params.ByName("name")
lastName = c.Params.ByName("last_name")
Expand Down Expand Up @@ -407,6 +408,29 @@ func TestRouteNotAllowedDisabled(t *testing.T) {
assert.Equal(t, http.StatusNotFound, w.Code)
}

func TestRouterNotFoundWithRemoveExtraSlash(t *testing.T) {
router := New()
router.RemoveExtraSlash = true
router.GET("/path", func(c *Context) {})
router.GET("/", func(c *Context) {})

testRoutes := []struct {
route string
code int
location string
}{
{"/../path", http.StatusOK, ""}, // CleanPath
{"/nope", http.StatusNotFound, ""}, // NotFound
}
for _, tr := range testRoutes {
w := performRequest(router, "GET", tr.route)
assert.Equal(t, tr.code, w.Code)
if w.Code != http.StatusNotFound {
assert.Equal(t, tr.location, fmt.Sprint(w.Header().Get("Location")))
}
}
}

func TestRouterNotFound(t *testing.T) {
router := New()
router.RedirectFixedPath = true
Expand All @@ -419,14 +443,14 @@ func TestRouterNotFound(t *testing.T) {
code int
location string
}{
{"/path/", http.StatusMovedPermanently, "/path"}, // TSR -/
{"/dir", http.StatusMovedPermanently, "/dir/"}, // TSR +/
{"/PATH", http.StatusMovedPermanently, "/path"}, // Fixed Case
{"/DIR/", http.StatusMovedPermanently, "/dir/"}, // Fixed Case
{"/PATH/", http.StatusMovedPermanently, "/path"}, // Fixed Case -/
{"/DIR", http.StatusMovedPermanently, "/dir/"}, // Fixed Case +/
{"/../path", http.StatusOK, ""}, // CleanPath
{"/nope", http.StatusNotFound, ""}, // NotFound
{"/path/", http.StatusMovedPermanently, "/path"}, // TSR -/
{"/dir", http.StatusMovedPermanently, "/dir/"}, // TSR +/
{"/PATH", http.StatusMovedPermanently, "/path"}, // Fixed Case
{"/DIR/", http.StatusMovedPermanently, "/dir/"}, // Fixed Case
{"/PATH/", http.StatusMovedPermanently, "/path"}, // Fixed Case -/
{"/DIR", http.StatusMovedPermanently, "/dir/"}, // Fixed Case +/
{"/../path", http.StatusMovedPermanently, "/path"}, // Without CleanPath
{"/nope", http.StatusNotFound, ""}, // NotFound
}
for _, tr := range testRoutes {
w := performRequest(router, "GET", tr.route)
Expand Down

0 comments on commit dee55d7

Please sign in to comment.