From b95b87381a282e1fe57295d145b71645d7801f07 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Wed, 16 Sep 2020 20:09:28 -0400 Subject: fileserver: Fix try_files for directories; windows fix (#3684) * fileserver: Fix try_files for directories, windows fix * fileserver: Add new file type placeholder, refactoring, tests * fileserver: Review cleanup * fileserver: Flip the return args order --- modules/caddyhttp/fileserver/matcher.go | 85 ++++++++++++-------- modules/caddyhttp/fileserver/matcher_test.go | 93 ++++++++++++++++++++++ modules/caddyhttp/fileserver/staticfiles.go | 14 +++- modules/caddyhttp/fileserver/staticfiles_test.go | 6 +- modules/caddyhttp/fileserver/testdata/foo.txt | 1 + .../caddyhttp/fileserver/testdata/foodir/foo.txt | 1 + 6 files changed, 165 insertions(+), 35 deletions(-) create mode 100644 modules/caddyhttp/fileserver/testdata/foo.txt create mode 100644 modules/caddyhttp/fileserver/testdata/foodir/foo.txt (limited to 'modules/caddyhttp') diff --git a/modules/caddyhttp/fileserver/matcher.go b/modules/caddyhttp/fileserver/matcher.go index 1844421..475a9ee 100644 --- a/modules/caddyhttp/fileserver/matcher.go +++ b/modules/caddyhttp/fileserver/matcher.go @@ -19,6 +19,7 @@ import ( "net/http" "os" "path" + "path/filepath" "strings" "time" @@ -34,7 +35,7 @@ func init() { // MatchFile is an HTTP request matcher that can match // requests based upon file existence. // -// Upon matching, two new placeholders will be made +// Upon matching, three new placeholders will be made // available: // // - `{http.matchers.file.relative}` The root-relative @@ -42,6 +43,8 @@ func init() { // requests. // - `{http.matchers.file.absolute}` The absolute path // of the matched file. +// - `{http.matchers.file.type}` Set to "directory" if +// the matched file is a directory, "file" otherwise. type MatchFile struct { // The root directory, used for creating absolute // file paths, and required when working with @@ -153,25 +156,18 @@ func (m MatchFile) Validate() error { } // Match returns true if r matches m. Returns true -// if a file was matched. If so, two placeholders +// if a file was matched. If so, three placeholders // will be available: // - http.matchers.file.relative // - http.matchers.file.absolute +// - http.matchers.file.type func (m MatchFile) Match(r *http.Request) bool { - repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) - rel, abs, matched := m.selectFile(r) - if matched { - repl.Set("http.matchers.file.relative", rel) - repl.Set("http.matchers.file.absolute", abs) - } - return matched + return m.selectFile(r) } // selectFile chooses a file according to m.TryPolicy by appending // the paths in m.TryFiles to m.Root, with placeholder replacements. -// It returns the root-relative path to the matched file, the full -// or absolute path, and whether a match was made. -func (m MatchFile) selectFile(r *http.Request) (rel, abs string, matched bool) { +func (m MatchFile) selectFile(r *http.Request) (matched bool) { repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) root := repl.ReplaceAll(m.Root, ".") @@ -183,13 +179,35 @@ func (m MatchFile) selectFile(r *http.Request) (rel, abs string, matched bool) { m.TryFiles = []string{r.URL.Path} } + // common preparation of the file into parts + prepareFilePath := func(file string) (string, string) { + suffix := m.firstSplit(path.Clean(repl.ReplaceAll(file, ""))) + if strings.HasSuffix(file, "/") { + suffix += "/" + } + fullpath := sanitizedPathJoin(root, suffix) + return suffix, fullpath + } + + // sets up the placeholders for the matched file + setPlaceholders := func(info os.FileInfo, rel string, abs string) { + repl.Set("http.matchers.file.relative", rel) + repl.Set("http.matchers.file.absolute", abs) + + fileType := "file" + if info.IsDir() { + fileType = "directory" + } + repl.Set("http.matchers.file.type", fileType) + } + switch m.TryPolicy { case "", tryPolicyFirstExist: for _, f := range m.TryFiles { - suffix := m.firstSplit(path.Clean(repl.ReplaceAll(f, ""))) - fullpath := sanitizedPathJoin(root, suffix) - if strictFileExists(fullpath) { - return suffix, fullpath, true + suffix, fullpath := prepareFilePath(f) + if info, exists := strictFileExists(fullpath); exists { + setPlaceholders(info, suffix, fullpath) + return true } } @@ -197,9 +215,9 @@ func (m MatchFile) selectFile(r *http.Request) (rel, abs string, matched bool) { var largestSize int64 var largestFilename string var largestSuffix string + var info os.FileInfo for _, f := range m.TryFiles { - suffix := m.firstSplit(path.Clean(repl.ReplaceAll(f, ""))) - fullpath := sanitizedPathJoin(root, suffix) + suffix, fullpath := prepareFilePath(f) info, err := os.Stat(fullpath) if err == nil && info.Size() > largestSize { largestSize = info.Size() @@ -207,15 +225,16 @@ func (m MatchFile) selectFile(r *http.Request) (rel, abs string, matched bool) { largestSuffix = suffix } } - return largestSuffix, largestFilename, true + setPlaceholders(info, largestSuffix, largestFilename) + return true case tryPolicySmallestSize: var smallestSize int64 var smallestFilename string var smallestSuffix string + var info os.FileInfo for _, f := range m.TryFiles { - suffix := m.firstSplit(path.Clean(repl.ReplaceAll(f, ""))) - fullpath := sanitizedPathJoin(root, suffix) + suffix, fullpath := prepareFilePath(f) info, err := os.Stat(fullpath) if err == nil && (smallestSize == 0 || info.Size() < smallestSize) { smallestSize = info.Size() @@ -223,15 +242,16 @@ func (m MatchFile) selectFile(r *http.Request) (rel, abs string, matched bool) { smallestSuffix = suffix } } - return smallestSuffix, smallestFilename, true + setPlaceholders(info, smallestSuffix, smallestFilename) + return true case tryPolicyMostRecentlyMod: var recentDate time.Time var recentFilename string var recentSuffix string + var info os.FileInfo for _, f := range m.TryFiles { - suffix := m.firstSplit(path.Clean(repl.ReplaceAll(f, ""))) - fullpath := sanitizedPathJoin(root, suffix) + suffix, fullpath := prepareFilePath(f) info, err := os.Stat(fullpath) if err == nil && (recentDate.IsZero() || info.ModTime().After(recentDate)) { @@ -240,7 +260,8 @@ func (m MatchFile) selectFile(r *http.Request) (rel, abs string, matched bool) { recentSuffix = suffix } } - return recentSuffix, recentFilename, true + setPlaceholders(info, recentSuffix, recentFilename) + return true } return @@ -252,7 +273,7 @@ func (m MatchFile) selectFile(r *http.Request) (rel, abs string, matched bool) { // the file must also be a directory; if it does // NOT end in a forward slash, the file must NOT // be a directory. -func strictFileExists(file string) bool { +func strictFileExists(file string) (os.FileInfo, bool) { stat, err := os.Stat(file) if err != nil { // in reality, this can be any error @@ -263,16 +284,16 @@ func strictFileExists(file string) bool { // the file exists, so we just treat any // error as if it does not exist; see // https://stackoverflow.com/a/12518877/1048862 - return false + return nil, false } - if strings.HasSuffix(file, "/") { + if strings.HasSuffix(file, string(filepath.Separator)) { // by convention, file paths ending - // in a slash must be a directory - return stat.IsDir() + // in a path separator must be a directory + return stat, stat.IsDir() } // by convention, file paths NOT ending - // in a slash must NOT be a directory - return !stat.IsDir() + // in a path separator must NOT be a directory + return stat, !stat.IsDir() } // firstSplit returns the first result where the path diff --git a/modules/caddyhttp/fileserver/matcher_test.go b/modules/caddyhttp/fileserver/matcher_test.go index 51cd284..ddd4f78 100644 --- a/modules/caddyhttp/fileserver/matcher_test.go +++ b/modules/caddyhttp/fileserver/matcher_test.go @@ -22,30 +22,114 @@ import ( "github.com/caddyserver/caddy/v2/modules/caddyhttp" ) +func TestFileMatcher(t *testing.T) { + for i, tc := range []struct { + path string + expectedPath string + expectedType string + matched bool + }{ + { + path: "/foo.txt", + expectedPath: "/foo.txt", + expectedType: "file", + matched: true, + }, + { + path: "/foo.txt/", + expectedPath: "/foo.txt", + expectedType: "file", + matched: true, + }, + { + path: "/foodir", + expectedPath: "/foodir/", + expectedType: "directory", + matched: true, + }, + { + path: "/foodir/", + expectedPath: "/foodir/", + expectedType: "directory", + matched: true, + }, + { + path: "/foodir/foo.txt", + expectedPath: "/foodir/foo.txt", + expectedType: "file", + matched: true, + }, + { + path: "/missingfile.php", + matched: false, + }, + } { + m := &MatchFile{ + Root: "./testdata", + TryFiles: []string{"{http.request.uri.path}", "{http.request.uri.path}/"}, + } + + u, err := url.Parse(tc.path) + if err != nil { + t.Fatalf("Test %d: parsing path: %v", i, err) + } + + req := &http.Request{URL: u} + repl := caddyhttp.NewTestReplacer(req) + + result := m.Match(req) + if result != tc.matched { + t.Fatalf("Test %d: expected match=%t, got %t", i, tc.matched, result) + } + + rel, ok := repl.Get("http.matchers.file.relative") + if !ok && result { + t.Fatalf("Test %d: expected replacer value", i) + } + if !result { + continue + } + + if rel != tc.expectedPath { + t.Fatalf("Test %d: actual path: %v, expected: %v", i, rel, tc.expectedPath) + } + + fileType, ok := repl.Get("http.matchers.file.type") + if fileType != tc.expectedType { + t.Fatalf("Test %d: actual file type: %v, expected: %v", i, fileType, tc.expectedType) + } + } +} + func TestPHPFileMatcher(t *testing.T) { for i, tc := range []struct { path string expectedPath string + expectedType string matched bool }{ { path: "/index.php", expectedPath: "/index.php", + expectedType: "file", matched: true, }, { path: "/index.php/somewhere", expectedPath: "/index.php", + expectedType: "file", matched: true, }, { path: "/remote.php", expectedPath: "/remote.php", + expectedType: "file", matched: true, }, { path: "/remote.php/somewhere", expectedPath: "/remote.php", + expectedType: "file", matched: true, }, { @@ -55,11 +139,13 @@ func TestPHPFileMatcher(t *testing.T) { { path: "/notphp.php.txt", expectedPath: "/notphp.php.txt", + expectedType: "file", matched: true, }, { path: "/notphp.php.txt/", expectedPath: "/notphp.php.txt", + expectedType: "file", matched: true, }, { @@ -69,12 +155,14 @@ func TestPHPFileMatcher(t *testing.T) { { path: "/foo.php.php/index.php", expectedPath: "/foo.php.php/index.php", + expectedType: "file", matched: true, }, { // See https://github.com/caddyserver/caddy/issues/3623 path: "/%E2%C3", expectedPath: "/%E2%C3", + expectedType: "file", matched: false, }, } { @@ -108,6 +196,11 @@ func TestPHPFileMatcher(t *testing.T) { if rel != tc.expectedPath { t.Fatalf("Test %d: actual path: %v, expected: %v", i, rel, tc.expectedPath) } + + fileType, ok := repl.Get("http.matchers.file.type") + if fileType != tc.expectedType { + t.Fatalf("Test %d: actual file type: %v, expected: %v", i, fileType, tc.expectedType) + } } } diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go index 735352b..2a7e013 100644 --- a/modules/caddyhttp/fileserver/staticfiles.go +++ b/modules/caddyhttp/fileserver/staticfiles.go @@ -23,7 +23,6 @@ import ( "mime" "net/http" "os" - "path" "path/filepath" "strconv" "strings" @@ -323,7 +322,18 @@ func sanitizedPathJoin(root, reqPath string) string { if root == "" { root = "." } - return filepath.Join(root, filepath.FromSlash(path.Clean("/"+reqPath))) + + path := filepath.Join(root, filepath.Clean("/"+reqPath)) + + // filepath.Join also cleans the path, and cleaning strips + // the trailing slash, so we need to re-add it afterwards. + // if the length is 1, then it's a path to the root, + // and that should return ".", so we don't append the separator. + if strings.HasSuffix(reqPath, "/") && len(reqPath) > 1 { + path += string(filepath.Separator) + } + + return path } // fileHidden returns true if filename is hidden diff --git a/modules/caddyhttp/fileserver/staticfiles_test.go b/modules/caddyhttp/fileserver/staticfiles_test.go index c47fbd1..1bbd77b 100644 --- a/modules/caddyhttp/fileserver/staticfiles_test.go +++ b/modules/caddyhttp/fileserver/staticfiles_test.go @@ -42,6 +42,10 @@ func TestSanitizedPathJoin(t *testing.T) { inputPath: "/foo", expect: "foo", }, + { + inputPath: "/foo/", + expect: "foo" + string(filepath.Separator), + }, { inputPath: "/foo/bar", expect: filepath.Join("foo", "bar"), @@ -73,7 +77,7 @@ func TestSanitizedPathJoin(t *testing.T) { { inputRoot: "/a/b", inputPath: "/%2e%2e%2f%2e%2e%2f", - expect: filepath.Join("/", "a", "b"), + expect: filepath.Join("/", "a", "b") + string(filepath.Separator), }, { inputRoot: "C:\\www", diff --git a/modules/caddyhttp/fileserver/testdata/foo.txt b/modules/caddyhttp/fileserver/testdata/foo.txt new file mode 100644 index 0000000..996f178 --- /dev/null +++ b/modules/caddyhttp/fileserver/testdata/foo.txt @@ -0,0 +1 @@ +foo.txt \ No newline at end of file diff --git a/modules/caddyhttp/fileserver/testdata/foodir/foo.txt b/modules/caddyhttp/fileserver/testdata/foodir/foo.txt new file mode 100644 index 0000000..0e3335b --- /dev/null +++ b/modules/caddyhttp/fileserver/testdata/foodir/foo.txt @@ -0,0 +1 @@ +foodir/foo.txt \ No newline at end of file -- cgit v1.2.3