From a210eea3bd1c3766d76968108dfcd83c331f549c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=94=B0=E6=AC=A7?= Date: Fri, 21 Sep 2018 10:21:59 +0800 Subject: [PATCH] improve panic information when a catch-all wildcard conflict occurs (#1529) --- tree.go | 11 +++++++++-- tree_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/tree.go b/tree.go index b6530665..ada62ceb 100644 --- a/tree.go +++ b/tree.go @@ -193,9 +193,16 @@ func (n *node) addRoute(path string, handlers HandlersChain) { } } - panic("path segment '" + path + + pathSeg := path + if n.nType != catchAll { + pathSeg = strings.SplitN(path, "/", 2)[0] + } + prefix := fullPath[:strings.Index(fullPath, pathSeg)] + n.path + panic("'" + pathSeg + + "' in new path '" + fullPath + "' conflicts with existing wildcard '" + n.path + - "' in path '" + fullPath + "'") + "' in existing prefix '" + prefix + + "'") } c := path[0] diff --git a/tree_test.go b/tree_test.go index 152f6331..a1b3bbe7 100644 --- a/tree_test.go +++ b/tree_test.go @@ -5,7 +5,9 @@ package gin import ( + "fmt" "reflect" + "regexp" "strings" "testing" ) @@ -653,3 +655,43 @@ func TestTreeInvalidNodeType(t *testing.T) { t.Fatalf("Expected panic '"+panicMsg+"', got '%v'", recv) } } + +func TestTreeWildcardConflictEx(t *testing.T) { + conflicts := [...]struct { + route string + segPath string + existPath string + existSegPath string + }{ + {"/who/are/foo", "/foo", `/who/are/\*you`, `/\*you`}, + {"/who/are/foo/", "/foo/", `/who/are/\*you`, `/\*you`}, + {"/who/are/foo/bar", "/foo/bar", `/who/are/\*you`, `/\*you`}, + {"/conxxx", "xxx", `/con:tact`, `:tact`}, + {"/conooo/xxx", "ooo", `/con:tact`, `:tact`}, + } + + for _, conflict := range conflicts { + // I have to re-create a 'tree', because the 'tree' will be + // in an inconsistent state when the loop recovers from the + // panic which threw by 'addRoute' function. + tree := &node{} + routes := [...]string{ + "/con:tact", + "/who/are/*you", + "/who/foo/hello", + } + + for _, route := range routes { + tree.addRoute(route, fakeHandler(route)) + } + + recv := catchPanic(func() { + tree.addRoute(conflict.route, fakeHandler(conflict.route)) + }) + + if !regexp.MustCompile(fmt.Sprintf("'%s' in new path .* conflicts with existing wildcard '%s' in existing prefix '%s'", + conflict.segPath, conflict.existSegPath, conflict.existPath)).MatchString(fmt.Sprint(recv)) { + t.Fatalf("invalid wildcard conflict error (%v)", recv) + } + } +}