From 7b4335da89ae064a75eccc3d5e6d05a223e54ff5 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Sun, 23 Apr 2017 21:20:20 -0400
Subject: [PATCH] Match subpaths correctly when path contains trailing slash

---
 pkg/router/template/router.go      |  19 +++-
 pkg/router/template/router_test.go | 140 +++++++++++++++++++++++++++++
 2 files changed, 158 insertions(+), 1 deletion(-)

diff --git a/pkg/router/template/router.go b/pkg/router/template/router.go
index ad10c89b144f..8f4e8172047f 100644
--- a/pkg/router/template/router.go
+++ b/pkg/router/template/router.go
@@ -250,7 +250,24 @@ func generateRouteRegexp(hostname, path string, wildcard bool) string {
 		}
 	}
 
-	return fmt.Sprintf("^%s(|:[0-9]+)%s(|/.*)$", hostRE, regexp.QuoteMeta(path))
+	portRE := "(:[0-9]+)?"
+
+	// build the correct subpath regex, depending on whether path ends with a segment separator
+	var pathRE, subpathRE string
+	switch {
+	case strings.TrimRight(path, "/") == "":
+		// Special-case paths consisting solely of "/" to match a root request to "" as well
+		pathRE = ""
+		subpathRE = "(/.*)?"
+	case strings.HasSuffix(path, "/"):
+		pathRE = regexp.QuoteMeta(path)
+		subpathRE = "(.*)?"
+	default:
+		pathRE = regexp.QuoteMeta(path)
+		subpathRE = "(/.*)?"
+	}
+
+	return "^" + hostRE + portRE + pathRE + subpathRE + "$"
 }
 
 // Generates the host name to use for serving/certificate matching.
diff --git a/pkg/router/template/router_test.go b/pkg/router/template/router_test.go
index de0ee5d30ba8..9b4cf1c9f35f 100644
--- a/pkg/router/template/router_test.go
+++ b/pkg/router/template/router_test.go
@@ -4,6 +4,7 @@ import (
 	"crypto/md5"
 	"fmt"
 	"reflect"
+	"regexp"
 	"testing"
 
 	kapi "k8s.io/kubernetes/pkg/api"
@@ -670,3 +671,142 @@ func TestAddRouteEdgeTerminationInsecurePolicy(t *testing.T) {
 		}
 	}
 }
+
+func TestGenerateRouteRegexp(t *testing.T) {
+	tests := []struct {
+		name     string
+		hostname string
+		path     string
+		wildcard bool
+
+		match   []string
+		nomatch []string
+	}{
+		{
+			name:     "no path",
+			hostname: "example.com",
+			path:     "",
+			wildcard: false,
+			match: []string{
+				"example.com",
+				"example.com:80",
+				"example.com/",
+				"example.com/sub",
+				"example.com/sub/",
+			},
+			nomatch: []string{"other.com"},
+		},
+		{
+			name:     "root path with trailing slash",
+			hostname: "example.com",
+			path:     "/",
+			wildcard: false,
+			match: []string{
+				"example.com",
+				"example.com:80",
+				"example.com/",
+				"example.com/sub",
+				"example.com/sub/",
+			},
+			nomatch: []string{"other.com"},
+		},
+		{
+			name:     "subpath with trailing slash",
+			hostname: "example.com",
+			path:     "/sub/",
+			wildcard: false,
+			match: []string{
+				"example.com/sub/",
+				"example.com/sub/subsub",
+			},
+			nomatch: []string{
+				"other.com",
+				"example.com",
+				"example.com:80",
+				"example.com/",
+				"example.com/sub",    // path with trailing slash doesn't match URL without
+				"example.com/subpar", // path segment boundary match required
+			},
+		},
+		{
+			name:     "subpath without trailing slash",
+			hostname: "example.com",
+			path:     "/sub",
+			wildcard: false,
+			match: []string{
+				"example.com/sub",
+				"example.com/sub/",
+				"example.com/sub/subsub",
+			},
+			nomatch: []string{
+				"other.com",
+				"example.com",
+				"example.com:80",
+				"example.com/",
+				"example.com/subpar", // path segment boundary match required
+			},
+		},
+		{
+			name:     "wildcard",
+			hostname: "www.example.com",
+			path:     "/",
+			wildcard: true,
+			match: []string{
+				"www.example.com",
+				"www.example.com/",
+				"www.example.com/sub",
+				"www.example.com/sub/",
+				"www.example.com:80",
+				"www.example.com:80/",
+				"www.example.com:80/sub",
+				"www.example.com:80/sub/",
+				"foo.example.com",
+				"foo.example.com/",
+				"foo.example.com/sub",
+				"foo.example.com/sub/",
+			},
+			nomatch: []string{
+				"wwwexample.com",
+				"foo.bar.example.com",
+			},
+		},
+		{
+			name:     "non-wildcard",
+			hostname: "www.example.com",
+			path:     "/",
+			wildcard: false,
+			match: []string{
+				"www.example.com",
+				"www.example.com/",
+				"www.example.com/sub",
+				"www.example.com/sub/",
+				"www.example.com:80",
+				"www.example.com:80/",
+				"www.example.com:80/sub",
+				"www.example.com:80/sub/",
+			},
+			nomatch: []string{
+				"foo.example.com",
+				"foo.example.com/",
+				"foo.example.com/sub",
+				"foo.example.com/sub/",
+				"wwwexample.com",
+				"foo.bar.example.com",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		r := regexp.MustCompile(generateRouteRegexp(tt.hostname, tt.path, tt.wildcard))
+		for _, s := range tt.match {
+			if !r.Match([]byte(s)) {
+				t.Errorf("%s: expected %s to match %s, but didn't", tt.name, r, s)
+			}
+		}
+		for _, s := range tt.nomatch {
+			if r.Match([]byte(s)) {
+				t.Errorf("%s: expected %s not to match %s, but did", tt.name, r, s)
+			}
+		}
+	}
+}