Skip to content

Commit 9d016f6

Browse files
qm012appleboy
qm012
authored and
appleboy
committed
fix #2786 (#2796)
* update match rule * add comments
1 parent 9bc4d8c commit 9d016f6

File tree

3 files changed

+197
-43
lines changed

3 files changed

+197
-43
lines changed

‎gin_integration_test.go

+82
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,25 @@ func TestTreeRunDynamicRouting(t *testing.T) {
349349
router.GET("/get/:param/abc/", func(c *Context) { c.String(http.StatusOK, "/get/:param/abc/") })
350350
router.GET("/something/:paramname/thirdthing", func(c *Context) { c.String(http.StatusOK, "/something/:paramname/thirdthing") })
351351
router.GET("/something/secondthing/test", func(c *Context) { c.String(http.StatusOK, "/something/secondthing/test") })
352+
router.GET("/get/abc", func(c *Context) { c.String(http.StatusOK, "/get/abc") })
353+
router.GET("/get/:param", func(c *Context) { c.String(http.StatusOK, "/get/:param") })
354+
router.GET("/get/abc/123abc", func(c *Context) { c.String(http.StatusOK, "/get/abc/123abc") })
355+
router.GET("/get/abc/:param", func(c *Context) { c.String(http.StatusOK, "/get/abc/:param") })
356+
router.GET("/get/abc/123abc/xxx8", func(c *Context) { c.String(http.StatusOK, "/get/abc/123abc/xxx8") })
357+
router.GET("/get/abc/123abc/:param", func(c *Context) { c.String(http.StatusOK, "/get/abc/123abc/:param") })
358+
router.GET("/get/abc/123abc/xxx8/1234", func(c *Context) { c.String(http.StatusOK, "/get/abc/123abc/xxx8/1234") })
359+
router.GET("/get/abc/123abc/xxx8/:param", func(c *Context) { c.String(http.StatusOK, "/get/abc/123abc/xxx8/:param") })
360+
router.GET("/get/abc/123abc/xxx8/1234/ffas", func(c *Context) { c.String(http.StatusOK, "/get/abc/123abc/xxx8/1234/ffas") })
361+
router.GET("/get/abc/123abc/xxx8/1234/:param", func(c *Context) { c.String(http.StatusOK, "/get/abc/123abc/xxx8/1234/:param") })
362+
router.GET("/get/abc/123abc/xxx8/1234/kkdd/12c", func(c *Context) { c.String(http.StatusOK, "/get/abc/123abc/xxx8/1234/kkdd/12c") })
363+
router.GET("/get/abc/123abc/xxx8/1234/kkdd/:param", func(c *Context) { c.String(http.StatusOK, "/get/abc/123abc/xxx8/1234/kkdd/:param") })
364+
router.GET("/get/abc/:param/test", func(c *Context) { c.String(http.StatusOK, "/get/abc/:param/test") })
365+
router.GET("/get/abc/123abd/:param", func(c *Context) { c.String(http.StatusOK, "/get/abc/123abd/:param") })
366+
router.GET("/get/abc/123abddd/:param", func(c *Context) { c.String(http.StatusOK, "/get/abc/123abddd/:param") })
367+
router.GET("/get/abc/123/:param", func(c *Context) { c.String(http.StatusOK, "/get/abc/123/:param") })
368+
router.GET("/get/abc/123abg/:param", func(c *Context) { c.String(http.StatusOK, "/get/abc/123abg/:param") })
369+
router.GET("/get/abc/123abf/:param", func(c *Context) { c.String(http.StatusOK, "/get/abc/123abf/:param") })
370+
router.GET("/get/abc/123abfff/:param", func(c *Context) { c.String(http.StatusOK, "/get/abc/123abfff/:param") })
352371

353372
ts := httptest.NewServer(router)
354373
defer ts.Close()
@@ -363,8 +382,26 @@ func TestTreeRunDynamicRouting(t *testing.T) {
363382
testRequest(t, ts.URL+"/c/d/e/ff", "", "/:cc/:dd/:ee/ff")
364383
testRequest(t, ts.URL+"/c/d/e/f/gg", "", "/:cc/:dd/:ee/:ff/gg")
365384
testRequest(t, ts.URL+"/c/d/e/f/g/hh", "", "/:cc/:dd/:ee/:ff/:gg/hh")
385+
testRequest(t, ts.URL+"/cc/dd/ee/ff/gg/hh", "", "/:cc/:dd/:ee/:ff/:gg/hh")
366386
testRequest(t, ts.URL+"/a", "", "/:cc")
387+
testRequest(t, ts.URL+"/d", "", "/:cc")
388+
testRequest(t, ts.URL+"/ad", "", "/:cc")
389+
testRequest(t, ts.URL+"/dd", "", "/:cc")
390+
testRequest(t, ts.URL+"/aa", "", "/:cc")
391+
testRequest(t, ts.URL+"/aaa", "", "/:cc")
392+
testRequest(t, ts.URL+"/aaa/cc", "", "/:cc/cc")
393+
testRequest(t, ts.URL+"/ab", "", "/:cc")
394+
testRequest(t, ts.URL+"/abb", "", "/:cc")
395+
testRequest(t, ts.URL+"/abb/cc", "", "/:cc/cc")
396+
testRequest(t, ts.URL+"/dddaa", "", "/:cc")
397+
testRequest(t, ts.URL+"/allxxxx", "", "/:cc")
398+
testRequest(t, ts.URL+"/alldd", "", "/:cc")
399+
testRequest(t, ts.URL+"/cc/cc", "", "/:cc/cc")
400+
testRequest(t, ts.URL+"/ccc/cc", "", "/:cc/cc")
401+
testRequest(t, ts.URL+"/deedwjfs/cc", "", "/:cc/cc")
402+
testRequest(t, ts.URL+"/acllcc/cc", "", "/:cc/cc")
367403
testRequest(t, ts.URL+"/get/test/abc/", "", "/get/test/abc/")
404+
testRequest(t, ts.URL+"/get/testaa/abc/", "", "/get/:param/abc/")
368405
testRequest(t, ts.URL+"/get/te/abc/", "", "/get/:param/abc/")
369406
testRequest(t, ts.URL+"/get/xx/abc/", "", "/get/:param/abc/")
370407
testRequest(t, ts.URL+"/get/tt/abc/", "", "/get/:param/abc/")
@@ -373,10 +410,55 @@ func TestTreeRunDynamicRouting(t *testing.T) {
373410
testRequest(t, ts.URL+"/get/aa/abc/", "", "/get/:param/abc/")
374411
testRequest(t, ts.URL+"/get/abas/abc/", "", "/get/:param/abc/")
375412
testRequest(t, ts.URL+"/something/secondthing/test", "", "/something/secondthing/test")
413+
testRequest(t, ts.URL+"/something/secondthingaaaa/thirdthing", "", "/something/:paramname/thirdthing")
376414
testRequest(t, ts.URL+"/something/abcdad/thirdthing", "", "/something/:paramname/thirdthing")
377415
testRequest(t, ts.URL+"/something/se/thirdthing", "", "/something/:paramname/thirdthing")
378416
testRequest(t, ts.URL+"/something/s/thirdthing", "", "/something/:paramname/thirdthing")
379417
testRequest(t, ts.URL+"/something/secondthing/thirdthing", "", "/something/:paramname/thirdthing")
418+
testRequest(t, ts.URL+"/get/abc", "", "/get/abc")
419+
testRequest(t, ts.URL+"/get/a", "", "/get/:param")
420+
testRequest(t, ts.URL+"/get/abz", "", "/get/:param")
421+
testRequest(t, ts.URL+"/get/12a", "", "/get/:param")
422+
testRequest(t, ts.URL+"/get/abcd", "", "/get/:param")
423+
testRequest(t, ts.URL+"/get/abc/123abc", "", "/get/abc/123abc")
424+
testRequest(t, ts.URL+"/get/abc/12", "", "/get/abc/:param")
425+
testRequest(t, ts.URL+"/get/abc/123ab", "", "/get/abc/:param")
426+
testRequest(t, ts.URL+"/get/abc/xyz", "", "/get/abc/:param")
427+
testRequest(t, ts.URL+"/get/abc/123abcddxx", "", "/get/abc/:param")
428+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8", "", "/get/abc/123abc/xxx8")
429+
testRequest(t, ts.URL+"/get/abc/123abc/x", "", "/get/abc/123abc/:param")
430+
testRequest(t, ts.URL+"/get/abc/123abc/xxx", "", "/get/abc/123abc/:param")
431+
testRequest(t, ts.URL+"/get/abc/123abc/abc", "", "/get/abc/123abc/:param")
432+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8xxas", "", "/get/abc/123abc/:param")
433+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8/1234", "", "/get/abc/123abc/xxx8/1234")
434+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8/1", "", "/get/abc/123abc/xxx8/:param")
435+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8/123", "", "/get/abc/123abc/xxx8/:param")
436+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8/78k", "", "/get/abc/123abc/xxx8/:param")
437+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8/1234xxxd", "", "/get/abc/123abc/xxx8/:param")
438+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8/1234/ffas", "", "/get/abc/123abc/xxx8/1234/ffas")
439+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8/1234/f", "", "/get/abc/123abc/xxx8/1234/:param")
440+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8/1234/ffa", "", "/get/abc/123abc/xxx8/1234/:param")
441+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8/1234/kka", "", "/get/abc/123abc/xxx8/1234/:param")
442+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8/1234/ffas321", "", "/get/abc/123abc/xxx8/1234/:param")
443+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8/1234/kkdd/12c", "", "/get/abc/123abc/xxx8/1234/kkdd/12c")
444+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8/1234/kkdd/1", "", "/get/abc/123abc/xxx8/1234/kkdd/:param")
445+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8/1234/kkdd/12", "", "/get/abc/123abc/xxx8/1234/kkdd/:param")
446+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8/1234/kkdd/12b", "", "/get/abc/123abc/xxx8/1234/kkdd/:param")
447+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8/1234/kkdd/34", "", "/get/abc/123abc/xxx8/1234/kkdd/:param")
448+
testRequest(t, ts.URL+"/get/abc/123abc/xxx8/1234/kkdd/12c2e3", "", "/get/abc/123abc/xxx8/1234/kkdd/:param")
449+
testRequest(t, ts.URL+"/get/abc/12/test", "", "/get/abc/:param/test")
450+
testRequest(t, ts.URL+"/get/abc/123abdd/test", "", "/get/abc/:param/test")
451+
testRequest(t, ts.URL+"/get/abc/123abdddf/test", "", "/get/abc/:param/test")
452+
testRequest(t, ts.URL+"/get/abc/123ab/test", "", "/get/abc/:param/test")
453+
testRequest(t, ts.URL+"/get/abc/123abgg/test", "", "/get/abc/:param/test")
454+
testRequest(t, ts.URL+"/get/abc/123abff/test", "", "/get/abc/:param/test")
455+
testRequest(t, ts.URL+"/get/abc/123abffff/test", "", "/get/abc/:param/test")
456+
testRequest(t, ts.URL+"/get/abc/123abd/test", "", "/get/abc/123abd/:param")
457+
testRequest(t, ts.URL+"/get/abc/123abddd/test", "", "/get/abc/123abddd/:param")
458+
testRequest(t, ts.URL+"/get/abc/123/test22", "", "/get/abc/123/:param")
459+
testRequest(t, ts.URL+"/get/abc/123abg/test", "", "/get/abc/123abg/:param")
460+
testRequest(t, ts.URL+"/get/abc/123abf/testss", "", "/get/abc/123abf/:param")
461+
testRequest(t, ts.URL+"/get/abc/123abfff/te", "", "/get/abc/123abfff/:param")
380462
// 404 not found
381463
testRequest(t, ts.URL+"/a/dd", "404 Not Found")
382464
testRequest(t, ts.URL+"/addr/dd/aa", "404 Not Found")

‎tree.go

+28-39
Original file line numberDiff line numberDiff line change
@@ -400,23 +400,10 @@ type nodeValue struct {
400400
// made if a handle exists with an extra (without the) trailing slash for the
401401
// given path.
402402
func (n *node) getValue(path string, params *Params, unescape bool) (value nodeValue) {
403-
// path: /abc/123/def
404-
// level 1 router:abc
405-
// level 2 router:123
406-
// level 3 router:def
407403
var (
408404
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
405+
latestNode = n // Caching the latest node
415406
)
416-
//if path == "/", no need to look for tree node
417-
if len(path) == 1 {
418-
matchNum = 1
419-
}
420407

421408
walk: // Outer loop for walking the tree
422409
for {
@@ -444,17 +431,13 @@ walk: // Outer loop for walking the tree
444431
}
445432

446433
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-
}
452434
continue walk
453435
}
454436
}
455-
456-
// level 2 router not found,the current node needs to be equal to latestNode
457-
if matchNum < 1 {
437+
// If the path at the end of the loop is not equal to '/' and the current node has no child nodes
438+
// the current node needs to be equal to the latest matching node
439+
matched := path != "/" && !n.wildChild
440+
if matched {
458441
n = latestNode
459442
}
460443

@@ -472,6 +455,16 @@ walk: // Outer loop for walking the tree
472455

473456
switch n.nType {
474457
case param:
458+
// fix truncate the parameter
459+
// tree_test.go line: 204
460+
if matched {
461+
path = prefix + path
462+
// The saved path is used after the prefix route is intercepted by matching
463+
if n.indices == "/" {
464+
path = skippedPath[1:]
465+
}
466+
}
467+
475468
// Find param end (either '/' or path end)
476469
end := 0
477470
for end < len(path) && path[end] != '/' {
@@ -503,18 +496,6 @@ walk: // Outer loop for walking the tree
503496
if len(n.children) > 0 {
504497
path = path[end:]
505498
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++
518499
continue walk
519500
}
520501

@@ -567,8 +548,9 @@ walk: // Outer loop for walking the tree
567548
}
568549

569550
if path == prefix {
570-
// level 2 router not found and latestNode.wildChild is true
571-
if matchNum < 1 && latestNode.wildChild {
551+
// If the current path does not equal '/' and the node does not have a registered handle and the most recently matched node has a child node
552+
// the current node needs to be equal to the latest matching node
553+
if latestNode.wildChild && n.handlers == nil && path != "/" {
572554
n = latestNode.children[len(latestNode.children)-1]
573555
}
574556
// We should have reached the node containing the handle.
@@ -600,10 +582,17 @@ walk: // Outer loop for walking the tree
600582
return
601583
}
602584

603-
// path != "/" && skippedPath != ""
604-
if len(path) != 1 && len(skippedPath) > 0 && strings.HasSuffix(skippedPath, path) {
585+
if path != "/" && len(skippedPath) > 0 && strings.HasSuffix(skippedPath, path) {
605586
path = skippedPath
606-
n = latestNode
587+
// Reduce the number of cycles
588+
n, latestNode = latestNode, n
589+
// skippedPath cannot execute
590+
// example:
591+
// * /:cc/cc
592+
// call /a/cc expectations:match/200 Actual:match/200
593+
// call /a/dd expectations:unmatch/404 Actual: panic
594+
// call /addr/dd/aa expectations:unmatch/404 Actual: panic
595+
// skippedPath: It can only be executed if the secondary route is not found
607596
skippedPath = ""
608597
continue walk
609598
}

0 commit comments

Comments
 (0)