diff options
author | Matthew Holt <mholt@users.noreply.github.com> | 2020-04-01 00:43:40 -0600 |
---|---|---|
committer | Matthew Holt <mholt@users.noreply.github.com> | 2020-04-01 00:43:40 -0600 |
commit | 809e72792c501ceeadac4a3b9b327dfc575b9dfd (patch) | |
tree | a0cf16092fd3270430c9718974450495148f0cb1 | |
parent | 9fb0b1e838e216f90d20c9a32e948ad60dcca07d (diff) |
rewrite: Fix for rewrites with URI placeholders (#3209)
If a placeholder in the path component injects a query string such as
the {http.request.uri} placeholder is wont to do, we need to separate it
out from the path.
-rw-r--r-- | modules/caddyhttp/rewrite/rewrite.go | 39 | ||||
-rw-r--r-- | modules/caddyhttp/rewrite/rewrite_test.go | 31 |
2 files changed, 63 insertions, 7 deletions
diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go index 3ba63c4..d47c388 100644 --- a/modules/caddyhttp/rewrite/rewrite.go +++ b/modules/caddyhttp/rewrite/rewrite.go @@ -114,7 +114,7 @@ func (rewr Rewrite) rewrite(r *http.Request, repl *caddy.Replacer, logger *zap.L r.Method = strings.ToUpper(repl.ReplaceAll(rewr.Method, "")) } - // uri (path, query string, and fragment... because why not) + // uri (path, query string and... fragment, because why not) if uri := rewr.URI; uri != "" { // find the bounds of each part of the URI that exist pathStart, qsStart, fragStart := -1, -1, -1 @@ -136,18 +136,43 @@ func (rewr Rewrite) rewrite(r *http.Request, repl *caddy.Replacer, logger *zap.L qsEnd = len(uri) } + // isolate the three main components of the URI + var path, query, frag string + if pathStart > -1 { + path = uri[pathStart:pathEnd] + } + if qsStart > -1 { + query = uri[qsStart:qsEnd] + } + if fragStart > -1 { + frag = uri[fragStart:] + } + // build components which are specified, and store them // in a temporary variable so that they all read the // same version of the URI var newPath, newQuery, newFrag string - if pathStart >= 0 { - newPath = repl.ReplaceAll(uri[pathStart:pathEnd], "") + if path != "" { + newPath = repl.ReplaceAll(path, "") } - if qsStart >= 0 { - newQuery = buildQueryString(uri[qsStart:qsEnd], repl) + + // before continuing, we need to check if a query string + // snuck into the path component during replacements + if quPos := strings.Index(newPath, "?"); quPos > -1 { + // recompute; new path contains a query string + var injectedQuery string + newPath, injectedQuery = newPath[:quPos], newPath[quPos+1:] + // don't overwrite explicitly-configured query string + if query == "" { + query = injectedQuery + } } - if fragStart >= 0 { - newFrag = repl.ReplaceAll(uri[fragStart:], "") + + if query != "" { + newQuery = buildQueryString(query, repl) + } + if frag != "" { + newFrag = repl.ReplaceAll(frag, "") } // update the URI with the new components diff --git a/modules/caddyhttp/rewrite/rewrite_test.go b/modules/caddyhttp/rewrite/rewrite_test.go index 34a0cdb..fb4931b 100644 --- a/modules/caddyhttp/rewrite/rewrite_test.go +++ b/modules/caddyhttp/rewrite/rewrite_test.go @@ -158,6 +158,36 @@ func TestRewrite(t *testing.T) { input: newRequest(t, "GET", "/foo/bar?a=b"), expect: newRequest(t, "GET", "/foo?a=b#frag"), }, + { + rule: Rewrite{URI: "/foo{http.request.uri}"}, + input: newRequest(t, "GET", "/bar?a=b"), + expect: newRequest(t, "GET", "/foo/bar?a=b"), + }, + { + rule: Rewrite{URI: "/foo{http.request.uri}"}, + input: newRequest(t, "GET", "/bar"), + expect: newRequest(t, "GET", "/foo/bar"), + }, + { + rule: Rewrite{URI: "/foo{http.request.uri}?c=d"}, + input: newRequest(t, "GET", "/bar?a=b"), + expect: newRequest(t, "GET", "/foo/bar?c=d"), + }, + { + rule: Rewrite{URI: "/foo{http.request.uri}?{http.request.uri.query}&c=d"}, + input: newRequest(t, "GET", "/bar?a=b"), + expect: newRequest(t, "GET", "/foo/bar?a=b&c=d"), + }, + { + rule: Rewrite{URI: "{http.request.uri}"}, + input: newRequest(t, "GET", "/bar?a=b"), + expect: newRequest(t, "GET", "/bar?a=b"), + }, + { + rule: Rewrite{URI: "{http.request.uri.path}bar?c=d"}, + input: newRequest(t, "GET", "/foo/?a=b"), + expect: newRequest(t, "GET", "/foo/bar?c=d"), + }, { rule: Rewrite{StripPathPrefix: "/prefix"}, @@ -211,6 +241,7 @@ func TestRewrite(t *testing.T) { } // populate the replacer just enough for our tests + repl.Set("http.request.uri", tc.input.RequestURI) repl.Set("http.request.uri.path", tc.input.URL.Path) repl.Set("http.request.uri.query", tc.input.URL.RawQuery) |