Skip to content

Commit 9bc4d8c

Browse files
qm012appleboy
authored andcommitted
fix #2762 (#2767)
1 parent 3f5c051 commit 9bc4d8c

File tree

3 files changed

+172
-30
lines changed

3 files changed

+172
-30
lines changed

gin_integration_test.go

Lines changed: 74 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,22 +22,43 @@ import (
2222
"github.com/stretchr/testify/assert"
2323
)
2424

25-
func testRequest(t *testing.T, url string) {
25+
// params[0]=url example:http://127.0.0.1:8080/index (cannot be empty)
26+
// params[1]=response status (custom compare status) default:"200 OK"
27+
// params[2]=response body (custom compare content) default:"it worked"
28+
func testRequest(t *testing.T, params ...string) {
29+
30+
if len(params) == 0 {
31+
t.Fatal("url cannot be empty")
32+
}
33+
2634
tr := &http.Transport{
2735
TLSClientConfig: &tls.Config{
2836
InsecureSkipVerify: true,
2937
},
3038
}
3139
client := &http.Client{Transport: tr}
3240

33-
resp, err := client.Get(url)
41+
resp, err := client.Get(params[0])
3442
assert.NoError(t, err)
3543
defer resp.Body.Close()
3644

3745
body, ioerr := ioutil.ReadAll(resp.Body)
3846
assert.NoError(t, ioerr)
39-
assert.Equal(t, "it worked", string(body), "resp body should match")
40-
assert.Equal(t, "200 OK", resp.Status, "should get a 200")
47+
48+
var responseStatus = "200 OK"
49+
if len(params) > 1 && params[1] != "" {
50+
responseStatus = params[1]
51+
}
52+
53+
var responseBody = "it worked"
54+
if len(params) > 2 && params[2] != "" {
55+
responseBody = params[2]
56+
}
57+
58+
assert.Equal(t, responseStatus, resp.Status, "should get a "+responseStatus)
59+
if responseStatus == "200 OK" {
60+
assert.Equal(t, responseBody, string(body), "resp body should match")
61+
}
4162
}
4263

4364
func TestRunEmpty(t *testing.T) {
@@ -312,3 +333,52 @@ func testGetRequestHandler(t *testing.T, h http.Handler, url string) {
312333
assert.Equal(t, "it worked", w.Body.String(), "resp body should match")
313334
assert.Equal(t, 200, w.Code, "should get a 200")
314335
}
336+
337+
func TestTreeRunDynamicRouting(t *testing.T) {
338+
router := New()
339+
router.GET("/aa/*xx", func(c *Context) { c.String(http.StatusOK, "/aa/*xx") })
340+
router.GET("/ab/*xx", func(c *Context) { c.String(http.StatusOK, "/ab/*xx") })
341+
router.GET("/", func(c *Context) { c.String(http.StatusOK, "home") })
342+
router.GET("/:cc", func(c *Context) { c.String(http.StatusOK, "/:cc") })
343+
router.GET("/:cc/cc", func(c *Context) { c.String(http.StatusOK, "/:cc/cc") })
344+
router.GET("/:cc/:dd/ee", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/ee") })
345+
router.GET("/:cc/:dd/:ee/ff", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/ff") })
346+
router.GET("/:cc/:dd/:ee/:ff/gg", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/:ff/gg") })
347+
router.GET("/:cc/:dd/:ee/:ff/:gg/hh", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/:ff/:gg/hh") })
348+
router.GET("/get/test/abc/", func(c *Context) { c.String(http.StatusOK, "/get/test/abc/") })
349+
router.GET("/get/:param/abc/", func(c *Context) { c.String(http.StatusOK, "/get/:param/abc/") })
350+
router.GET("/something/:paramname/thirdthing", func(c *Context) { c.String(http.StatusOK, "/something/:paramname/thirdthing") })
351+
router.GET("/something/secondthing/test", func(c *Context) { c.String(http.StatusOK, "/something/secondthing/test") })
352+
353+
ts := httptest.NewServer(router)
354+
defer ts.Close()
355+
356+
testRequest(t, ts.URL+"/", "", "home")
357+
testRequest(t, ts.URL+"/aa/aa", "", "/aa/*xx")
358+
testRequest(t, ts.URL+"/ab/ab", "", "/ab/*xx")
359+
testRequest(t, ts.URL+"/all", "", "/:cc")
360+
testRequest(t, ts.URL+"/all/cc", "", "/:cc/cc")
361+
testRequest(t, ts.URL+"/a/cc", "", "/:cc/cc")
362+
testRequest(t, ts.URL+"/c/d/ee", "", "/:cc/:dd/ee")
363+
testRequest(t, ts.URL+"/c/d/e/ff", "", "/:cc/:dd/:ee/ff")
364+
testRequest(t, ts.URL+"/c/d/e/f/gg", "", "/:cc/:dd/:ee/:ff/gg")
365+
testRequest(t, ts.URL+"/c/d/e/f/g/hh", "", "/:cc/:dd/:ee/:ff/:gg/hh")
366+
testRequest(t, ts.URL+"/a", "", "/:cc")
367+
testRequest(t, ts.URL+"/get/test/abc/", "", "/get/test/abc/")
368+
testRequest(t, ts.URL+"/get/te/abc/", "", "/get/:param/abc/")
369+
testRequest(t, ts.URL+"/get/xx/abc/", "", "/get/:param/abc/")
370+
testRequest(t, ts.URL+"/get/tt/abc/", "", "/get/:param/abc/")
371+
testRequest(t, ts.URL+"/get/a/abc/", "", "/get/:param/abc/")
372+
testRequest(t, ts.URL+"/get/t/abc/", "", "/get/:param/abc/")
373+
testRequest(t, ts.URL+"/get/aa/abc/", "", "/get/:param/abc/")
374+
testRequest(t, ts.URL+"/get/abas/abc/", "", "/get/:param/abc/")
375+
testRequest(t, ts.URL+"/something/secondthing/test", "", "/something/secondthing/test")
376+
testRequest(t, ts.URL+"/something/abcdad/thirdthing", "", "/something/:paramname/thirdthing")
377+
testRequest(t, ts.URL+"/something/se/thirdthing", "", "/something/:paramname/thirdthing")
378+
testRequest(t, ts.URL+"/something/s/thirdthing", "", "/something/:paramname/thirdthing")
379+
testRequest(t, ts.URL+"/something/secondthing/thirdthing", "", "/something/:paramname/thirdthing")
380+
// 404 not found
381+
testRequest(t, ts.URL+"/a/dd", "404 Not Found")
382+
testRequest(t, ts.URL+"/addr/dd/aa", "404 Not Found")
383+
testRequest(t, ts.URL+"/something/secondthing/121", "404 Not Found")
384+
}

tree.go

Lines changed: 62 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,6 @@ type node struct {
118118
fullPath string
119119
}
120120

121-
type skip struct {
122-
path string
123-
paramNode *node
124-
}
125-
126121
// Increments priority of the given child and reorders if necessary
127122
func (n *node) incrementChildPrio(pos int) int {
128123
cs := n.children
@@ -405,7 +400,23 @@ type nodeValue struct {
405400
// made if a handle exists with an extra (without the) trailing slash for the
406401
// given path.
407402
func (n *node) getValue(path string, params *Params, unescape bool) (value nodeValue) {
408-
var skipped *skip
403+
// path: /abc/123/def
404+
// level 1 router:abc
405+
// level 2 router:123
406+
// level 3 router:def
407+
var (
408+
skippedPath string
409+
latestNode = n // not found `level 2 router` use latestNode
410+
411+
// match '/' count
412+
// matchNum < 1: `level 2 router` not found,the current node needs to be equal to latestNode
413+
// matchNum >= 1: `level (2 or 3 or 4 or ...) router`: Normal handling
414+
matchNum int // each match will accumulate
415+
)
416+
//if path == "/", no need to look for tree node
417+
if len(path) == 1 {
418+
matchNum = 1
419+
}
409420

410421
walk: // Outer loop for walking the tree
411422
for {
@@ -418,32 +429,41 @@ walk: // Outer loop for walking the tree
418429
idxc := path[0]
419430
for i, c := range []byte(n.indices) {
420431
if c == idxc {
421-
if strings.HasPrefix(n.children[len(n.children)-1].path, ":") {
422-
skipped = &skip{
423-
path: prefix + path,
424-
paramNode: &node{
425-
path: n.path,
426-
wildChild: n.wildChild,
427-
nType: n.nType,
428-
priority: n.priority,
429-
children: n.children,
430-
handlers: n.handlers,
431-
fullPath: n.fullPath,
432-
},
432+
// strings.HasPrefix(n.children[len(n.children)-1].path, ":") == n.wildChild
433+
if n.wildChild {
434+
skippedPath = prefix + path
435+
latestNode = &node{
436+
path: n.path,
437+
wildChild: n.wildChild,
438+
nType: n.nType,
439+
priority: n.priority,
440+
children: n.children,
441+
handlers: n.handlers,
442+
fullPath: n.fullPath,
433443
}
434444
}
435445

436446
n = n.children[i]
447+
448+
// match '/', If this condition is matched, the next route is found
449+
if (len(n.fullPath) != 0 && n.wildChild) || path[len(path)-1] == '/' {
450+
matchNum++
451+
}
437452
continue walk
438453
}
439454
}
440455

456+
// level 2 router not found,the current node needs to be equal to latestNode
457+
if matchNum < 1 {
458+
n = latestNode
459+
}
460+
441461
// If there is no wildcard pattern, recommend a redirection
442462
if !n.wildChild {
443463
// Nothing found.
444464
// We can recommend to redirect to the same URL without a
445465
// trailing slash if a leaf exists for that path.
446-
value.tsr = (path == "/" && n.handlers != nil)
466+
value.tsr = path == "/" && n.handlers != nil
447467
return
448468
}
449469

@@ -483,6 +503,18 @@ walk: // Outer loop for walking the tree
483503
if len(n.children) > 0 {
484504
path = path[end:]
485505
n = n.children[0]
506+
// next node,the latestNode needs to be equal to currentNode and handle next router
507+
latestNode = n
508+
// not found router in (level 1 router and handle next node),skippedPath cannot execute
509+
// example:
510+
// * /:cc/cc
511+
// call /a/cc expectations:match/200 Actual:match/200
512+
// call /a/dd expectations:unmatch/404 Actual: panic
513+
// call /addr/dd/aa expectations:unmatch/404 Actual: panic
514+
// skippedPath: It can only be executed if the secondary route is not found
515+
// matchNum: Go to the next level of routing tree node search,need add matchNum
516+
skippedPath = ""
517+
matchNum++
486518
continue walk
487519
}
488520

@@ -535,6 +567,10 @@ walk: // Outer loop for walking the tree
535567
}
536568

537569
if path == prefix {
570+
// level 2 router not found and latestNode.wildChild is true
571+
if matchNum < 1 && latestNode.wildChild {
572+
n = latestNode.children[len(latestNode.children)-1]
573+
}
538574
// We should have reached the node containing the handle.
539575
// Check if this node has a handle registered.
540576
if value.handlers = n.handlers; value.handlers != nil {
@@ -564,18 +600,18 @@ walk: // Outer loop for walking the tree
564600
return
565601
}
566602

567-
if path != "/" && skipped != nil && strings.HasSuffix(skipped.path, path) {
568-
path = skipped.path
569-
n = skipped.paramNode
570-
skipped = nil
603+
// path != "/" && skippedPath != ""
604+
if len(path) != 1 && len(skippedPath) > 0 && strings.HasSuffix(skippedPath, path) {
605+
path = skippedPath
606+
n = latestNode
607+
skippedPath = ""
571608
continue walk
572609
}
573610

574611
// Nothing found. We can recommend to redirect to the same URL with an
575612
// extra trailing slash if a leaf exists for that path
576-
value.tsr = (path == "/") ||
577-
(len(prefix) == len(path)+1 && prefix[len(path)] == '/' &&
578-
path == prefix[:len(prefix)-1] && n.handlers != nil)
613+
value.tsr = path == "/" ||
614+
(len(prefix) == len(path)+1 && n.handlers != nil)
579615
return
580616
}
581617
}

tree_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,18 @@ func TestTreeWildcard(t *testing.T) {
154154
"/info/:user/public",
155155
"/info/:user/project/:project",
156156
"/info/:user/project/golang",
157+
"/aa/*xx",
158+
"/ab/*xx",
159+
"/:cc",
160+
"/:cc/cc",
161+
"/:cc/:dd/ee",
162+
"/:cc/:dd/:ee/ff",
163+
"/:cc/:dd/:ee/:ff/gg",
164+
"/:cc/:dd/:ee/:ff/:gg/hh",
165+
"/get/test/abc/",
166+
"/get/:param/abc/",
167+
"/something/:paramname/thirdthing",
168+
"/something/secondthing/test",
157169
}
158170
for _, route := range routes {
159171
tree.addRoute(route, fakeHandler(route))
@@ -186,6 +198,30 @@ func TestTreeWildcard(t *testing.T) {
186198
{"/info/gordon/public", false, "/info/:user/public", Params{Param{Key: "user", Value: "gordon"}}},
187199
{"/info/gordon/project/go", false, "/info/:user/project/:project", Params{Param{Key: "user", Value: "gordon"}, Param{Key: "project", Value: "go"}}},
188200
{"/info/gordon/project/golang", false, "/info/:user/project/golang", Params{Param{Key: "user", Value: "gordon"}}},
201+
{"/aa/aa", false, "/aa/*xx", Params{Param{Key: "xx", Value: "/aa"}}},
202+
{"/ab/ab", false, "/ab/*xx", Params{Param{Key: "xx", Value: "/ab"}}},
203+
{"/a", false, "/:cc", Params{Param{Key: "cc", Value: "a"}}},
204+
// * level 1 router match param will be Intercept first
205+
// new PR handle (/all /all/cc /a/cc)
206+
{"/all", false, "/:cc", Params{Param{Key: "cc", Value: "ll"}}},
207+
{"/all/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "ll"}}},
208+
{"/a/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: ""}}},
209+
{"/get/test/abc/", false, "/get/test/abc/", nil},
210+
{"/get/te/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "te"}}},
211+
{"/get/xx/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "xx"}}},
212+
{"/get/tt/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "tt"}}},
213+
{"/get/a/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "a"}}},
214+
{"/get/t/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "t"}}},
215+
{"/get/aa/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "aa"}}},
216+
{"/get/abas/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "abas"}}},
217+
{"/something/secondthing/test", false, "/something/secondthing/test", nil},
218+
{"/something/abcdad/thirdthing", false, "/something/:paramname/thirdthing", Params{Param{Key: "paramname", Value: "abcdad"}}},
219+
{"/something/se/thirdthing", false, "/something/:paramname/thirdthing", Params{Param{Key: "paramname", Value: "se"}}},
220+
{"/something/s/thirdthing", false, "/something/:paramname/thirdthing", Params{Param{Key: "paramname", Value: "s"}}},
221+
{"/c/d/ee", false, "/:cc/:dd/ee", Params{Param{Key: "cc", Value: "c"}, Param{Key: "dd", Value: "d"}}},
222+
{"/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"}}},
223+
{"/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"}}},
224+
{"/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"}}},
189225
})
190226

191227
checkPriorities(t, tree)

0 commit comments

Comments
 (0)