From 13a37688dcdc1ffa8e9322dad0bffac0c0c9893a Mon Sep 17 00:00:00 2001
From: Tran Phong <letranphong2k1@gmail.com>
Date: Tue, 16 May 2023 22:16:07 +0700
Subject: rewrite: use escaped path, fix #5278 (#5504)

* use escaped path while rewriting

Signed-off-by: TP-O <letranphong2k1@gmail.com>

* restore line break

---------

Signed-off-by: TP-O <letranphong2k1@gmail.com>
---
 modules/caddyhttp/rewrite/rewrite.go      | 20 +++++++++-----------
 modules/caddyhttp/rewrite/rewrite_test.go | 12 +++++++++++-
 2 files changed, 20 insertions(+), 12 deletions(-)

(limited to 'modules/caddyhttp/rewrite')

diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go
index 31c9778..499d217 100644
--- a/modules/caddyhttp/rewrite/rewrite.go
+++ b/modules/caddyhttp/rewrite/rewrite.go
@@ -195,16 +195,10 @@ func (rewr Rewrite) Rewrite(r *http.Request, repl *caddy.Replacer) bool {
 		var newPath, newQuery, newFrag string
 
 		if path != "" {
-			// Since the 'uri' placeholder performs a URL-encode,
-			// we need to intercept it so that it doesn't, because
-			// otherwise we risk a double-encode of the path.
-			uriPlaceholder := "{http.request.uri}"
-			if strings.Contains(path, uriPlaceholder) {
-				tmpUri := r.URL.Path
-				if r.URL.RawQuery != "" {
-					tmpUri += "?" + r.URL.RawQuery
-				}
-				path = strings.ReplaceAll(path, uriPlaceholder, tmpUri)
+			// replace the `path` placeholder to escaped path
+			pathPlaceholder := "{http.request.uri.path}"
+			if strings.Contains(path, pathPlaceholder) {
+				path = strings.ReplaceAll(path, pathPlaceholder, r.URL.EscapedPath())
 			}
 
 			newPath = repl.ReplaceAll(path, "")
@@ -232,7 +226,11 @@ func (rewr Rewrite) Rewrite(r *http.Request, repl *caddy.Replacer) bool {
 		// update the URI with the new components
 		// only after building them
 		if pathStart >= 0 {
-			r.URL.Path = newPath
+			if path, err := url.PathUnescape(newPath); err != nil {
+				r.URL.Path = newPath
+			} else {
+				r.URL.Path = path
+			}
 		}
 		if qsStart >= 0 {
 			r.URL.RawQuery = newQuery
diff --git a/modules/caddyhttp/rewrite/rewrite_test.go b/modules/caddyhttp/rewrite/rewrite_test.go
index 5875983..bb937ec 100644
--- a/modules/caddyhttp/rewrite/rewrite_test.go
+++ b/modules/caddyhttp/rewrite/rewrite_test.go
@@ -59,6 +59,16 @@ func TestRewrite(t *testing.T) {
 			input:  newRequest(t, "GET", "/"),
 			expect: newRequest(t, "GET", "foo"),
 		},
+		{
+			rule:   Rewrite{URI: "{http.request.uri}"},
+			input:  newRequest(t, "GET", "/bar%3Fbaz?c=d"),
+			expect: newRequest(t, "GET", "/bar%3Fbaz?c=d"),
+		},
+		{
+			rule:   Rewrite{URI: "{http.request.uri.path}"},
+			input:  newRequest(t, "GET", "/bar%3Fbaz"),
+			expect: newRequest(t, "GET", "/bar%3Fbaz"),
+		},
 		{
 			rule:   Rewrite{URI: "/foo{http.request.uri.path}"},
 			input:  newRequest(t, "GET", "/bar"),
@@ -323,7 +333,7 @@ func TestRewrite(t *testing.T) {
 			input:  newRequest(t, "GET", "/foo/findme%2Fbar"),
 			expect: newRequest(t, "GET", "/foo/replaced%2Fbar"),
 		},
-
+		
 		{
 			rule:   Rewrite{PathRegexp: []*regexReplacer{{Find: "/{2,}", Replace: "/"}}},
 			input:  newRequest(t, "GET", "/foo//bar///baz?a=b//c"),
-- 
cgit v1.2.3