From a2e7f85bb927c03f3fee2bba95989c9fc77f3375 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 3 May 2021 23:38:25 -0400 Subject: [PATCH 1/2] fileserver: Fix `file` matcher with empty `try_files` Fixes https://github.com/caddyserver/caddy/issues/4146 If `TryFiles` is empty, we fill it with `r.URL.Path`. In this case, this is `/`. Then later, in `prepareFilePath()`, we run the replacer (which turns `{path}` into `/` at that point) but `file` remains the original value (and the placeholder is still the placeholder there). So then `strings.HasSuffix(file, "/")` will be `false` for the placeholder, but `true` for the empty `TryFiles` codepath, because `file` was `/` due to being set to the actual request value beforehand. This means that `suffix` becomes `//` in that case, so after `sanitizedPathJoin`, it becomes `./`, so `strictFileExists`'s `strings.HasSuffix(file, separator)` codepath will return true. I think we should change the `m.TryFiles == nil` codepath to `m.TryFiles = []string{"{http.request.uri.path}"}` for consistency. (And maybe consider hoisting this to `Provision` cause there's no point doing this on every request). I don't think this "optimization" of directly using `r.URL.Path` is so valuable, cause it causes this edgecase with directories. --- modules/caddyhttp/fileserver/matcher.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/modules/caddyhttp/fileserver/matcher.go b/modules/caddyhttp/fileserver/matcher.go index 44ef9edc04c..516095428ab 100644 --- a/modules/caddyhttp/fileserver/matcher.go +++ b/modules/caddyhttp/fileserver/matcher.go @@ -139,6 +139,10 @@ func (m *MatchFile) Provision(_ caddy.Context) error { if m.Root == "" { m.Root = "{http.vars.root}" } + // if list of files to try was omitted entirely, assume URL path + if m.TryFiles == nil { + m.TryFiles = []string{"{http.request.uri.path}"} + } return nil } @@ -174,13 +178,6 @@ func (m MatchFile) selectFile(r *http.Request) (matched bool) { root := repl.ReplaceAll(m.Root, ".") - // if list of files to try was omitted entirely, - // assume URL path - if m.TryFiles == nil { - // m is not a pointer, so this is safe - m.TryFiles = []string{r.URL.Path} - } - // common preparation of the file into parts prepareFilePath := func(file string) (suffix, fullpath, remainder string) { suffix, remainder = m.firstSplit(path.Clean(repl.ReplaceAll(file, ""))) From cacf9bc03c1451c5eeef425b69b4a3527f7f31b0 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Tue, 4 May 2021 10:48:46 -0400 Subject: [PATCH 2/2] Update modules/caddyhttp/fileserver/matcher.go Co-authored-by: Matt Holt --- modules/caddyhttp/fileserver/matcher.go | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/caddyhttp/fileserver/matcher.go b/modules/caddyhttp/fileserver/matcher.go index 516095428ab..064398c67a9 100644 --- a/modules/caddyhttp/fileserver/matcher.go +++ b/modules/caddyhttp/fileserver/matcher.go @@ -140,6 +140,7 @@ func (m *MatchFile) Provision(_ caddy.Context) error { m.Root = "{http.vars.root}" } // if list of files to try was omitted entirely, assume URL path + // (use placeholder instead of r.URL.Path; see issue #4146) if m.TryFiles == nil { m.TryFiles = []string{"{http.request.uri.path}"} }