From 5e467883b882780ff0334cc9a5487cd6abd26dc9 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Tue, 2 May 2017 09:43:43 -0600 Subject: [PATCH] httpserver: Base path of "/" matches all paths, even empty ones Fixes #1645 --- caddyhttp/httpserver/path.go | 18 +++++--- caddyhttp/httpserver/path_test.go | 61 +++++++++++++++++++++++++ caddyhttp/httpserver/tplcontext_test.go | 2 +- 3 files changed, 74 insertions(+), 7 deletions(-) create mode 100644 caddyhttp/httpserver/path_test.go 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 { - if CaseSensitivePath { - return strings.HasPrefix(string(p), other) +func (p Path) Matches(base string) bool { + if base == "/" { + return true } - return strings.HasPrefix(strings.ToLower(string(p)), strings.ToLower(other)) + if CaseSensitivePath { + return strings.HasPrefix(string(p), base) + } + 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 {