From b1872ec3697b9a61c2a5135ac706094b18e355e5 Mon Sep 17 00:00:00 2001 From: Sergey Egorov Date: Tue, 28 Feb 2017 11:29:41 +0100 Subject: [PATCH] The url.RawPath used when engine.UseRawPath is set to true. (#810) --- gin.go | 25 +++++++++++++++++++--- routes_test.go | 39 +++++++++++++++++++++++++++++++++++ tree.go | 22 +++++++++++++++++--- tree_test.go | 56 ++++++++++++++++++++++++++++++++++++++++++++------ 4 files changed, 130 insertions(+), 12 deletions(-) diff --git a/gin.go b/gin.go index 1205409..61ac5c0 100644 --- a/gin.go +++ b/gin.go @@ -83,6 +83,13 @@ type ( // #726 #755 If enabled, it will thrust some headers starting with // 'X-AppEngine...' for better integration with that PaaS. AppEngine bool + + // If enabled, the url.RawPath will be used to find parameters. + UseRawPath bool + // If true, the path value will be unescaped. + // If UseRawPath is false (by default), the UnescapePathValues effectively is true, + // as url.Path gonna be used, which is already unescaped. + UnescapePathValues bool } ) @@ -94,6 +101,8 @@ var _ IRouter = &Engine{} // - RedirectFixedPath: false // - HandleMethodNotAllowed: false // - ForwardedByClientIP: true +// - UseRawPath: false +// - UnescapePathValues: true func New() *Engine { debugPrintWARNINGNew() engine := &Engine{ @@ -107,6 +116,8 @@ func New() *Engine { HandleMethodNotAllowed: false, ForwardedByClientIP: true, AppEngine: defaultAppEngine, + UseRawPath: false, + UnescapePathValues: true, trees: make(methodTrees, 0, 9), } engine.RouterGroup.engine = engine @@ -284,7 +295,15 @@ func (engine *Engine) HandleContext(c *Context) { func (engine *Engine) handleHTTPRequest(context *Context) { httpMethod := context.Request.Method - path := context.Request.URL.Path + var path string + var unescape bool + if engine.UseRawPath && len(context.Request.URL.RawPath) > 0 { + path = context.Request.URL.RawPath + unescape = engine.UnescapePathValues + } else { + path = context.Request.URL.Path + unescape = false + } // Find root of the tree for the given HTTP method t := engine.trees @@ -292,7 +311,7 @@ func (engine *Engine) handleHTTPRequest(context *Context) { if t[i].method == httpMethod { root := t[i].root // Find route in tree - handlers, params, tsr := root.getValue(path, context.Params) + handlers, params, tsr := root.getValue(path, context.Params, unescape) if handlers != nil { context.handlers = handlers context.Params = params @@ -317,7 +336,7 @@ func (engine *Engine) handleHTTPRequest(context *Context) { if engine.HandleMethodNotAllowed { for _, tree := range engine.trees { if tree.method != httpMethod { - if handlers, _, _ := tree.root.getValue(path, nil); handlers != nil { + if handlers, _, _ := tree.root.getValue(path, nil, unescape); handlers != nil { context.handlers = engine.allNoMethod serveError(context, 405, default405Body) return diff --git a/routes_test.go b/routes_test.go index 32f0098..7464d5d 100644 --- a/routes_test.go +++ b/routes_test.go @@ -400,3 +400,42 @@ func TestRouterNotFound(t *testing.T) { w = performRequest(router, "GET", "/") assert.Equal(t, w.Code, 404) } + +func TestRouteRawPath(t *testing.T) { + route := New() + route.UseRawPath = true + + route.POST("/project/:name/build/:num", func(c *Context) { + name := c.Params.ByName("name") + num := c.Params.ByName("num") + + assert.Equal(t, c.Param("name"), name) + assert.Equal(t, c.Param("num"), num) + + assert.Equal(t, "Some/Other/Project", name) + assert.Equal(t, "222", num) + }) + + w := performRequest(route, "POST", "/project/Some%2FOther%2FProject/build/222") + assert.Equal(t, w.Code, 200) +} + +func TestRouteRawPathNoUnescape(t *testing.T) { + route := New() + route.UseRawPath = true + route.UnescapePathValues = false + + route.POST("/project/:name/build/:num", func(c *Context) { + name := c.Params.ByName("name") + num := c.Params.ByName("num") + + assert.Equal(t, c.Param("name"), name) + assert.Equal(t, c.Param("num"), num) + + assert.Equal(t, "Some%2FOther%2FProject", name) + assert.Equal(t, "333", num) + }) + + w := performRequest(route, "POST", "/project/Some%2FOther%2FProject/build/333") + assert.Equal(t, w.Code, 200) +} diff --git a/tree.go b/tree.go index 4f1da27..eee6bab 100644 --- a/tree.go +++ b/tree.go @@ -5,6 +5,7 @@ package gin import ( + "net/url" "strings" "unicode" ) @@ -363,7 +364,7 @@ func (n *node) insertChild(numParams uint8, path string, fullPath string, handle // If no handle can be found, a TSR (trailing slash redirect) recommendation is // made if a handle exists with an extra (without the) trailing slash for the // given path. -func (n *node) getValue(path string, po Params) (handlers HandlersChain, p Params, tsr bool) { +func (n *node) getValue(path string, po Params, unescape bool) (handlers HandlersChain, p Params, tsr bool) { p = po walk: // Outer loop for walking the tree for { @@ -406,7 +407,15 @@ walk: // Outer loop for walking the tree i := len(p) p = p[:i+1] // expand slice within preallocated capacity p[i].Key = n.path[1:] - p[i].Value = path[:end] + val := path[:end] + if unescape { + var err error + if p[i].Value, err = url.QueryUnescape(val); err != nil { + p[i].Value = val // fallback, in case of error + } + } else { + p[i].Value = val + } // we need to go deeper! if end < len(path) { @@ -440,7 +449,14 @@ walk: // Outer loop for walking the tree i := len(p) p = p[:i+1] // expand slice within preallocated capacity p[i].Key = n.path[2:] - p[i].Value = path + if unescape { + var err error + if p[i].Value, err = url.QueryUnescape(path); err != nil { + p[i].Value = path // fallback, in case of error + } + } else { + p[i].Value = path + } handlers = n.handlers return diff --git a/tree_test.go b/tree_test.go index ed21783..22f0131 100644 --- a/tree_test.go +++ b/tree_test.go @@ -37,9 +37,14 @@ type testRequests []struct { ps Params } -func checkRequests(t *testing.T, tree *node, requests testRequests) { +func checkRequests(t *testing.T, tree *node, requests testRequests, unescapes ...bool) { + unescape := false + if len(unescapes) >= 1 { + unescape = unescapes[0] + } + for _, request := range requests { - handler, ps, _ := tree.getValue(request.path, nil) + handler, ps, _ := tree.getValue(request.path, nil, unescape) if handler == nil { if !request.nilHandler { @@ -197,6 +202,45 @@ func TestTreeWildcard(t *testing.T) { checkMaxParams(t, tree) } +func TestUnescapeParameters(t *testing.T) { + tree := &node{} + + routes := [...]string{ + "/", + "/cmd/:tool/:sub", + "/cmd/:tool/", + "/src/*filepath", + "/search/:query", + "/files/:dir/*filepath", + "/info/:user/project/:project", + "/info/:user", + } + for _, route := range routes { + tree.addRoute(route, fakeHandler(route)) + } + + //printChildren(tree, "") + unescape := true + checkRequests(t, tree, testRequests{ + {"/", false, "/", nil}, + {"/cmd/test/", false, "/cmd/:tool/", Params{Param{"tool", "test"}}}, + {"/cmd/test", true, "", Params{Param{"tool", "test"}}}, + {"/src/some/file.png", false, "/src/*filepath", Params{Param{"filepath", "/some/file.png"}}}, + {"/src/some/file+test.png", false, "/src/*filepath", Params{Param{"filepath", "/some/file test.png"}}}, + {"/src/some/file++++%%%%test.png", false, "/src/*filepath", Params{Param{"filepath", "/some/file++++%%%%test.png"}}}, + {"/src/some/file%2Ftest.png", false, "/src/*filepath", Params{Param{"filepath", "/some/file/test.png"}}}, + {"/search/someth!ng+in+ünìcodé", false, "/search/:query", Params{Param{"query", "someth!ng in ünìcodé"}}}, + {"/info/gordon/project/go", false, "/info/:user/project/:project", Params{Param{"user", "gordon"}, Param{"project", "go"}}}, + {"/info/slash%2Fgordon", false, "/info/:user", Params{Param{"user", "slash/gordon"}}}, + {"/info/slash%2Fgordon/project/Project%20%231", false, "/info/:user/project/:project", Params{Param{"user", "slash/gordon"}, Param{"project", "Project #1"}}}, + {"/info/slash%%%%", false, "/info/:user", Params{Param{"user", "slash%%%%"}}}, + {"/info/slash%%%%2Fgordon/project/Project%%%%20%231", false, "/info/:user/project/:project", Params{Param{"user", "slash%%%%2Fgordon"}, Param{"project", "Project%%%%20%231"}}}, + }, unescape) + + checkPriorities(t, tree) + checkMaxParams(t, tree) +} + func catchPanic(testFunc func()) (recv interface{}) { defer func() { recv = recover() @@ -430,7 +474,7 @@ func TestTreeTrailingSlashRedirect(t *testing.T) { "/doc/", } for _, route := range tsrRoutes { - handler, _, tsr := tree.getValue(route, nil) + handler, _, tsr := tree.getValue(route, nil, false) if handler != nil { t.Fatalf("non-nil handler for TSR route '%s", route) } else if !tsr { @@ -447,7 +491,7 @@ func TestTreeTrailingSlashRedirect(t *testing.T) { "/api/world/abc", } for _, route := range noTsrRoutes { - handler, _, tsr := tree.getValue(route, nil) + handler, _, tsr := tree.getValue(route, nil, false) if handler != nil { t.Fatalf("non-nil handler for No-TSR route '%s", route) } else if tsr { @@ -466,7 +510,7 @@ func TestTreeRootTrailingSlashRedirect(t *testing.T) { t.Fatalf("panic inserting test route: %v", recv) } - handler, _, tsr := tree.getValue("/", nil) + handler, _, tsr := tree.getValue("/", nil, false) if handler != nil { t.Fatalf("non-nil handler") } else if tsr { @@ -617,7 +661,7 @@ func TestTreeInvalidNodeType(t *testing.T) { // normal lookup recv := catchPanic(func() { - tree.getValue("/test", nil) + tree.getValue("/test", nil, false) }) if rs, ok := recv.(string); !ok || rs != panicMsg { t.Fatalf("Expected panic '"+panicMsg+"', got '%v'", recv)