From 5e9d81b507e0beb46b3812e21566bfef79c87af4 Mon Sep 17 00:00:00 2001
From: Matthew Holt <mholt@users.noreply.github.com>
Date: Thu, 12 Dec 2019 15:27:09 -0700
Subject: [PATCH] try_files, rewrite: allow query string in try_files (fix
#2891)
Also some minor cleanup/improvements discovered along the way
---
caddyconfig/httpcaddyfile/directives.go | 13 +++--
modules/caddyhttp/fileserver/caddyfile.go | 52 +++++++++++++++----
.../reverseproxy/fastcgi/caddyfile.go | 10 ++--
modules/caddyhttp/rewrite/rewrite.go | 5 --
4 files changed, 57 insertions(+), 23 deletions(-)
diff --git a/caddyconfig/httpcaddyfile/directives.go b/caddyconfig/httpcaddyfile/directives.go
index 58aff98a6..5ec499301 100644
--- a/caddyconfig/httpcaddyfile/directives.go
+++ b/caddyconfig/httpcaddyfile/directives.go
@@ -53,6 +53,8 @@ func RegisterDirective(dir string, setupFunc UnmarshalFunc) {
// RegisterHandlerDirective is like RegisterDirective, but for
// directives which specifically output only an HTTP handler.
+// Directives registered with this function will always have
+// an optional matcher token as the first argument.
func RegisterHandlerDirective(dir string, setupFunc UnmarshalHandlerFunc) {
RegisterDirective(dir, func(h Helper) ([]ConfigValue, error) {
if !h.Next() {
@@ -117,7 +119,7 @@ func (h Helper) Caddyfiles() []string {
}
// JSON converts val into JSON. Any errors are added to warnings.
-func (h Helper) JSON(val interface{}, warnings *[]caddyconfig.Warning) json.RawMessage {
+func (h Helper) JSON(val interface{}) json.RawMessage {
return caddyconfig.JSON(val, h.warnings)
}
@@ -135,9 +137,14 @@ func (h Helper) MatcherToken() (caddy.ModuleMap, bool, error) {
// NewRoute returns config values relevant to creating a new HTTP route.
func (h Helper) NewRoute(matcherSet caddy.ModuleMap,
handler caddyhttp.MiddlewareHandler) []ConfigValue {
- mod, err := caddy.GetModule(caddy.GetModuleName(handler))
+ mod, err := caddy.GetModule(caddy.GetModuleID(handler))
if err != nil {
- // TODO: append to warnings
+ *h.warnings = append(*h.warnings, caddyconfig.Warning{
+ File: h.File(),
+ Line: h.Line(),
+ Message: err.Error(),
+ })
+ return nil
}
var matcherSetsRaw []caddy.ModuleMap
if matcherSet != nil {
diff --git a/modules/caddyhttp/fileserver/caddyfile.go b/modules/caddyhttp/fileserver/caddyfile.go
index fb931a1d4..8013fa24b 100644
--- a/modules/caddyhttp/fileserver/caddyfile.go
+++ b/modules/caddyhttp/fileserver/caddyfile.go
@@ -15,6 +15,8 @@
package fileserver
import (
+ "strings"
+
"github.com/caddyserver/caddy/v2"
"github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile"
"github.com/caddyserver/caddy/v2/modules/caddyhttp"
@@ -98,7 +100,7 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
//
// try_files <files...>
//
-// and is shorthand for:
+// and is basically shorthand for:
//
// matcher:try_files {
// file {
@@ -107,25 +109,55 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
// }
// rewrite match:try_files {http.matchers.file.relative}{http.request.uri.query_string}
//
+// If any of the files in the list have a query string, the query string will
+// be ignored when checking for file existence, but will be augmented into
+// the request's URI when rewriting the request.
func parseTryFiles(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) {
if !h.Next() {
return nil, h.ArgErr()
}
- try := h.RemainingArgs()
- if len(try) == 0 {
+ tryFiles := h.RemainingArgs()
+ if len(tryFiles) == 0 {
return nil, h.ArgErr()
}
- handler := rewrite.Rewrite{
- URI: "{http.matchers.file.relative}{http.request.uri.query_string}",
+ // makeRoute returns a route that tries the files listed in try
+ // and then rewrites to the matched file, and appends writeURIAppend
+ // to the end of the query string.
+ makeRoute := func(try []string, writeURIAppend string) []httpcaddyfile.ConfigValue {
+ handler := rewrite.Rewrite{
+ URI: "{http.matchers.file.relative}{http.request.uri.query_string}" + writeURIAppend,
+ }
+ matcherSet := caddy.ModuleMap{
+ "file": h.JSON(MatchFile{
+ TryFiles: try,
+ }),
+ }
+ return h.NewRoute(matcherSet, handler)
}
- matcherSet := caddy.ModuleMap{
- "file": h.JSON(MatchFile{
- TryFiles: try,
- }, nil),
+ var result []httpcaddyfile.ConfigValue
+
+ // if there are query strings in the list, we have to split into
+ // a separate route for each item with a query string, because
+ // the rewrite is different for that item
+ var try []string
+ for _, item := range tryFiles {
+ if idx := strings.Index(item, "?"); idx >= 0 {
+ if len(try) > 0 {
+ result = append(result, makeRoute(try, "")...)
+ try = []string{}
+ }
+ result = append(result, makeRoute([]string{item[:idx]}, "&"+item[idx+1:])...)
+ continue
+ }
+ // accumulate consecutive non-query-string parameters
+ try = append(try, item)
+ }
+ if len(try) > 0 {
+ result = append(result, makeRoute(try, "")...)
}
- return h.NewRoute(matcherSet, handler), nil
+ return result, nil
}
diff --git a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go
index 8e723b2c4..699f6808a 100644
--- a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go
+++ b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go
@@ -125,12 +125,12 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error
redirMatcherSet := caddy.ModuleMap{
"file": h.JSON(fileserver.MatchFile{
TryFiles: []string{"{http.request.uri.path}/index.php"},
- }, nil),
+ }),
"not": h.JSON(caddyhttp.MatchNegate{
MatchersRaw: caddy.ModuleMap{
- "path": h.JSON(caddyhttp.MatchPath{"*/"}, nil),
+ "path": h.JSON(caddyhttp.MatchPath{"*/"}),
},
- }, nil),
+ }),
}
redirHandler := caddyhttp.StaticResponse{
StatusCode: caddyhttp.WeakString("308"),
@@ -145,7 +145,7 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error
rewriteMatcherSet := caddy.ModuleMap{
"file": h.JSON(fileserver.MatchFile{
TryFiles: []string{"{http.request.uri.path}", "{http.request.uri.path}/index.php", "index.php"},
- }, nil),
+ }),
}
rewriteHandler := rewrite.Rewrite{
URI: "{http.matchers.file.relative}{http.request.uri.query_string}",
@@ -159,7 +159,7 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error
// route to actually reverse proxy requests to PHP files;
// match only requests that are for PHP files
rpMatcherSet := caddy.ModuleMap{
- "path": h.JSON([]string{"*.php"}, nil),
+ "path": h.JSON([]string{"*.php"}),
}
// if the user specified a matcher token, use that
diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go
index a142080fb..adb90f484 100644
--- a/modules/caddyhttp/rewrite/rewrite.go
+++ b/modules/caddyhttp/rewrite/rewrite.go
@@ -190,11 +190,6 @@ func (rep replacer) do(r *http.Request, repl caddy.Replacer) bool {
r.URL.Path = strings.Replace(oldPath, find, replace, lim)
r.URL.RawQuery = strings.Replace(oldQuery, find, replace, lim)
- // changed := r.URL.Path != oldPath && r.URL.RawQuery != oldQuery
- // if changed {
- // r.RequestURI = r.URL.RequestURI()
- // }
-
return r.URL.Path != oldPath && r.URL.RawQuery != oldQuery
}