mirror of
https://github.com/golang/go
synced 2024-11-18 18:04:46 -07:00
net/http: explain why two patterns conflict
It can be difficult to tell at a glance why two patterns conflict, so explain it with example paths. Change-Id: Ie384f0a4ef64f30e6e6898bce4b88027bc81034b Reviewed-on: https://go-review.googlesource.com/c/go/+/529122 Run-TryBot: Jonathan Amsterdam <jba@google.com> Reviewed-by: Damien Neil <dneil@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
This commit is contained in:
Jonathan Amsterdam
parent
be11422b1e
commit
ad42fedda5
@ -388,3 +388,132 @@ func isLitOrSingle(seg segment) bool {
|
||||
}
|
||||
return seg.s != "/"
|
||||
}
|
||||
|
||||
// describeConflict returns an explanation of why two patterns conflict.
|
||||
func describeConflict(p1, p2 *pattern) string {
|
||||
mrel := p1.compareMethods(p2)
|
||||
prel := p1.comparePaths(p2)
|
||||
rel := combineRelationships(mrel, prel)
|
||||
if rel == equivalent {
|
||||
return fmt.Sprintf("%s matches the same requests as %s", p1, p2)
|
||||
}
|
||||
if rel != overlaps {
|
||||
panic("describeConflict called with non-conflicting patterns")
|
||||
}
|
||||
if prel == overlaps {
|
||||
return fmt.Sprintf(`%[1]s and %[2]s both match some paths, like %[3]q.
|
||||
But neither is more specific than the other.
|
||||
%[1]s matches %[4]q, but %[2]s doesn't.
|
||||
%[2]s matches %[5]q, but %[1]s doesn't.`,
|
||||
p1, p2, commonPath(p1, p2), differencePath(p1, p2), differencePath(p2, p1))
|
||||
}
|
||||
if mrel == moreGeneral && prel == moreSpecific {
|
||||
return fmt.Sprintf("%s matches more methods than %s, but has a more specific path pattern", p1, p2)
|
||||
}
|
||||
if mrel == moreSpecific && prel == moreGeneral {
|
||||
return fmt.Sprintf("%s matches fewer methods than %s, but has a more general path pattern", p1, p2)
|
||||
}
|
||||
return fmt.Sprintf("bug: unexpected way for two patterns %s and %s to conflict: methods %s, paths %s", p1, p2, mrel, prel)
|
||||
}
|
||||
|
||||
// writeMatchingPath writes to b a path that matches the segments.
|
||||
func writeMatchingPath(b *strings.Builder, segs []segment) {
|
||||
for _, s := range segs {
|
||||
writeSegment(b, s)
|
||||
}
|
||||
}
|
||||
|
||||
func writeSegment(b *strings.Builder, s segment) {
|
||||
b.WriteByte('/')
|
||||
if !s.multi && s.s != "/" {
|
||||
b.WriteString(s.s)
|
||||
}
|
||||
}
|
||||
|
||||
// commonPath returns a path that both p1 and p2 match.
|
||||
// It assumes there is such a path.
|
||||
func commonPath(p1, p2 *pattern) string {
|
||||
var b strings.Builder
|
||||
var segs1, segs2 []segment
|
||||
for segs1, segs2 = p1.segments, p2.segments; len(segs1) > 0 && len(segs2) > 0; segs1, segs2 = segs1[1:], segs2[1:] {
|
||||
if s1 := segs1[0]; s1.wild {
|
||||
writeSegment(&b, segs2[0])
|
||||
} else {
|
||||
writeSegment(&b, s1)
|
||||
}
|
||||
}
|
||||
if len(segs1) > 0 {
|
||||
writeMatchingPath(&b, segs1)
|
||||
} else if len(segs2) > 0 {
|
||||
writeMatchingPath(&b, segs2)
|
||||
}
|
||||
return b.String()
|
||||
}
|
||||
|
||||
// differencePath returns a path that p1 matches and p2 doesn't.
|
||||
// It assumes there is such a path.
|
||||
func differencePath(p1, p2 *pattern) string {
|
||||
var b strings.Builder
|
||||
|
||||
var segs1, segs2 []segment
|
||||
for segs1, segs2 = p1.segments, p2.segments; len(segs1) > 0 && len(segs2) > 0; segs1, segs2 = segs1[1:], segs2[1:] {
|
||||
s1 := segs1[0]
|
||||
s2 := segs2[0]
|
||||
if s1.multi && s2.multi {
|
||||
// From here the patterns match the same paths, so we must have found a difference earlier.
|
||||
b.WriteByte('/')
|
||||
return b.String()
|
||||
|
||||
}
|
||||
if s1.multi && !s2.multi {
|
||||
// s1 ends in a "..." wildcard but s2 does not.
|
||||
// A trailing slash will distinguish them, unless s2 ends in "{$}",
|
||||
// in which case any segment will do; prefer the wildcard name if
|
||||
// it has one.
|
||||
b.WriteByte('/')
|
||||
if s2.s == "/" {
|
||||
if s1.s != "" {
|
||||
b.WriteString(s1.s)
|
||||
} else {
|
||||
b.WriteString("x")
|
||||
}
|
||||
}
|
||||
return b.String()
|
||||
}
|
||||
if !s1.multi && s2.multi {
|
||||
writeSegment(&b, s1)
|
||||
} else if s1.wild && s2.wild {
|
||||
// Both patterns will match whatever we put here; use
|
||||
// the first wildcard name.
|
||||
writeSegment(&b, s1)
|
||||
} else if s1.wild && !s2.wild {
|
||||
// s1 is a wildcard, s2 is a literal.
|
||||
// Any segment other than s2.s will work.
|
||||
// Prefer the wildcard name, but if it's the same as the literal,
|
||||
// tweak the literal.
|
||||
if s1.s != s2.s {
|
||||
writeSegment(&b, s1)
|
||||
} else {
|
||||
b.WriteByte('/')
|
||||
b.WriteString(s2.s + "x")
|
||||
}
|
||||
} else if !s1.wild && s2.wild {
|
||||
writeSegment(&b, s1)
|
||||
} else {
|
||||
// Both are literals. A precondition of this function is that the
|
||||
// patterns overlap, so they must be the same literal. Use it.
|
||||
if s1.s != s2.s {
|
||||
panic(fmt.Sprintf("literals differ: %q and %q", s1.s, s2.s))
|
||||
}
|
||||
writeSegment(&b, s1)
|
||||
}
|
||||
}
|
||||
if len(segs1) > 0 {
|
||||
// p1 is longer than p2, and p2 does not end in a multi.
|
||||
// Anything that matches the rest of p1 will do.
|
||||
writeMatchingPath(&b, segs1)
|
||||
} else if len(segs2) > 0 {
|
||||
writeMatchingPath(&b, segs2)
|
||||
}
|
||||
return b.String()
|
||||
}
|
||||
|
||||
@ -392,3 +392,115 @@ func TestConflictsWith(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRegisterConflict(t *testing.T) {
|
||||
mux := NewServeMux()
|
||||
pat1 := "/a/{x}/"
|
||||
if err := mux.registerErr(pat1, NotFoundHandler()); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
pat2 := "/a/{y}/{z...}"
|
||||
err := mux.registerErr(pat2, NotFoundHandler())
|
||||
var got string
|
||||
if err == nil {
|
||||
got = "<nil>"
|
||||
} else {
|
||||
got = err.Error()
|
||||
}
|
||||
want := "matches the same requests as"
|
||||
if !strings.Contains(got, want) {
|
||||
t.Errorf("got\n%s\nwant\n%s", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDescribeConflict(t *testing.T) {
|
||||
for _, test := range []struct {
|
||||
p1, p2 string
|
||||
want string
|
||||
}{
|
||||
{"/a/{x}", "/a/{y}", "the same requests"},
|
||||
{"/", "/{m...}", "the same requests"},
|
||||
{"/a/{x}", "/{y}/b", "both match some paths"},
|
||||
{"/a", "GET /{x}", "matches more methods than GET /{x}, but has a more specific path pattern"},
|
||||
{"GET /a", "HEAD /", "matches more methods than HEAD /, but has a more specific path pattern"},
|
||||
{"POST /", "/a", "matches fewer methods than /a, but has a more general path pattern"},
|
||||
} {
|
||||
got := describeConflict(mustParsePattern(t, test.p1), mustParsePattern(t, test.p2))
|
||||
if !strings.Contains(got, test.want) {
|
||||
t.Errorf("%s vs. %s:\ngot:\n%s\nwhich does not contain %q",
|
||||
test.p1, test.p2, got, test.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestCommonPath(t *testing.T) {
|
||||
for _, test := range []struct {
|
||||
p1, p2 string
|
||||
want string
|
||||
}{
|
||||
{"/a/{x}", "/{x}/a", "/a/a"},
|
||||
{"/a/{z}/", "/{z}/a/", "/a/a/"},
|
||||
{"/a/{z}/{m...}", "/{z}/a/", "/a/a/"},
|
||||
{"/{z}/{$}", "/a/", "/a/"},
|
||||
{"/{z}/{$}", "/a/{x...}", "/a/"},
|
||||
{"/a/{z}/{$}", "/{z}/a/", "/a/a/"},
|
||||
{"/a/{x}/b/{y...}", "/{x}/c/{y...}", "/a/c/b/"},
|
||||
{"/a/{x}/b/", "/{x}/c/{y...}", "/a/c/b/"},
|
||||
{"/a/{x}/b/{$}", "/{x}/c/{y...}", "/a/c/b/"},
|
||||
{"/a/{z}/{x...}", "/{z}/b/{y...}", "/a/b/"},
|
||||
} {
|
||||
pat1 := mustParsePattern(t, test.p1)
|
||||
pat2 := mustParsePattern(t, test.p2)
|
||||
if pat1.comparePaths(pat2) != overlaps {
|
||||
t.Fatalf("%s does not overlap %s", test.p1, test.p2)
|
||||
}
|
||||
got := commonPath(pat1, pat2)
|
||||
if got != test.want {
|
||||
t.Errorf("%s vs. %s: got %q, want %q", test.p1, test.p2, got, test.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestDifferencePath(t *testing.T) {
|
||||
for _, test := range []struct {
|
||||
p1, p2 string
|
||||
want string
|
||||
}{
|
||||
{"/a/{x}", "/{x}/a", "/a/x"},
|
||||
{"/{x}/a", "/a/{x}", "/x/a"},
|
||||
{"/a/{z}/", "/{z}/a/", "/a/z/"},
|
||||
{"/{z}/a/", "/a/{z}/", "/z/a/"},
|
||||
{"/{a}/a/", "/a/{z}/", "/ax/a/"},
|
||||
{"/a/{z}/{x...}", "/{z}/b/{y...}", "/a/z/"},
|
||||
{"/{z}/b/{y...}", "/a/{z}/{x...}", "/z/b/"},
|
||||
{"/a/b/", "/a/b/c", "/a/b/"},
|
||||
{"/a/b/{x...}", "/a/b/c", "/a/b/"},
|
||||
{"/a/b/{x...}", "/a/b/c/d", "/a/b/"},
|
||||
{"/a/b/{x...}", "/a/b/c/d/", "/a/b/"},
|
||||
{"/a/{z}/{m...}", "/{z}/a/", "/a/z/"},
|
||||
{"/{z}/a/", "/a/{z}/{m...}", "/z/a/"},
|
||||
{"/{z}/{$}", "/a/", "/z/"},
|
||||
{"/a/", "/{z}/{$}", "/a/x"},
|
||||
{"/{z}/{$}", "/a/{x...}", "/z/"},
|
||||
{"/a/{foo...}", "/{z}/{$}", "/a/foo"},
|
||||
{"/a/{z}/{$}", "/{z}/a/", "/a/z/"},
|
||||
{"/{z}/a/", "/a/{z}/{$}", "/z/a/x"},
|
||||
{"/a/{x}/b/{y...}", "/{x}/c/{y...}", "/a/x/b/"},
|
||||
{"/{x}/c/{y...}", "/a/{x}/b/{y...}", "/x/c/"},
|
||||
{"/a/{c}/b/", "/{x}/c/{y...}", "/a/cx/b/"},
|
||||
{"/{x}/c/{y...}", "/a/{c}/b/", "/x/c/"},
|
||||
{"/a/{x}/b/{$}", "/{x}/c/{y...}", "/a/x/b/"},
|
||||
{"/{x}/c/{y...}", "/a/{x}/b/{$}", "/x/c/"},
|
||||
} {
|
||||
pat1 := mustParsePattern(t, test.p1)
|
||||
pat2 := mustParsePattern(t, test.p2)
|
||||
rel := pat1.comparePaths(pat2)
|
||||
if rel != overlaps && rel != moreGeneral {
|
||||
t.Fatalf("%s vs. %s are %s, need overlaps or moreGeneral", pat1, pat2, rel)
|
||||
}
|
||||
got := differencePath(pat1, pat2)
|
||||
if got != test.want {
|
||||
t.Errorf("%s vs. %s: got %q, want %q", test.p1, test.p2, got, test.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -2655,8 +2655,9 @@ func (mux *ServeMux) registerErr(patstr string, handler Handler) error {
|
||||
// Check for conflict.
|
||||
if err := mux.index.possiblyConflictingPatterns(pat, func(pat2 *pattern) error {
|
||||
if pat.conflictsWith(pat2) {
|
||||
return fmt.Errorf("pattern %q (registered at %s) conflicts with pattern %q (registered at %s)",
|
||||
pat, pat.loc, pat2, pat2.loc)
|
||||
d := describeConflict(pat, pat2)
|
||||
return fmt.Errorf("pattern %q (registered at %s) conflicts with pattern %q (registered at %s):\n%s",
|
||||
pat, pat.loc, pat2, pat2.loc, d)
|
||||
}
|
||||
return nil
|
||||
}); err != nil {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user