rewrite: Add option to force modifying the query

This commit is contained in:
Francis Lavoie 2023-03-16 17:17:12 -04:00
parent 2182270a2c
commit 0839b54ebb
No known key found for this signature in database
GPG Key ID: 0F66EE1687682239
4 changed files with 58 additions and 6 deletions

View File

@ -11,7 +11,9 @@ reverse_proxy 127.0.0.1:65535 {
@accel header X-Accel-Redirect * @accel header X-Accel-Redirect *
handle_response @accel { handle_response @accel {
respond "Header X-Accel-Redirect!" rewrite * {rp.header.X-Accel-Redirect} {
modify_query
}
} }
@another { @another {
@ -104,10 +106,12 @@ reverse_proxy 127.0.0.1:65535 {
}, },
"routes": [ "routes": [
{ {
"group": "group0",
"handle": [ "handle": [
{ {
"body": "Header X-Accel-Redirect!", "handler": "rewrite",
"handler": "static_response" "modify_query": true,
"uri": "{http.reverse_proxy.header.X-Accel-Redirect}"
} }
] ]
} }

View File

@ -34,7 +34,9 @@ func init() {
// parseCaddyfileRewrite sets up a basic rewrite handler from Caddyfile tokens. Syntax: // parseCaddyfileRewrite sets up a basic rewrite handler from Caddyfile tokens. Syntax:
// //
// rewrite [<matcher>] <to> // rewrite [<matcher>] <to> {
// modify_query
// }
// //
// Only URI components which are given in <to> will be set in the resulting URI. // Only URI components which are given in <to> will be set in the resulting URI.
// See the docs for the rewrite handler for more information. // See the docs for the rewrite handler for more information.
@ -48,6 +50,16 @@ func parseCaddyfileRewrite(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler,
if h.NextArg() { if h.NextArg() {
return nil, h.ArgErr() return nil, h.ArgErr()
} }
for nesting := h.Nesting(); h.NextBlock(nesting); {
switch h.Val() {
case "modify_query":
rewr.ModifyQuery = true
default:
return nil, h.Errf("unknown subdirective: %s", h.Val())
}
}
} }
return rewr, nil return rewr, nil
} }

View File

@ -88,6 +88,17 @@ type Rewrite struct {
// Performs regular expression replacements on the URI path. // Performs regular expression replacements on the URI path.
PathRegexp []*regexReplacer `json:"path_regexp,omitempty"` PathRegexp []*regexReplacer `json:"path_regexp,omitempty"`
// If true, the rewrite will be forced to also apply to the
// query part of the URL. This is only needed if the configured
// URI does not include a '?' character which is normally used
// to determine whether the query should be modified. In other
// words, this allows rewriting both the path and query when
// using a placeholder as the replacement value, whereas otherwise
// only the path would be rewritten because the placeholder itself
// does not contain a '?' character. Only use this if the placeholder
// is trusted to not be vulnerable to query injections.
ModifyQuery bool `json:"modify_query,omitempty"`
logger *zap.Logger logger *zap.Logger
} }
@ -216,10 +227,15 @@ func (rewr Rewrite) Rewrite(r *http.Request, repl *caddy.Replacer) bool {
// recompute; new path contains a query string // recompute; new path contains a query string
var injectedQuery string var injectedQuery string
newPath, injectedQuery = before, after newPath, injectedQuery = before, after
// don't overwrite explicitly-configured query string
if query == "" { // don't overwrite explicitly-configured query string,
// unless configured explicitly to do so
if query == "" || rewr.ModifyQuery {
query = injectedQuery query = injectedQuery
} }
if rewr.ModifyQuery {
qsStart = 0
}
} }
if query != "" { if query != "" {

View File

@ -214,6 +214,23 @@ func TestRewrite(t *testing.T) {
input: newRequest(t, "GET", "/foo#fragFirst?c=d"), input: newRequest(t, "GET", "/foo#fragFirst?c=d"),
expect: newRequest(t, "GET", "/bar#fragFirst?c=d"), expect: newRequest(t, "GET", "/bar#fragFirst?c=d"),
}, },
{
rule: Rewrite{URI: "{test.path_and_query}"},
input: newRequest(t, "GET", "/"),
expect: newRequest(t, "GET", "/foo"),
},
{
// TODO: This might be an incorrect result, since it also replaces
// the path with empty string when that might not be the intent.
rule: Rewrite{URI: "{test.query}", ModifyQuery: true},
input: newRequest(t, "GET", "/foo"),
expect: newRequest(t, "GET", "?bar=1"),
},
{
rule: Rewrite{URI: "{test.path_and_query}", ModifyQuery: true},
input: newRequest(t, "GET", "/"),
expect: newRequest(t, "GET", "/foo?bar=1"),
},
{ {
rule: Rewrite{StripPathPrefix: "/prefix"}, rule: Rewrite{StripPathPrefix: "/prefix"},
@ -343,6 +360,9 @@ func TestRewrite(t *testing.T) {
repl.Set("http.request.uri", tc.input.RequestURI) repl.Set("http.request.uri", tc.input.RequestURI)
repl.Set("http.request.uri.path", tc.input.URL.Path) 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", tc.input.URL.RawQuery)
repl.Set("test.path", "/foo")
repl.Set("test.query", "?bar=1")
repl.Set("test.path_and_query", "/foo?bar=1")
// we can't directly call Provision() without a valid caddy.Context // we can't directly call Provision() without a valid caddy.Context
// (TODO: fix that) so here we ad-hoc compile the regex // (TODO: fix that) so here we ad-hoc compile the regex