Merge pull request #1861 from mholt/fix1859

httpserver: Fix #1859 by cleaning paths when matching them

caddyhttp/browse/browse_test.go

@@ -227,7 +227,7 @@ func TestBrowseTemplate(t *testing.T) {
 func TestBrowseJson(t *testing.T) {
 	b := Browse{
 		Next: httpserver.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) {
-			t.Fatalf("Next shouldn't be called")
+			t.Fatalf("Next shouldn't be called: %s", r.URL)
 			return 0, nil
 		}),
 		Configs: []Config{

caddyhttp/httpserver/path.go

@@ -2,6 +2,7 @@ package httpserver
 
 import (
 	"net/http"
+	"path"
 	"strings"
 )
 
@@ -16,10 +17,27 @@ type Path string
 // Path matching will probably not always be a direct
 // comparison; this method assures that paths can be
 // easily and consistently matched.
+//
+// Multiple slashes are collapsed/merged. See issue #1859.
 func (p Path) Matches(base string) bool {
-	if base == "/" {
+	if base == "/" || base == "" {
 		return true
 	}
+
+	// sanitize the paths for comparison, very important
+	// (slightly lossy if the base path requires multiple
+	// consecutive forward slashes, since those will be merged)
+	pHasTrailingSlash := strings.HasSuffix(string(p), "/")
+	baseHasTrailingSlash := strings.HasSuffix(base, "/")
+	p = Path(path.Clean(string(p)))
+	base = path.Clean(base)
+	if pHasTrailingSlash {
+		p += "/"
+	}
+	if baseHasTrailingSlash {
+		base += "/"
+	}
+
 	if CaseSensitivePath {
 		return strings.HasPrefix(string(p), base)
 	}

caddyhttp/httpserver/path_test.go

@@ -29,6 +29,11 @@ func TestPathMatches(t *testing.T) {
 			rulePath:    "/foo/bar",
 			shouldMatch: false,
 		},
+		{
+			reqPath:     "/foo/",
+			rulePath:    "/foo/",
+			shouldMatch: true,
+		},
 		{
 			reqPath:     "/Foobar",
 			rulePath:    "/Foo",
@@ -86,10 +91,41 @@ func TestPathMatches(t *testing.T) {
 			rulePath:    "",
 			shouldMatch: true,
 		},
+		{
+			// see issue #1859
+			reqPath:     "//double-slash",
+			rulePath:    "/double-slash",
+			shouldMatch: true,
+		},
+		{
+			reqPath:     "/double//slash",
+			rulePath:    "/double/slash",
+			shouldMatch: true,
+		},
+		{
+			reqPath:     "//more/double//slashes",
+			rulePath:    "/more/double/slashes",
+			shouldMatch: true,
+		},
+		{
+			reqPath:     "/path/../traversal",
+			rulePath:    "/traversal",
+			shouldMatch: true,
+		},
+		{
+			reqPath:     "/path/../traversal",
+			rulePath:    "/path",
+			shouldMatch: false,
+		},
+		{
+			reqPath:     "/keep-slashes/http://something/foo/bar",
+			rulePath:    "/keep-slashes/http://something",
+			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",
+			t.Errorf("Test %d: For request path '%s' and base path '%s': expected %v, got %v",
 				i, testcase.reqPath, testcase.rulePath, want, got)
 		}
 	}