1
Fork 0
mirror of https://github.com/caddyserver/caddy.git synced 2024-12-16 21:56:40 -05:00

rewrite: Make URI modifications more transactional (#2891)

Before, modifying the path might have affected how a new query string
was built if the query string relied on the path. Now, we build each
component in isolation and only change the URI on the request later.

Also, prevent trailing & in query string.
This commit is contained in:
Matthew Holt 2020-01-15 11:44:21 -07:00
parent 271b5af148
commit 07ad4655db
No known key found for this signature in database
GPG key ID: 2A349DD577D586A5
2 changed files with 31 additions and 5 deletions

View file

@ -112,7 +112,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 just because)
// 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
@ -134,14 +134,30 @@ func (rewr Rewrite) rewrite(r *http.Request, repl *caddy.Replacer, logger *zap.L
qsEnd = len(uri)
}
// 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 {
r.URL.Path = repl.ReplaceAll(uri[pathStart:pathEnd], "")
newPath = repl.ReplaceAll(uri[pathStart:pathEnd], "")
}
if qsStart >= 0 {
r.URL.RawQuery = buildQueryString(uri[qsStart:qsEnd], repl)
newQuery = buildQueryString(uri[qsStart:qsEnd], repl)
}
if fragStart >= 0 {
r.URL.Fragment = repl.ReplaceAll(uri[fragStart:], "")
newFrag = repl.ReplaceAll(uri[fragStart:], "")
}
// update the URI with the new components
// only after building them
if pathStart >= 0 {
r.URL.Path = newPath
}
if qsStart >= 0 {
r.URL.RawQuery = newQuery
}
if fragStart >= 0 {
r.URL.Fragment = newFrag
}
}
@ -206,7 +222,7 @@ func buildQueryString(qs string, repl *caddy.Replacer) string {
// if previous iteration wrote a value,
// that means we are writing a key
if wroteVal {
if sb.Len() > 0 {
if sb.Len() > 0 && len(comp) > 0 {
sb.WriteRune('&')
}
} else {

View file

@ -63,6 +63,16 @@ func TestRewrite(t *testing.T) {
input: newRequest(t, "GET", "/bar"),
expect: newRequest(t, "GET", "/foo/bar"),
},
{
rule: Rewrite{URI: "/index.php?p={http.request.uri.path}"},
input: newRequest(t, "GET", "/foo/bar"),
expect: newRequest(t, "GET", "/index.php?p=%2Ffoo%2Fbar"),
},
{
rule: Rewrite{URI: "?a=b&{http.request.uri.query}"},
input: newRequest(t, "GET", "/"),
expect: newRequest(t, "GET", "/?a=b"),
},
{
rule: Rewrite{URI: "/?c=d"},
input: newRequest(t, "GET", "/"),