Commit f6d75bb7 authored by Matthew Holt's avatar Matthew Holt

httpserver: Fix #1859 by cleaning paths when matching them

Signed-off-by: default avatarMatthew Holt <mholt@users.noreply.github.com>
parent 32bb6a4c
...@@ -227,7 +227,7 @@ func TestBrowseTemplate(t *testing.T) { ...@@ -227,7 +227,7 @@ func TestBrowseTemplate(t *testing.T) {
func TestBrowseJson(t *testing.T) { func TestBrowseJson(t *testing.T) {
b := Browse{ b := Browse{
Next: httpserver.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) { 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 return 0, nil
}), }),
Configs: []Config{ Configs: []Config{
......
...@@ -2,6 +2,7 @@ package httpserver ...@@ -2,6 +2,7 @@ package httpserver
import ( import (
"net/http" "net/http"
"path"
"strings" "strings"
) )
...@@ -16,10 +17,27 @@ type Path string ...@@ -16,10 +17,27 @@ type Path string
// Path matching will probably not always be a direct // Path matching will probably not always be a direct
// comparison; this method assures that paths can be // comparison; this method assures that paths can be
// easily and consistently matched. // easily and consistently matched.
//
// Multiple slashes are collapsed/merged. See issue #1859.
func (p Path) Matches(base string) bool { func (p Path) Matches(base string) bool {
if base == "/" { if base == "/" || base == "" {
return true 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 { if CaseSensitivePath {
return strings.HasPrefix(string(p), base) return strings.HasPrefix(string(p), base)
} }
......
...@@ -29,6 +29,11 @@ func TestPathMatches(t *testing.T) { ...@@ -29,6 +29,11 @@ func TestPathMatches(t *testing.T) {
rulePath: "/foo/bar", rulePath: "/foo/bar",
shouldMatch: false, shouldMatch: false,
}, },
{
reqPath: "/foo/",
rulePath: "/foo/",
shouldMatch: true,
},
{ {
reqPath: "/Foobar", reqPath: "/Foobar",
rulePath: "/Foo", rulePath: "/Foo",
...@@ -86,10 +91,41 @@ func TestPathMatches(t *testing.T) { ...@@ -86,10 +91,41 @@ func TestPathMatches(t *testing.T) {
rulePath: "", rulePath: "",
shouldMatch: true, 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 CaseSensitivePath = !testcase.caseInsensitive
if got, want := testcase.reqPath.Matches(testcase.rulePath), testcase.shouldMatch; got != want { 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) i, testcase.reqPath, testcase.rulePath, want, got)
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment