summaryrefslogtreecommitdiff
path: root/modules
diff options
context:
space:
mode:
authorgo-d <37667595+go-d@users.noreply.github.com>2021-01-11 17:18:53 +0100
committerGitHub <noreply@github.com>2021-01-11 09:18:53 -0700
commit88a38bd00d386457aec71696a386449d3bcf8990 (patch)
tree12df326ddc9282279b5b7be118dee158c38902b3 /modules
parent4f64105fbb9b315e8faf7a15144fe27e780d5384 (diff)
rewrite: Use RawPath instead of Path (fix #3596) (#3918)
Prevent information loss, i.e. the encoded form that was sent by the client, when using URL strip/replace.
Diffstat (limited to 'modules')
-rw-r--r--modules/caddyhttp/rewrite/rewrite.go33
-rw-r--r--modules/caddyhttp/rewrite/rewrite_test.go15
2 files changed, 37 insertions, 11 deletions
diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go
index d47c388..ad1c470 100644
--- a/modules/caddyhttp/rewrite/rewrite.go
+++ b/modules/caddyhttp/rewrite/rewrite.go
@@ -191,11 +191,21 @@ func (rewr Rewrite) rewrite(r *http.Request, repl *caddy.Replacer, logger *zap.L
// strip path prefix or suffix
if rewr.StripPathPrefix != "" {
prefix := repl.ReplaceAll(rewr.StripPathPrefix, "")
- r.URL.Path = strings.TrimPrefix(r.URL.Path, prefix)
+ r.URL.RawPath = strings.TrimPrefix(r.URL.RawPath, prefix)
+ if p, err := url.PathUnescape(r.URL.RawPath); err == nil && p != "" {
+ r.URL.Path = p
+ } else {
+ r.URL.Path = strings.TrimPrefix(r.URL.Path, prefix)
+ }
}
if rewr.StripPathSuffix != "" {
suffix := repl.ReplaceAll(rewr.StripPathSuffix, "")
- r.URL.Path = strings.TrimSuffix(r.URL.Path, suffix)
+ r.URL.RawPath = strings.TrimSuffix(r.URL.RawPath, suffix)
+ if p, err := url.PathUnescape(r.URL.RawPath); err == nil && p != "" {
+ r.URL.Path = p
+ } else {
+ r.URL.Path = strings.TrimSuffix(r.URL.Path, suffix)
+ }
}
// substring replacements in URI
@@ -289,10 +299,10 @@ type replacer struct {
Limit int `json:"limit,omitempty"`
}
-// do performs the replacement on r and returns true if any changes were made.
-func (rep replacer) do(r *http.Request, repl *caddy.Replacer) bool {
+// do performs the replacement on r.
+func (rep replacer) do(r *http.Request, repl *caddy.Replacer) {
if rep.Find == "" || rep.Replace == "" {
- return false
+ return
}
lim := rep.Limit
@@ -303,13 +313,14 @@ func (rep replacer) do(r *http.Request, repl *caddy.Replacer) bool {
find := repl.ReplaceAll(rep.Find, "")
replace := repl.ReplaceAll(rep.Replace, "")
- oldPath := r.URL.Path
- oldQuery := r.URL.RawQuery
-
- r.URL.Path = strings.Replace(oldPath, find, replace, lim)
- r.URL.RawQuery = strings.Replace(oldQuery, find, replace, lim)
+ r.URL.RawPath = strings.Replace(r.URL.RawPath, find, replace, lim)
+ if p, err := url.PathUnescape(r.URL.RawPath); err == nil && p != "" {
+ r.URL.Path = p
+ } else {
+ r.URL.Path = strings.Replace(r.URL.Path, find, replace, lim)
+ }
- return r.URL.Path != oldPath && r.URL.RawQuery != oldQuery
+ r.URL.RawQuery = strings.Replace(r.URL.RawQuery, find, replace, lim)
}
// Interface guard
diff --git a/modules/caddyhttp/rewrite/rewrite_test.go b/modules/caddyhttp/rewrite/rewrite_test.go
index fb4931b..9329a04 100644
--- a/modules/caddyhttp/rewrite/rewrite_test.go
+++ b/modules/caddyhttp/rewrite/rewrite_test.go
@@ -201,6 +201,11 @@ func TestRewrite(t *testing.T) {
},
{
rule: Rewrite{StripPathPrefix: "/prefix"},
+ input: newRequest(t, "GET", "/prefix/foo%2Fbar"),
+ expect: newRequest(t, "GET", "/foo%2Fbar"),
+ },
+ {
+ rule: Rewrite{StripPathPrefix: "/prefix"},
input: newRequest(t, "GET", "/foo/prefix/bar"),
expect: newRequest(t, "GET", "/foo/prefix/bar"),
},
@@ -216,6 +221,11 @@ func TestRewrite(t *testing.T) {
expect: newRequest(t, "GET", "/foo/bar/"),
},
{
+ rule: Rewrite{StripPathSuffix: "suffix"},
+ input: newRequest(t, "GET", "/foo%2Fbar/suffix"),
+ expect: newRequest(t, "GET", "/foo%2Fbar/"),
+ },
+ {
rule: Rewrite{StripPathSuffix: "/suffix"},
input: newRequest(t, "GET", "/foo/suffix/bar"),
expect: newRequest(t, "GET", "/foo/suffix/bar"),
@@ -231,6 +241,11 @@ func TestRewrite(t *testing.T) {
input: newRequest(t, "GET", "/foo/findme/bar"),
expect: newRequest(t, "GET", "/foo/replaced/bar"),
},
+ {
+ rule: Rewrite{URISubstring: []replacer{{Find: "findme", Replace: "replaced"}}},
+ input: newRequest(t, "GET", "/foo/findme%2Fbar"),
+ expect: newRequest(t, "GET", "/foo/replaced%2Fbar"),
+ },
} {
// copy the original input just enough so that we can
// compare it after the rewrite to see if it changed