From 3fc08c9daf819a392adf2dd501b94067f56643b0 Mon Sep 17 00:00:00 2001 From: qm012 Date: Sun, 27 Jun 2021 20:36:18 +0800 Subject: [PATCH 01/12] handle level 1 router add more comments update comment add example fix benchmark not found add comment and update test method gin_integration_test.go#L407 update comment and lastedNode directly assign current node optimize code Optimize the search next router logic optimize code Adjust the matching rules Adjust the matching order update condition code --- .gitignore | 2 +- gin_integration_test.go | 48 +++++++++++++++++++++-- tree.go | 86 ++++++++++++++++++++++++++++------------- tree_test.go | 26 ++++++++++++- 4 files changed, 129 insertions(+), 33 deletions(-) diff --git a/.gitignore b/.gitignore index bdd50c95cf..2ac6ea8cce 100644 --- a/.gitignore +++ b/.gitignore @@ -4,4 +4,4 @@ coverage.out count.out test profile.out -tmp.out +tmp.out \ No newline at end of file diff --git a/gin_integration_test.go b/gin_integration_test.go index 2eb2d52bf8..b8ba6418e9 100644 --- a/gin_integration_test.go +++ b/gin_integration_test.go @@ -22,7 +22,14 @@ import ( "github.com/stretchr/testify/assert" ) -func testRequest(t *testing.T, url string) { +// params[0]=url example:http://127.0.0.1:8080/index (cannot be empty) +// params[1]=response body (custom compare content) +func testRequest(t *testing.T, params ...string) { + + if len(params) == 0 { + t.Fatal("url cannot be empty") + } + tr := &http.Transport{ TLSClientConfig: &tls.Config{ InsecureSkipVerify: true, @@ -30,13 +37,18 @@ func testRequest(t *testing.T, url string) { } client := &http.Client{Transport: tr} - resp, err := client.Get(url) + resp, err := client.Get(params[0]) assert.NoError(t, err) defer resp.Body.Close() body, ioerr := ioutil.ReadAll(resp.Body) assert.NoError(t, ioerr) - assert.Equal(t, "it worked", string(body), "resp body should match") + + var expected = "it worked" + if len(params) > 1 { + expected = params[1] + } + assert.Equal(t, expected, string(body), "resp body should match") assert.Equal(t, "200 OK", resp.Status, "should get a 200") } @@ -373,3 +385,33 @@ func testGetRequestHandler(t *testing.T, h http.Handler, url string) { assert.Equal(t, "it worked", w.Body.String(), "resp body should match") assert.Equal(t, 200, w.Code, "should get a 200") } + +func TestRunDynamicRouting(t *testing.T) { + router := New() + router.GET("/aa/*xx", func(c *Context) { c.String(http.StatusOK, "/aa/*xx") }) + router.GET("/ab/*xx", func(c *Context) { c.String(http.StatusOK, "/ab/*xx") }) + router.GET("/", func(c *Context) { c.String(http.StatusOK, "home") }) + router.GET("/:cc", func(c *Context) { c.String(http.StatusOK, "/:cc") }) + router.GET("/:cc/cc", func(c *Context) { c.String(http.StatusOK, "/:cc/cc") }) + router.GET("/get/test/abc/", func(c *Context) { c.String(http.StatusOK, "/get/test/abc/") }) + router.GET("/get/:param/abc/", func(c *Context) { c.String(http.StatusOK, "/get/:param/abc/") }) + + ts := httptest.NewServer(router) + defer ts.Close() + + testRequest(t, ts.URL+"/", "home") + testRequest(t, ts.URL+"/aa/aa", "/aa/*xx") + testRequest(t, ts.URL+"/ab/ab", "/ab/*xx") + testRequest(t, ts.URL+"/all", "/:cc") + testRequest(t, ts.URL+"/all/cc", "/:cc/cc") + testRequest(t, ts.URL+"/a/cc", "/:cc/cc") + testRequest(t, ts.URL+"/a", "/:cc") + testRequest(t, ts.URL+"/get/test/abc/", "/get/test/abc/") + testRequest(t, ts.URL+"/get/te/abc/", "/get/:param/abc/") + testRequest(t, ts.URL+"/get/xx/abc/", "/get/:param/abc/") + testRequest(t, ts.URL+"/get/tt/abc/", "/get/:param/abc/") + testRequest(t, ts.URL+"/get/a/abc/", "/get/:param/abc/") + testRequest(t, ts.URL+"/get/t/abc/", "/get/:param/abc/") + testRequest(t, ts.URL+"/get/aa/abc/", "/get/:param/abc/") + testRequest(t, ts.URL+"/get/abas/abc/", "/get/:param/abc/") +} diff --git a/tree.go b/tree.go index 4b5656fb08..6eacf6da52 100644 --- a/tree.go +++ b/tree.go @@ -118,11 +118,6 @@ type node struct { fullPath string } -type skip struct { - path string - paramNode *node -} - // Increments priority of the given child and reorders if necessary func (n *node) incrementChildPrio(pos int) int { cs := n.children @@ -405,7 +400,23 @@ type nodeValue struct { // made if a handle exists with an extra (without the) trailing slash for the // given path. func (n *node) getValue(path string, params *Params, unescape bool) (value nodeValue) { - var skipped *skip + // path: /abc/123/def + // level 1 router:abc + // level 2 router:123 + // level 3 router:def + var ( + skippedPath string + latestNode = n // not found `level 2 router` use latestNode + + // match '/' count + // matchNum < 1: `level 2 router` not found,the current node needs to be equal to latestNode + // matchNum >= 1: `level (2 or 3 or 4 or ...) router`: Normal handling + matchNum int // each match will accumulate + ) + // if path = '/', no need to look for router + if len(path) == 1 { + matchNum = 1 + } walk: // Outer loop for walking the tree for { @@ -418,32 +429,41 @@ walk: // Outer loop for walking the tree idxc := path[0] for i, c := range []byte(n.indices) { if c == idxc { - if strings.HasPrefix(n.children[len(n.children)-1].path, ":") { - skipped = &skip{ - path: prefix + path, - paramNode: &node{ - path: n.path, - wildChild: n.wildChild, - nType: n.nType, - priority: n.priority, - children: n.children, - handlers: n.handlers, - fullPath: n.fullPath, - }, + // strings.HasPrefix(n.children[len(n.children)-1].path, ":") == n.wildChild + if n.wildChild { + skippedPath = prefix + path + latestNode = &node{ + path: n.path, + wildChild: n.wildChild, + nType: n.nType, + priority: n.priority, + children: n.children, + handlers: n.handlers, + fullPath: n.fullPath, } } n = n.children[i] + + // match '/', If this condition is matched, the next route is found + if len(n.fullPath) != 0 && n.wildChild { + matchNum++ + } continue walk } } + // level 2 router not found,the current node needs to be equal to latestNode + if matchNum < 1 { + n = latestNode + } + // If there is no wildcard pattern, recommend a redirection if !n.wildChild { // Nothing found. // We can recommend to redirect to the same URL without a // trailing slash if a leaf exists for that path. - value.tsr = (path == "/" && n.handlers != nil) + value.tsr = path == "/" && n.handlers != nil return } @@ -483,6 +503,16 @@ walk: // Outer loop for walking the tree if len(n.children) > 0 { path = path[end:] n = n.children[0] + // next node,the latestNode needs to be equal to currentNode and handle next router + latestNode = n + // not found router in (level 1 router and handle next node),skipped cannot execute + // example: + // * /:cc/cc + // call /a/cc expectations:match/200 Actual:match/200 + // call /a/dd expectations:unmatch/404 Actual: panic + // call /addr/dd/aa expectations:unmatch/404 Actual: panic + // skippedPath: It can only be executed if the secondary route is not found + skippedPath = "" continue walk } @@ -533,8 +563,12 @@ walk: // Outer loop for walking the tree } } } - + // path = n.path if path == prefix { + // level 2 router not found and latestNode.wildChild is ture + if matchNum < 1 && latestNode.wildChild { + n = latestNode.children[len(latestNode.children)-1] + } // We should have reached the node containing the handle. // Check if this node has a handle registered. if value.handlers = n.handlers; value.handlers != nil { @@ -564,18 +598,16 @@ walk: // Outer loop for walking the tree return } - if path != "/" && skipped != nil && strings.HasSuffix(skipped.path, path) { - path = skipped.path - n = skipped.paramNode - skipped = nil + if path != "/" && strings.HasSuffix(skippedPath, path) { + path = skippedPath + n = latestNode + skippedPath = "" continue walk } // Nothing found. We can recommend to redirect to the same URL with an // extra trailing slash if a leaf exists for that path - value.tsr = (path == "/") || - (len(prefix) == len(path)+1 && prefix[len(path)] == '/' && - path == prefix[:len(prefix)-1] && n.handlers != nil) + value.tsr = true return } } diff --git a/tree_test.go b/tree_test.go index 7459317fa3..ad8f214663 100644 --- a/tree_test.go +++ b/tree_test.go @@ -154,6 +154,12 @@ func TestTreeWildcard(t *testing.T) { "/info/:user/public", "/info/:user/project/:project", "/info/:user/project/golang", + "/aa/*xx", + "/ab/*xx", + "/:cc", + "/:cc/cc", + "/get/test/abc/", + "/get/:param/abc/", } for _, route := range routes { tree.addRoute(route, fakeHandler(route)) @@ -186,6 +192,22 @@ func TestTreeWildcard(t *testing.T) { {"/info/gordon/public", false, "/info/:user/public", Params{Param{Key: "user", Value: "gordon"}}}, {"/info/gordon/project/go", false, "/info/:user/project/:project", Params{Param{Key: "user", Value: "gordon"}, Param{Key: "project", Value: "go"}}}, {"/info/gordon/project/golang", false, "/info/:user/project/golang", Params{Param{Key: "user", Value: "gordon"}}}, + {"/aa/aa", false, "/aa/*xx", Params{Param{Key: "xx", Value: "/aa"}}}, + {"/ab/ab", false, "/ab/*xx", Params{Param{Key: "xx", Value: "/ab"}}}, + {"/a", false, "/:cc", Params{Param{Key: "cc", Value: "a"}}}, + // * level 1 router match param will be Intercept first + // new PR handle (/all /all/cc /a/cc) + {"/all", false, "/:cc", Params{Param{Key: "cc", Value: "ll"}}}, + {"/all/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "ll"}}}, + {"/a/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: ""}}}, + {"/get/test/abc/", false, "/get/test/abc/", nil}, + {"/get/te/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "te"}}}, + {"/get/xx/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "xx"}}}, + {"/get/tt/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "tt"}}}, + {"/get/a/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "a"}}}, + {"/get/t/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "t"}}}, + {"/get/aa/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "aa"}}}, + {"/get/abas/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "abas"}}}, }) checkPriorities(t, tree) @@ -565,8 +587,8 @@ func TestTreeFindCaseInsensitivePath(t *testing.T) { "/u/öpfêl", "/v/Äpfêl/", "/v/Öpfêl", - "/w/♬", // 3 byte - "/w/♭/", // 3 byte, last byte differs + "/w/♬", // 3 byte + "/w/♭/", // 3 byte, last byte differs "/w/𠜎", // 4 byte "/w/𠜏/", // 4 byte longPath, From 0c0a50ebabf119b0221acbcc846be87cfa128f05 Mon Sep 17 00:00:00 2001 From: qm012 Date: Fri, 2 Jul 2021 11:26:31 +0800 Subject: [PATCH 02/12] update match rules --- tree.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tree.go b/tree.go index 6eacf6da52..cf08adc0d6 100644 --- a/tree.go +++ b/tree.go @@ -405,18 +405,14 @@ func (n *node) getValue(path string, params *Params, unescape bool) (value nodeV // level 2 router:123 // level 3 router:def var ( - skippedPath string + skippedPath = path latestNode = n // not found `level 2 router` use latestNode // match '/' count - // matchNum < 1: `level 2 router` not found,the current node needs to be equal to latestNode + // matchNum < 1 && skippedPath != '/': `level 2 router` not found,the current node needs to be equal to latestNode // matchNum >= 1: `level (2 or 3 or 4 or ...) router`: Normal handling matchNum int // each match will accumulate ) - // if path = '/', no need to look for router - if len(path) == 1 { - matchNum = 1 - } walk: // Outer loop for walking the tree for { @@ -563,10 +559,10 @@ walk: // Outer loop for walking the tree } } } - // path = n.path + if path == prefix { // level 2 router not found and latestNode.wildChild is ture - if matchNum < 1 && latestNode.wildChild { + if skippedPath != "/" && matchNum < 1 && latestNode.wildChild { n = latestNode.children[len(latestNode.children)-1] } // We should have reached the node containing the handle. From 211a63445181f085e95200f28bae8476211ade26 Mon Sep 17 00:00:00 2001 From: qm012 Date: Fri, 2 Jul 2021 14:14:46 +0800 Subject: [PATCH 03/12] revert and update rules --- tree.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tree.go b/tree.go index cf08adc0d6..42c843d598 100644 --- a/tree.go +++ b/tree.go @@ -405,7 +405,7 @@ func (n *node) getValue(path string, params *Params, unescape bool) (value nodeV // level 2 router:123 // level 3 router:def var ( - skippedPath = path + skippedPath string latestNode = n // not found `level 2 router` use latestNode // match '/' count @@ -413,6 +413,10 @@ func (n *node) getValue(path string, params *Params, unescape bool) (value nodeV // matchNum >= 1: `level (2 or 3 or 4 or ...) router`: Normal handling matchNum int // each match will accumulate ) + //if path == "/", no need to look for router + if len(path) == 1 { + matchNum = 1 + } walk: // Outer loop for walking the tree for { @@ -562,7 +566,7 @@ walk: // Outer loop for walking the tree if path == prefix { // level 2 router not found and latestNode.wildChild is ture - if skippedPath != "/" && matchNum < 1 && latestNode.wildChild { + if matchNum < 1 && latestNode.wildChild { n = latestNode.children[len(latestNode.children)-1] } // We should have reached the node containing the handle. @@ -594,7 +598,7 @@ walk: // Outer loop for walking the tree return } - if path != "/" && strings.HasSuffix(skippedPath, path) { + if path != "/" { path = skippedPath n = latestNode skippedPath = "" From 9ec1d256177b18204b2e30b2d58d122601144720 Mon Sep 17 00:00:00 2001 From: qm012 Date: Sat, 3 Jul 2021 20:21:43 +0800 Subject: [PATCH 04/12] reset .gitignore --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 2ac6ea8cce..bdd50c95cf 100644 --- a/.gitignore +++ b/.gitignore @@ -4,4 +4,4 @@ coverage.out count.out test profile.out -tmp.out \ No newline at end of file +tmp.out From dfbc051df93c4aee8ebc1acf63d8349aeefbed04 Mon Sep 17 00:00:00 2001 From: qm012 Date: Sun, 4 Jul 2021 15:12:24 +0800 Subject: [PATCH 05/12] add more test and fix multiple params error --- gin_integration_test.go | 72 +++++++++++++++++++++++++++++------------ tree.go | 6 ++-- tree_test.go | 18 +++++++++-- 3 files changed, 71 insertions(+), 25 deletions(-) diff --git a/gin_integration_test.go b/gin_integration_test.go index b8ba6418e9..9bcc19d4e4 100644 --- a/gin_integration_test.go +++ b/gin_integration_test.go @@ -23,7 +23,8 @@ import ( ) // params[0]=url example:http://127.0.0.1:8080/index (cannot be empty) -// params[1]=response body (custom compare content) +// params[1]=response status (custom compare status) default:"200 OK" +// params[2]=response body (custom compare content) default:"it worked" func testRequest(t *testing.T, params ...string) { if len(params) == 0 { @@ -44,12 +45,20 @@ func testRequest(t *testing.T, params ...string) { body, ioerr := ioutil.ReadAll(resp.Body) assert.NoError(t, ioerr) - var expected = "it worked" - if len(params) > 1 { - expected = params[1] + var responseStatus = "200 OK" + if len(params) > 1 && params[1] != "" { + responseStatus = params[1] + } + + var responseBody = "it worked" + if len(params) > 2 && params[2] != "" { + responseBody = params[2] + } + + assert.Equal(t, responseStatus, resp.Status, "should get a "+responseStatus) + if responseStatus == "200 OK" { + assert.Equal(t, responseBody, string(body), "resp body should match") } - assert.Equal(t, expected, string(body), "resp body should match") - assert.Equal(t, "200 OK", resp.Status, "should get a 200") } func TestRunEmpty(t *testing.T) { @@ -393,25 +402,46 @@ func TestRunDynamicRouting(t *testing.T) { router.GET("/", func(c *Context) { c.String(http.StatusOK, "home") }) router.GET("/:cc", func(c *Context) { c.String(http.StatusOK, "/:cc") }) router.GET("/:cc/cc", func(c *Context) { c.String(http.StatusOK, "/:cc/cc") }) + + router.GET("/:cc/:dd/ee", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/ee") }) + router.GET("/:cc/:dd/:ee/ff", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/ff") }) + router.GET("/:cc/:dd/:ee/:ff/gg", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/:ff/gg") }) + router.GET("/:cc/:dd/:ee/:ff/:gg/hh", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/:ff/:gg/hh") }) + router.GET("/get/test/abc/", func(c *Context) { c.String(http.StatusOK, "/get/test/abc/") }) router.GET("/get/:param/abc/", func(c *Context) { c.String(http.StatusOK, "/get/:param/abc/") }) + router.GET("/something/:paramname/thirdthing", func(c *Context) { c.String(http.StatusOK, "/something/:paramname/thirdthing") }) + router.GET("/something/secondthing/test", func(c *Context) { c.String(http.StatusOK, "/something/secondthing/test") }) ts := httptest.NewServer(router) defer ts.Close() - testRequest(t, ts.URL+"/", "home") - testRequest(t, ts.URL+"/aa/aa", "/aa/*xx") - testRequest(t, ts.URL+"/ab/ab", "/ab/*xx") - testRequest(t, ts.URL+"/all", "/:cc") - testRequest(t, ts.URL+"/all/cc", "/:cc/cc") - testRequest(t, ts.URL+"/a/cc", "/:cc/cc") - testRequest(t, ts.URL+"/a", "/:cc") - testRequest(t, ts.URL+"/get/test/abc/", "/get/test/abc/") - testRequest(t, ts.URL+"/get/te/abc/", "/get/:param/abc/") - testRequest(t, ts.URL+"/get/xx/abc/", "/get/:param/abc/") - testRequest(t, ts.URL+"/get/tt/abc/", "/get/:param/abc/") - testRequest(t, ts.URL+"/get/a/abc/", "/get/:param/abc/") - testRequest(t, ts.URL+"/get/t/abc/", "/get/:param/abc/") - testRequest(t, ts.URL+"/get/aa/abc/", "/get/:param/abc/") - testRequest(t, ts.URL+"/get/abas/abc/", "/get/:param/abc/") + testRequest(t, ts.URL+"/", "", "home") + testRequest(t, ts.URL+"/aa/aa", "", "/aa/*xx") + testRequest(t, ts.URL+"/ab/ab", "", "/ab/*xx") + testRequest(t, ts.URL+"/all", "", "/:cc") + testRequest(t, ts.URL+"/all/cc", "", "/:cc/cc") + testRequest(t, ts.URL+"/a/cc", "", "/:cc/cc") + testRequest(t, ts.URL+"/c/d/ee", "", "/:cc/:dd/ee") + testRequest(t, ts.URL+"/c/d/e/ff", "", "/:cc/:dd/:ee/ff") + testRequest(t, ts.URL+"/c/d/e/f/gg", "", "/:cc/:dd/:ee/:ff/gg") + testRequest(t, ts.URL+"/c/d/e/f/g/hh", "", "/:cc/:dd/:ee/:ff/:gg/hh") + testRequest(t, ts.URL+"/a", "", "/:cc") + testRequest(t, ts.URL+"/get/test/abc/", "", "/get/test/abc/") + testRequest(t, ts.URL+"/get/te/abc/", "", "/get/:param/abc/") + testRequest(t, ts.URL+"/get/xx/abc/", "", "/get/:param/abc/") + testRequest(t, ts.URL+"/get/tt/abc/", "", "/get/:param/abc/") + testRequest(t, ts.URL+"/get/a/abc/", "", "/get/:param/abc/") + testRequest(t, ts.URL+"/get/t/abc/", "", "/get/:param/abc/") + testRequest(t, ts.URL+"/get/aa/abc/", "", "/get/:param/abc/") + testRequest(t, ts.URL+"/get/abas/abc/", "", "/get/:param/abc/") + testRequest(t, ts.URL+"/something/secondthing/test", "", "/something/secondthing/test") + testRequest(t, ts.URL+"/something/abcdad/thirdthing", "", "/something/:paramname/thirdthing") + testRequest(t, ts.URL+"/something/se/thirdthing", "", "/something/:paramname/thirdthing") + testRequest(t, ts.URL+"/something/s/thirdthing", "", "/something/:paramname/thirdthing") + testRequest(t, ts.URL+"/something/secondthing/thirdthing", "", "/something/:paramname/thirdthing") + // 404 not found + testRequest(t, ts.URL+"/a/dd", "404 Not Found") + testRequest(t, ts.URL+"/addr/dd/aa", "404 Not Found") + testRequest(t, ts.URL+"/something/secondthing/121", "404 Not Found") } diff --git a/tree.go b/tree.go index 42c843d598..ad232aef0a 100644 --- a/tree.go +++ b/tree.go @@ -512,7 +512,9 @@ walk: // Outer loop for walking the tree // call /a/dd expectations:unmatch/404 Actual: panic // call /addr/dd/aa expectations:unmatch/404 Actual: panic // skippedPath: It can only be executed if the secondary route is not found + // matchNum: not match,need add matchNum skippedPath = "" + matchNum++ continue walk } @@ -598,7 +600,7 @@ walk: // Outer loop for walking the tree return } - if path != "/" { + if path != "/" && skippedPath != "" { path = skippedPath n = latestNode skippedPath = "" @@ -607,7 +609,7 @@ walk: // Outer loop for walking the tree // Nothing found. We can recommend to redirect to the same URL with an // extra trailing slash if a leaf exists for that path - value.tsr = true + // tree.go line:569 handle leaf nodes return } } diff --git a/tree_test.go b/tree_test.go index ad8f214663..91213eee06 100644 --- a/tree_test.go +++ b/tree_test.go @@ -158,8 +158,14 @@ func TestTreeWildcard(t *testing.T) { "/ab/*xx", "/:cc", "/:cc/cc", + "/:cc/:dd/ee", + "/:cc/:dd/:ee/ff", + "/:cc/:dd/:ee/:ff/gg", + "/:cc/:dd/:ee/:ff/:gg/hh", "/get/test/abc/", "/get/:param/abc/", + "/something/:paramname/thirdthing", + "/something/secondthing/test", } for _, route := range routes { tree.addRoute(route, fakeHandler(route)) @@ -208,6 +214,14 @@ func TestTreeWildcard(t *testing.T) { {"/get/t/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "t"}}}, {"/get/aa/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "aa"}}}, {"/get/abas/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "abas"}}}, + {"/something/secondthing/test", false, "/something/secondthing/test", nil}, + {"/something/abcdad/thirdthing", false, "/something/:paramname/thirdthing", Params{Param{Key: "paramname", Value: "abcdad"}}}, + {"/something/se/thirdthing", false, "/something/:paramname/thirdthing", Params{Param{Key: "paramname", Value: "se"}}}, + {"/something/s/thirdthing", false, "/something/:paramname/thirdthing", Params{Param{Key: "paramname", Value: "s"}}}, + {"/c/d/ee", false, "/:cc/:dd/ee", Params{Param{Key: "cc", Value: "c"}, Param{Key: "dd", Value: "d"}}}, + {"/c/d/e/ff", false, "/:cc/:dd/:ee/ff", Params{Param{Key: "cc", Value: "c"}, Param{Key: "dd", Value: "d"}, Param{Key: "ee", Value: "e"}}}, + {"/c/d/e/f/gg", false, "/:cc/:dd/:ee/:ff/gg", Params{Param{Key: "cc", Value: "c"}, Param{Key: "dd", Value: "d"}, Param{Key: "ee", Value: "e"}, Param{Key: "ff", Value: "f"}}}, + {"/c/d/e/f/g/hh", false, "/:cc/:dd/:ee/:ff/:gg/hh", Params{Param{Key: "cc", Value: "c"}, Param{Key: "dd", Value: "d"}, Param{Key: "ee", Value: "e"}, Param{Key: "ff", Value: "f"}, Param{Key: "gg", Value: "g"}}}, }) checkPriorities(t, tree) @@ -587,8 +601,8 @@ func TestTreeFindCaseInsensitivePath(t *testing.T) { "/u/öpfêl", "/v/Äpfêl/", "/v/Öpfêl", - "/w/♬", // 3 byte - "/w/♭/", // 3 byte, last byte differs + "/w/♬", // 3 byte + "/w/♭/", // 3 byte, last byte differs "/w/𠜎", // 4 byte "/w/𠜏/", // 4 byte longPath, From 5dbe044ca454c4351b8cd6ec08335e792f08f0c8 Mon Sep 17 00:00:00 2001 From: qm012 Date: Mon, 5 Jul 2021 10:20:09 +0800 Subject: [PATCH 06/12] update comment --- tree.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tree.go b/tree.go index ad232aef0a..b6cc2db476 100644 --- a/tree.go +++ b/tree.go @@ -409,11 +409,11 @@ func (n *node) getValue(path string, params *Params, unescape bool) (value nodeV latestNode = n // not found `level 2 router` use latestNode // match '/' count - // matchNum < 1 && skippedPath != '/': `level 2 router` not found,the current node needs to be equal to latestNode + // matchNum < 1: `level 2 router` not found,the current node needs to be equal to latestNode // matchNum >= 1: `level (2 or 3 or 4 or ...) router`: Normal handling matchNum int // each match will accumulate ) - //if path == "/", no need to look for router + //if path == "/", no need to look for tree node if len(path) == 1 { matchNum = 1 } @@ -512,7 +512,7 @@ walk: // Outer loop for walking the tree // call /a/dd expectations:unmatch/404 Actual: panic // call /addr/dd/aa expectations:unmatch/404 Actual: panic // skippedPath: It can only be executed if the secondary route is not found - // matchNum: not match,need add matchNum + // matchNum: Go to the next level of routing tree node search,need add matchNum skippedPath = "" matchNum++ continue walk From 23f623829a9f60d0b8a74249bd84370b4390a108 Mon Sep 17 00:00:00 2001 From: qm012 Date: Mon, 5 Jul 2021 11:17:37 +0800 Subject: [PATCH 07/12] fix checks error --- tree.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tree.go b/tree.go index b6cc2db476..94875e77f3 100644 --- a/tree.go +++ b/tree.go @@ -446,7 +446,7 @@ walk: // Outer loop for walking the tree n = n.children[i] // match '/', If this condition is matched, the next route is found - if len(n.fullPath) != 0 && n.wildChild { + if (len(n.fullPath) != 0 && n.wildChild) || strings.HasSuffix(path, "/") { matchNum++ } continue walk @@ -609,7 +609,9 @@ walk: // Outer loop for walking the tree // Nothing found. We can recommend to redirect to the same URL with an // extra trailing slash if a leaf exists for that path - // tree.go line:569 handle leaf nodes + value.tsr = (path == "/") || + (len(prefix) == len(path)+1 && prefix[len(path)] == '/' && + path == prefix[:len(prefix)-1] && n.handlers != nil) return } } From ec091bf4d88a9e88aa1aaeedb6756dfa80009d11 Mon Sep 17 00:00:00 2001 From: qm012 Date: Mon, 5 Jul 2021 15:19:36 +0800 Subject: [PATCH 08/12] update math condition --- gin_integration_test.go | 2 -- tree.go | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/gin_integration_test.go b/gin_integration_test.go index 9bcc19d4e4..4f44757d42 100644 --- a/gin_integration_test.go +++ b/gin_integration_test.go @@ -402,12 +402,10 @@ func TestRunDynamicRouting(t *testing.T) { router.GET("/", func(c *Context) { c.String(http.StatusOK, "home") }) router.GET("/:cc", func(c *Context) { c.String(http.StatusOK, "/:cc") }) router.GET("/:cc/cc", func(c *Context) { c.String(http.StatusOK, "/:cc/cc") }) - router.GET("/:cc/:dd/ee", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/ee") }) router.GET("/:cc/:dd/:ee/ff", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/ff") }) router.GET("/:cc/:dd/:ee/:ff/gg", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/:ff/gg") }) router.GET("/:cc/:dd/:ee/:ff/:gg/hh", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/:ff/:gg/hh") }) - router.GET("/get/test/abc/", func(c *Context) { c.String(http.StatusOK, "/get/test/abc/") }) router.GET("/get/:param/abc/", func(c *Context) { c.String(http.StatusOK, "/get/:param/abc/") }) router.GET("/something/:paramname/thirdthing", func(c *Context) { c.String(http.StatusOK, "/something/:paramname/thirdthing") }) diff --git a/tree.go b/tree.go index 94875e77f3..de2d9cd028 100644 --- a/tree.go +++ b/tree.go @@ -446,7 +446,7 @@ walk: // Outer loop for walking the tree n = n.children[i] // match '/', If this condition is matched, the next route is found - if (len(n.fullPath) != 0 && n.wildChild) || strings.HasSuffix(path, "/") { + if (len(n.fullPath) != 0 && n.wildChild) || path[len(path)-1] == '/' { matchNum++ } continue walk From 82f3c92dede5533ff0a62ed71e3c645e0a3dc47d Mon Sep 17 00:00:00 2001 From: qm012 Date: Tue, 6 Jul 2021 08:29:47 +0800 Subject: [PATCH 09/12] update description --- tree.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tree.go b/tree.go index de2d9cd028..b30ef62d59 100644 --- a/tree.go +++ b/tree.go @@ -505,7 +505,7 @@ walk: // Outer loop for walking the tree n = n.children[0] // next node,the latestNode needs to be equal to currentNode and handle next router latestNode = n - // not found router in (level 1 router and handle next node),skipped cannot execute + // not found router in (level 1 router and handle next node),skippedPath cannot execute // example: // * /:cc/cc // call /a/cc expectations:match/200 Actual:match/200 @@ -567,7 +567,7 @@ walk: // Outer loop for walking the tree } if path == prefix { - // level 2 router not found and latestNode.wildChild is ture + // level 2 router not found and latestNode.wildChild is true if matchNum < 1 && latestNode.wildChild { n = latestNode.children[len(latestNode.children)-1] } From 085b582bea9f68bc626facb192839d1dbacb743f Mon Sep 17 00:00:00 2001 From: qm012 Date: Tue, 6 Jul 2021 15:13:50 +0800 Subject: [PATCH 10/12] update `value.tsr` match rule --- gin_integration_test.go | 2 +- tree.go | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/gin_integration_test.go b/gin_integration_test.go index 4f44757d42..ed7196fd6e 100644 --- a/gin_integration_test.go +++ b/gin_integration_test.go @@ -395,7 +395,7 @@ func testGetRequestHandler(t *testing.T, h http.Handler, url string) { assert.Equal(t, 200, w.Code, "should get a 200") } -func TestRunDynamicRouting(t *testing.T) { +func TestTreeRunDynamicRouting(t *testing.T) { router := New() router.GET("/aa/*xx", func(c *Context) { c.String(http.StatusOK, "/aa/*xx") }) router.GET("/ab/*xx", func(c *Context) { c.String(http.StatusOK, "/ab/*xx") }) diff --git a/tree.go b/tree.go index b30ef62d59..615deb3f1d 100644 --- a/tree.go +++ b/tree.go @@ -609,9 +609,8 @@ walk: // Outer loop for walking the tree // Nothing found. We can recommend to redirect to the same URL with an // extra trailing slash if a leaf exists for that path - value.tsr = (path == "/") || - (len(prefix) == len(path)+1 && prefix[len(path)] == '/' && - path == prefix[:len(prefix)-1] && n.handlers != nil) + value.tsr = path == "/" || + (path == prefix[:len(prefix)-1] && n.handlers != nil) return } } From a3ee27128769c91c7bca44d729dedd0d96949353 Mon Sep 17 00:00:00 2001 From: qm012 Date: Tue, 6 Jul 2021 16:21:44 +0800 Subject: [PATCH 11/12] update `value.tsr` match rule --- tree.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree.go b/tree.go index 615deb3f1d..63549b1584 100644 --- a/tree.go +++ b/tree.go @@ -610,7 +610,7 @@ walk: // Outer loop for walking the tree // Nothing found. We can recommend to redirect to the same URL with an // extra trailing slash if a leaf exists for that path value.tsr = path == "/" || - (path == prefix[:len(prefix)-1] && n.handlers != nil) + (len(prefix) == len(path)+1 && n.handlers != nil) return } } From 2d657d45d2f96132daac61beae6a9b1709ad5a9c Mon Sep 17 00:00:00 2001 From: qm012 Date: Thu, 8 Jul 2021 10:09:04 +0800 Subject: [PATCH 12/12] update if condition --- tree.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tree.go b/tree.go index 63549b1584..2e46b8e513 100644 --- a/tree.go +++ b/tree.go @@ -600,7 +600,8 @@ walk: // Outer loop for walking the tree return } - if path != "/" && skippedPath != "" { + // path != "/" && skippedPath != "" + if len(path) != 1 && len(skippedPath) > 0 && strings.HasSuffix(skippedPath, path) { path = skippedPath n = latestNode skippedPath = ""