net/http: fix bugs in comparePaths and combineRelationships

combineRelationships was wrong on one case: if one part of a pattern
overlaps and the other is disjoint, the result is disjoint, not overlaps.
For example:

    /a/{x}/c
    /{x}/b/d

Here the prefix consisting of the first two segments overlaps, but the
third segments are disjoint. The patterns as a whole are disjoint.

comparePaths was wrong in a couple of ways:

First, the loop shouldn't exit early when it sees an overlap,
for the reason above: later information may change that.

Once the loop was allowed to continue, we had to handle the "overlaps"
case at the end. The insight there, which generalized the existing
code, is that if the shorter path ends in a multi, that multi matches
the remainder of the longer path and more. (It must be "and more": the
longer path has at least two segments, so it couldn't match one
segment while the shorter path's multi can.) That means we can treat
the result as the combination moreGeneral and the relationship of the
common prefix.

Change-Id: I11dab2c020d820730fb38296d9d6b072bd2a5350
Reviewed-on: https://go-review.googlesource.com/c/go/+/529119
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/net/http/pattern.go b/src/net/http/pattern.go
index 3fd20b711e0a7b8524ce77ea2a530300eb566d5c..6b9e535bee7a99638b328cd66d6a09dfae26a38f 100644 (file)
--- a/src/net/http/pattern.go
+++ b/src/net/http/pattern.go
@@ -273,12 +273,13 @@ func (p1 *pattern) comparePaths(p2 *pattern) relationship {
        if len(p1.segments) != len(p2.segments) && !p1.lastSegment().multi && !p2.lastSegment().multi {
                return disjoint
        }
+
+       // Consider corresponding segments in the two path patterns.
        var segs1, segs2 []segment
-       // Look at corresponding segments in the two path patterns.
        rel := equivalent
        for segs1, segs2 = p1.segments, p2.segments; len(segs1) > 0 && len(segs2) > 0; segs1, segs2 = segs1[1:], segs2[1:] {
                rel = combineRelationships(rel, compareSegments(segs1[0], segs2[0]))
-               if rel == disjoint || rel == overlaps {
+               if rel == disjoint {
                        return rel
                }
        }
@@ -289,12 +290,13 @@ func (p1 *pattern) comparePaths(p2 *pattern) relationship {
                return rel
        }
        // Otherwise, the only way they could fail to be disjoint is if the shorter
-       // pattern ends in a multi and is more general.
-       if len(segs1) < len(segs2) && p1.lastSegment().multi && rel == moreGeneral {
-               return moreGeneral
+       // pattern ends in a multi. In that case, that multi is more general
+       // than the remainder of the longer pattern, so combine those two relationships.
+       if len(segs1) < len(segs2) && p1.lastSegment().multi {
+               return combineRelationships(rel, moreGeneral)
        }
-       if len(segs2) < len(segs1) && p2.lastSegment().multi && rel == moreSpecific {
-               return moreSpecific
+       if len(segs2) < len(segs1) && p2.lastSegment().multi {
+               return combineRelationships(rel, moreSpecific)
        }
        return disjoint
 }
@@ -345,8 +347,13 @@ func combineRelationships(r1, r2 relationship) relationship {
        switch r1 {
        case equivalent:
                return r2
-       case disjoint, overlaps:
-               return r1
+       case disjoint:
+               return disjoint
+       case overlaps:
+               if r2 == disjoint {
+                       return disjoint
+               }
+               return overlaps
        case moreGeneral, moreSpecific:
                switch r2 {
                case equivalent:
@@ -373,3 +380,11 @@ func inverseRelationship(r relationship) relationship {
                return r
        }
 }
+
+// isLitOrSingle reports whether the segment is a non-dollar literal or a single wildcard.
+func isLitOrSingle(seg segment) bool {
+       if seg.wild {
+               return !seg.multi
+       }
+       return seg.s != "/"
+}

diff --git a/src/net/http/pattern_test.go b/src/net/http/pattern_test.go
index cd27cd8db8a3b21f3efd8a0ab0e0a72454d9e264..7c5189790715160f6ee39f8012cbc37064f53547 100644 (file)
--- a/src/net/http/pattern_test.go
+++ b/src/net/http/pattern_test.go
@@ -296,6 +296,7 @@ func TestComparePaths(t *testing.T) {
                {"/a/{z}/{m...}", "/{z}/a/", overlaps},
                {"/a/{z}/{m...}", "/{z}/b/{y...}", overlaps},
                {"/a/{z}/b/{m...}", "/{x}/c/{y...}", overlaps},
+               {"/a/{z}/a/{m...}", "/{x}/b", disjoint},
 
                // Dollar on left.
                {"/{$}", "/a", disjoint},
@@ -314,6 +315,8 @@ func TestComparePaths(t *testing.T) {
                {"/b/{$}", "/b/{x...}", moreSpecific},
                {"/b/{$}", "/b/c/{x...}", disjoint},
                {"/b/{x}/a/{$}", "/{x}/c/{y...}", overlaps},
+               {"/{x}/b/{$}", "/a/{x}/{y}", disjoint},
+               {"/{x}/b/{$}", "/a/{x}/c", disjoint},
 
                {"/{z}/{$}", "/{z}/a", disjoint},
                {"/{z}/{$}", "/{z}/a/b", disjoint},