@marcel91200 / caddy Mirror · read-only / commits / 724c7286
Fork 0

Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS

Mirrored from https://github.com/Slazee/caddy.git

Synced Nov 14, 05:15 UTC

rewrite: Attempt query string fix (#2891)

Matthew Holt committed Dec 17, 2019 at 16:30 UTC 724c72867835f9287b3df3ee5a0d75327c0780cf
3 files changed +28 -7
modules/caddyhttp/fileserver/caddyfile.go
+2 -1
@@ -127,7 +127,8 @@ func parseTryFiles(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error)
127 // to the end of the query string.
128 makeRoute := func(try []string, writeURIAppend string) []httpcaddyfile.ConfigValue {
129 handler := rewrite.Rewrite{
130 - URI: "{http.matchers.file.relative}{http.request.uri.query_string}" + writeURIAppend,
130 + Rehandle: true,
131 + URI: "{http.matchers.file.relative}{http.request.uri.query_string}" + writeURIAppend,
132 }
133 matcherSet := caddy.ModuleMap{
134 "file": h.JSON(MatchFile{
modules/caddyhttp/rewrite/rewrite.go
+18 -3
@@ -126,9 +126,24 @@ func (rewr Rewrite) rewrite(r *http.Request, repl caddy.Replacer, logger *zap.Lo
126 if newU.Path != "" {
127 r.URL.Path = newU.Path
128 }
129 - if newU.RawQuery != "" {
130 - newU.RawQuery = strings.TrimPrefix(newU.RawQuery, "&")
131 - r.URL.RawQuery = newU.RawQuery
129 + if strings.Contains(newURI, "?") {
130 + // you'll notice we check for existence of a question mark
131 + // instead of RawQuery != "". We do this because if the user
132 + // wants to remove an existing query string, they do that by
133 + // appending "?" to the path: "/foo?" -- in this case, then,
134 + // RawQuery is "" but we still want to set it to that; hence,
135 + // we check for a "?", which always starts a query string
136 + inputQuery := newU.Query()
137 + outputQuery := make(url.Values)
138 + for k := range inputQuery {
139 + // overwrite existing values; we don't simply keep
140 + // appending because it can cause rewrite rules like
141 + // "{path}{query}&a=b" with rehandling enabled to go
142 + // on forever: "/foo.html?a=b&a=b&a=b..."
143 + outputQuery.Set(k, inputQuery.Get(k))
144 + }
145 + // this sorts the keys, oh well
146 + r.URL.RawQuery = outputQuery.Encode()
147 }
148 if newU.Fragment != "" {
149 r.URL.Fragment = newU.Fragment
modules/caddyhttp/rewrite/rewrite_test.go
+8 -3
@@ -126,12 +126,17 @@ func TestRewrite(t *testing.T) {
126 {
127 rule: Rewrite{URI: "/index.php?c=d&{http.request.uri.query}"},
128 input: newRequest(t, "GET", "/?a=b"),
129 - expect: newRequest(t, "GET", "/index.php?c=d&a=b"),
129 + expect: newRequest(t, "GET", "/index.php?a=b&c=d"),
130 },
131 {
132 rule: Rewrite{URI: "/index.php?{http.request.uri.query}&p={http.request.uri.path}"},
133 input: newRequest(t, "GET", "/foo/bar?a=b"),
134 - expect: newRequest(t, "GET", "/index.php?a=b&p=/foo/bar"),
134 + expect: newRequest(t, "GET", "/index.php?a=b&p=%2Ffoo%2Fbar"),
135 + },
136 + {
137 + rule: Rewrite{URI: "{http.request.uri.path}?"},
138 + input: newRequest(t, "GET", "/foo/bar?a=b&c=d"),
139 + expect: newRequest(t, "GET", "/foo/bar"),
140 },
141
142 {
@@ -188,7 +193,7 @@ func TestRewrite(t *testing.T) {
193 // populate the replacer just enough for our tests
194 repl.Set("http.request.uri.path", tc.input.URL.Path)
195 repl.Set("http.request.uri.query", tc.input.URL.RawQuery)
191 - repl.Set("http.request.uri.query_string", "?"+tc.input.URL.Query().Encode())
196 + repl.Set("http.request.uri.query_string", "?"+tc.input.URL.RawQuery)
197
198 changed := tc.rule.rewrite(tc.input, repl, nil)
199