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>

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.
+	// pattern ends in a multi. In that case, that multi is more general
-	if len(segs1) < len(segs2) && p1.lastSegment().multi && rel == moreGeneral {
+	// than the remainder of the longer pattern, so combine those two relationships.
-		return moreGeneral
+	if len(segs1) < len(segs2) && p1.lastSegment().multi {
+		return combineRelationships(rel, moreGeneral)
 	}
-	if len(segs2) < len(segs1) && p2.lastSegment().multi && rel == moreSpecific {
+	if len(segs2) < len(segs1) && p2.lastSegment().multi {
-		return moreSpecific
+		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:
+	case disjoint:
-		return r1
+		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 != "/"
+}

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},