diff --git a/modules/caddyhttp/fileserver/caddyfile.go b/modules/caddyhttp/fileserver/caddyfile.go index 8013fa24..3bd0ae47 100644 --- a/modules/caddyhttp/fileserver/caddyfile.go +++ b/modules/caddyhttp/fileserver/caddyfile.go @@ -127,7 +127,8 @@ func parseTryFiles(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) // 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, + Rehandle: true, + URI: "{http.matchers.file.relative}{http.request.uri.query_string}" + writeURIAppend, } matcherSet := caddy.ModuleMap{ "file": h.JSON(MatchFile{ diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go index adb90f48..3a644ef4 100644 --- a/modules/caddyhttp/rewrite/rewrite.go +++ b/modules/caddyhttp/rewrite/rewrite.go @@ -126,9 +126,24 @@ func (rewr Rewrite) rewrite(r *http.Request, repl caddy.Replacer, logger *zap.Lo if newU.Path != "" { r.URL.Path = newU.Path } - if newU.RawQuery != "" { - newU.RawQuery = strings.TrimPrefix(newU.RawQuery, "&") - r.URL.RawQuery = newU.RawQuery + if strings.Contains(newURI, "?") { + // you'll notice we check for existence of a question mark + // instead of RawQuery != "". We do this because if the user + // wants to remove an existing query string, they do that by + // appending "?" to the path: "/foo?" -- in this case, then, + // RawQuery is "" but we still want to set it to that; hence, + // we check for a "?", which always starts a query string + inputQuery := newU.Query() + outputQuery := make(url.Values) + for k := range inputQuery { + // overwrite existing values; we don't simply keep + // appending because it can cause rewrite rules like + // "{path}{query}&a=b" with rehandling enabled to go + // on forever: "/foo.html?a=b&a=b&a=b..." + outputQuery.Set(k, inputQuery.Get(k)) + } + // this sorts the keys, oh well + r.URL.RawQuery = outputQuery.Encode() } if newU.Fragment != "" { r.URL.Fragment = newU.Fragment diff --git a/modules/caddyhttp/rewrite/rewrite_test.go b/modules/caddyhttp/rewrite/rewrite_test.go index f496f0ae..2fb5c662 100644 --- a/modules/caddyhttp/rewrite/rewrite_test.go +++ b/modules/caddyhttp/rewrite/rewrite_test.go @@ -126,12 +126,17 @@ func TestRewrite(t *testing.T) { { rule: Rewrite{URI: "/index.php?c=d&{http.request.uri.query}"}, input: newRequest(t, "GET", "/?a=b"), - expect: newRequest(t, "GET", "/index.php?c=d&a=b"), + expect: newRequest(t, "GET", "/index.php?a=b&c=d"), }, { rule: Rewrite{URI: "/index.php?{http.request.uri.query}&p={http.request.uri.path}"}, input: newRequest(t, "GET", "/foo/bar?a=b"), - expect: newRequest(t, "GET", "/index.php?a=b&p=/foo/bar"), + expect: newRequest(t, "GET", "/index.php?a=b&p=%2Ffoo%2Fbar"), + }, + { + rule: Rewrite{URI: "{http.request.uri.path}?"}, + input: newRequest(t, "GET", "/foo/bar?a=b&c=d"), + expect: newRequest(t, "GET", "/foo/bar"), }, { @@ -188,7 +193,7 @@ func TestRewrite(t *testing.T) { // populate the replacer just enough for our tests repl.Set("http.request.uri.path", tc.input.URL.Path) repl.Set("http.request.uri.query", tc.input.URL.RawQuery) - repl.Set("http.request.uri.query_string", "?"+tc.input.URL.Query().Encode()) + repl.Set("http.request.uri.query_string", "?"+tc.input.URL.RawQuery) changed := tc.rule.rewrite(tc.input, repl, nil)