httpserver: Base path of "/" matches all paths, even empty ones

Fixes #1645

3 files changed, 73 insertions, 6 deletions

diff --git a/caddyhttp/httpserver/path.go b/caddyhttp/httpserver/path.go
index 6af5039d9..92136395d 100644
--- a/caddyhttp/httpserver/path.go
+++ b/caddyhttp/httpserver/path.go
@@ -5,19 +5,25 @@ import (
 	"strings"
 )
 
-// Path represents a URI path.
+// Path represents a URI path. It should usually be
+// set to the value of a request path.
 type Path string
 
-// Matches checks to see if other matches p.
+// Matches checks to see if base matches p. The correct
+// usage of this method sets p as the request path, and
+// base as a Caddyfile (user-defined) rule path.
 //
 // Path matching will probably not always be a direct
 // comparison; this method assures that paths can be
 // easily and consistently matched.
-func (p Path) Matches(other string) bool {
+func (p Path) Matches(base string) bool {
+	if base == "/" {
+		return true
+	}
 	if CaseSensitivePath {
-		return strings.HasPrefix(string(p), other)
+		return strings.HasPrefix(string(p), base)
 	}
-	return strings.HasPrefix(strings.ToLower(string(p)), strings.ToLower(other))
+	return strings.HasPrefix(strings.ToLower(string(p)), strings.ToLower(base))
 }
 
 // PathMatcher is a Path RequestMatcher.
diff --git a/caddyhttp/httpserver/path_test.go b/caddyhttp/httpserver/path_test.go
new file mode 100644
index 000000000..4e46cbe6b
--- /dev/null
+++ b/caddyhttp/httpserver/path_test.go
@@ -0,0 +1,61 @@
+package httpserver
+
+import "testing"
+
+func TestPathMatches(t *testing.T) {
+	for i, testcase := range []struct {
+		reqPath         Path
+		rulePath        string
+		shouldMatch     bool
+		caseInsensitive bool
+	}{
+		{
+			reqPath:     "/",
+			rulePath:    "/",
+			shouldMatch: true,
+		},
+		{
+			reqPath:     "/foo/bar",
+			rulePath:    "/foo",
+			shouldMatch: true,
+		},
+		{
+			reqPath:     "/foobar",
+			rulePath:    "/foo/",
+			shouldMatch: false,
+		},
+		{
+			reqPath:     "/foobar",
+			rulePath:    "/foo/bar",
+			shouldMatch: false,
+		},
+		{
+			reqPath:     "/Foobar",
+			rulePath:    "/Foo",
+			shouldMatch: true,
+		},
+		{
+
+			reqPath:     "/FooBar",
+			rulePath:    "/Foo",
+			shouldMatch: true,
+		},
+		{
+			reqPath:         "/foobar",
+			rulePath:        "/FooBar",
+			shouldMatch:     true,
+			caseInsensitive: true,
+		},
+		{
+			reqPath:     "",
+			rulePath:    "/", // a lone forward slash means to match all requests (see issue #1645)
+			shouldMatch: true,
+		},
+	} {
+		CaseSensitivePath = !testcase.caseInsensitive
+		if got, want := testcase.reqPath.Matches(testcase.rulePath), testcase.shouldMatch; got != want {
+			t.Errorf("Test %d: For request path '%s' and other path '%s': expected %v, got %v",
+				i, testcase.reqPath, testcase.rulePath, want, got)
+		}
+	}
+}
diff --git a/caddyhttp/httpserver/tplcontext_test.go b/caddyhttp/httpserver/tplcontext_test.go
index 01ca55bcb..c912f7151 100644
--- a/caddyhttp/httpserver/tplcontext_test.go
+++ b/caddyhttp/httpserver/tplcontext_test.go
@@ -497,7 +497,7 @@ func TestMethod(t *testing.T) {
 
 }
 
-func TestPathMatches(t *testing.T) {
+func TestContextPathMatches(t *testing.T) {
 	context := getContextOrFail(t)
 
 	tests := []struct {