summaryrefslogtreecommitdiff
path: root/modules
diff options
context:
space:
mode:
authorMatthew Holt <mholt@users.noreply.github.com>2019-12-17 16:30:26 -0700
committerMatthew Holt <mholt@users.noreply.github.com>2019-12-17 16:30:26 -0700
commit724c72867835f9287b3df3ee5a0d75327c0780cf (patch)
tree2da51af8e1098655aebb25ed09e4cdd47627c278 /modules
parent21408212dafc1b37e2f2d51a4c4afbcc9cef403b (diff)
rewrite: Attempt query string fix (#2891)
Diffstat (limited to 'modules')
-rw-r--r--modules/caddyhttp/fileserver/caddyfile.go3
-rw-r--r--modules/caddyhttp/rewrite/rewrite.go21
-rw-r--r--modules/caddyhttp/rewrite/rewrite_test.go11
3 files changed, 28 insertions, 7 deletions
diff --git a/modules/caddyhttp/fileserver/caddyfile.go b/modules/caddyhttp/fileserver/caddyfile.go
index 8013fa2..3bd0ae4 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 adb90f4..3a644ef 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 f496f0a..2fb5c66 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)