Added support for replacement containing escaped characters

Co-authored-by: Ludovic Fernandez <ldez@users.noreply.github.com>

pkg/middlewares/replacepath/replace_path.go

@@ -3,6 +3,7 @@ package replacepath
 import (
 	"context"
 	"net/http"
+	"net/url"
 
 	"github.com/containous/traefik/v2/pkg/config/dynamic"
 	"github.com/containous/traefik/v2/pkg/log"
@@ -40,8 +41,22 @@ func (r *replacePath) GetTracingInformation() (string, ext.SpanKindEnum) {
 }
 
 func (r *replacePath) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
+	if req.URL.RawPath == "" {
 		req.Header.Add(ReplacedPathHeader, req.URL.Path)
-	req.URL.Path = r.path
+	} else {
+		req.Header.Add(ReplacedPathHeader, req.URL.RawPath)
+	}
+
+	req.URL.RawPath = r.path
+
+	var err error
+	req.URL.Path, err = url.PathUnescape(req.URL.RawPath)
+	if err != nil {
+		log.FromContext(middlewares.GetLoggerCtx(context.Background(), r.name, typeName)).Error(err)
+		http.Error(rw, err.Error(), http.StatusInternalServerError)
+		return
+	}
+
 	req.RequestURI = req.URL.RequestURI()
 
 	r.next.ServeHTTP(rw, req)

pkg/middlewares/replacepath/replace_path_test.go

@@ -3,43 +3,93 @@ package replacepath
 import (
 	"context"
 	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/containous/traefik/v2/pkg/config/dynamic"
-	"github.com/containous/traefik/v2/pkg/testhelpers"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestReplacePath(t *testing.T) {
-	var replacementConfig = dynamic.ReplacePath{
+	testCases := []struct {
+		desc            string
+		path            string
+		config          dynamic.ReplacePath
+		expectedPath    string
+		expectedRawPath string
+		expectedHeader  string
+	}{
+		{
+			desc: "simple path",
+			path: "/example",
+			config: dynamic.ReplacePath{
 				Path: "/replacement-path",
+			},
+			expectedPath:    "/replacement-path",
+			expectedRawPath: "",
+			expectedHeader:  "/example",
+		},
+		{
+			desc: "long path",
+			path: "/some/really/long/path",
+			config: dynamic.ReplacePath{
+				Path: "/replacement-path",
+			},
+			expectedPath:    "/replacement-path",
+			expectedRawPath: "",
+			expectedHeader:  "/some/really/long/path",
+		},
+		{
+			desc: "path with escaped value",
+			path: "/foo%2Fbar",
+			config: dynamic.ReplacePath{
+				Path: "/replacement-path",
+			},
+			expectedPath:    "/replacement-path",
+			expectedRawPath: "",
+			expectedHeader:  "/foo%2Fbar",
+		},
+		{
+			desc: "replacement with escaped value",
+			path: "/path",
+			config: dynamic.ReplacePath{
+				Path: "/foo%2Fbar",
+			},
+			expectedPath:    "/foo/bar",
+			expectedRawPath: "/foo%2Fbar",
+			expectedHeader:  "/path",
+		},
 	}
 
-	paths := []string{
+	for _, test := range testCases {
-		"/example",
+		t.Run(test.desc, func(t *testing.T) {
-		"/some/really/long/path",
+			var actualPath, actualRawPath, actualHeader, requestURI string
-	}
-
-	for _, path := range paths {
-		t.Run(path, func(t *testing.T) {
-			var expectedPath, actualHeader, requestURI string
 			next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-				expectedPath = r.URL.Path
+				actualPath = r.URL.Path
+				actualRawPath = r.URL.RawPath
 				actualHeader = r.Header.Get(ReplacedPathHeader)
 				requestURI = r.RequestURI
 			})
 
-			handler, err := New(context.Background(), next, replacementConfig, "foo-replace-path")
+			handler, err := New(context.Background(), next, test.config, "foo-replace-path")
 			require.NoError(t, err)
 
-			req := testhelpers.MustNewRequest(http.MethodGet, "http://localhost"+path, nil)
+			server := httptest.NewServer(handler)
+			defer server.Close()
 
-			handler.ServeHTTP(nil, req)
+			resp, err := http.Get(server.URL + test.path)
+			require.NoError(t, err)
+			require.Equal(t, http.StatusOK, resp.StatusCode)
 
-			assert.Equal(t, expectedPath, replacementConfig.Path, "Unexpected path.")
+			assert.Equal(t, test.expectedPath, actualPath, "Unexpected path.")
-			assert.Equal(t, path, actualHeader, "Unexpected '%s' header.", ReplacedPathHeader)
+			assert.Equal(t, test.expectedHeader, actualHeader, "Unexpected '%s' header.", ReplacedPathHeader)
-			assert.Equal(t, expectedPath, requestURI, "Unexpected request URI.")
+
+			if actualRawPath == "" {
+				assert.Equal(t, actualPath, requestURI, "Unexpected request URI.")
+			} else {
+				assert.Equal(t, actualRawPath, requestURI, "Unexpected request URI.")
+			}
 		})
 	}
 }

pkg/middlewares/replacepathregex/replace_path_regex.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"net/url"
 	"regexp"
 	"strings"
 
@@ -49,10 +50,31 @@ func (rp *replacePathRegex) GetTracingInformation() (string, ext.SpanKindEnum) {
 }
 
 func (rp *replacePathRegex) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
-	if rp.regexp != nil && len(rp.replacement) > 0 && rp.regexp.MatchString(req.URL.Path) {
+	var currentPath string
-		req.Header.Add(replacepath.ReplacedPathHeader, req.URL.Path)
+	if req.URL.RawPath == "" {
-		req.URL.Path = rp.regexp.ReplaceAllString(req.URL.Path, rp.replacement)
+		currentPath = req.URL.Path
+	} else {
+		currentPath = req.URL.RawPath
+	}
+
+	if rp.regexp != nil && len(rp.replacement) > 0 && rp.regexp.MatchString(currentPath) {
+		req.Header.Add(replacepath.ReplacedPathHeader, currentPath)
+
+		req.URL.RawPath = rp.regexp.ReplaceAllString(currentPath, rp.replacement)
+
+		// as replacement can introduce escaped characters
+		// Path must remain an unescaped version of RawPath
+		// Doesn't handle multiple times encoded replacement (`/` => `%2F` => `%252F` => ...)
+		var err error
+		req.URL.Path, err = url.PathUnescape(req.URL.RawPath)
+		if err != nil {
+			log.FromContext(middlewares.GetLoggerCtx(context.Background(), rp.name, typeName)).Error(err)
+			http.Error(rw, err.Error(), http.StatusInternalServerError)
+			return
+		}
+
 		req.RequestURI = req.URL.RequestURI()
 	}
+
 	rp.next.ServeHTTP(rw, req)
 }

pkg/middlewares/replacepathregex/replace_path_regex_test.go

@@ -3,11 +3,11 @@ package replacepathregex
 import (
 	"context"
 	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/containous/traefik/v2/pkg/config/dynamic"
 	"github.com/containous/traefik/v2/pkg/middlewares/replacepath"
-	"github.com/containous/traefik/v2/pkg/testhelpers"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -18,6 +18,7 @@ func TestReplacePathRegex(t *testing.T) {
 		path            string
 		config          dynamic.ReplacePathRegex
 		expectedPath    string
+		expectedRawPath string
 		expectedHeader  string
 		expectsError    bool
 	}{
@@ -29,6 +30,7 @@ func TestReplacePathRegex(t *testing.T) {
 				Regex:       `^/whoami/(.*)`,
 			},
 			expectedPath:    "/who-am-i/and/whoami",
+			expectedRawPath: "/who-am-i/and/whoami",
 			expectedHeader:  "/whoami/and/whoami",
 		},
 		{
@@ -39,6 +41,7 @@ func TestReplacePathRegex(t *testing.T) {
 				Regex:       `/whoami`,
 			},
 			expectedPath:    "/who-am-i/and/who-am-i",
+			expectedRawPath: "/who-am-i/and/who-am-i",
 			expectedHeader:  "/whoami/and/whoami",
 		},
 		{
@@ -58,6 +61,7 @@ func TestReplacePathRegex(t *testing.T) {
 				Regex:       `^(?i)/downloads/([^/]+)/([^/]+)$`,
 			},
 			expectedPath:    "/downloads/src-source.go",
+			expectedRawPath: "/downloads/src-source.go",
 			expectedHeader:  "/downloads/src/source.go",
 		},
 		{
@@ -70,13 +74,46 @@ func TestReplacePathRegex(t *testing.T) {
 			expectedPath: "/invalid/regexp/test",
 			expectsError: true,
 		},
+		{
+			desc: "replacement with escaped char",
+			path: "/aaa/bbb",
+			config: dynamic.ReplacePathRegex{
+				Replacement: "/foo%2Fbar",
+				Regex:       `/aaa/bbb`,
+			},
+			expectedPath:    "/foo/bar",
+			expectedRawPath: "/foo%2Fbar",
+			expectedHeader:  "/aaa/bbb",
+		},
+		{
+			desc: "path and regex with escaped char",
+			path: "/aaa%2Fbbb",
+			config: dynamic.ReplacePathRegex{
+				Replacement: "/foo/bar",
+				Regex:       `/aaa%2Fbbb`,
+			},
+			expectedPath:    "/foo/bar",
+			expectedRawPath: "/foo/bar",
+			expectedHeader:  "/aaa%2Fbbb",
+		},
+		{
+			desc: "path with escaped char (no match)",
+			path: "/aaa%2Fbbb",
+			config: dynamic.ReplacePathRegex{
+				Replacement: "/foo/bar",
+				Regex:       `/aaa/bbb`,
+			},
+			expectedPath:    "/aaa/bbb",
+			expectedRawPath: "/aaa%2Fbbb",
+		},
 	}
 
 	for _, test := range testCases {
 		t.Run(test.desc, func(t *testing.T) {
-			var actualPath, actualHeader, requestURI string
+			var actualPath, actualRawPath, actualHeader, requestURI string
 			next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 				actualPath = r.URL.Path
+				actualRawPath = r.URL.RawPath
 				actualHeader = r.Header.Get(replacepath.ReplacedPathHeader)
 				requestURI = r.RequestURI
 			})
@@ -84,20 +121,30 @@ func TestReplacePathRegex(t *testing.T) {
 			handler, err := New(context.Background(), next, test.config, "foo-replace-path-regexp")
 			if test.expectsError {
 				require.Error(t, err)
-			} else {
+				return
+			}
+
 			require.NoError(t, err)
 
-				req := testhelpers.MustNewRequest(http.MethodGet, "http://localhost"+test.path, nil)
+			server := httptest.NewServer(handler)
-				req.RequestURI = test.path
+			defer server.Close()
 
-				handler.ServeHTTP(nil, req)
+			resp, err := http.Get(server.URL + test.path)
+			require.NoError(t, err, "Unexpected error while making test request")
+			require.Equal(t, http.StatusOK, resp.StatusCode)
 
 			assert.Equal(t, test.expectedPath, actualPath, "Unexpected path.")
+			assert.Equal(t, test.expectedRawPath, actualRawPath, "Unexpected raw path.")
+
+			if actualRawPath == "" {
 				assert.Equal(t, actualPath, requestURI, "Unexpected request URI.")
+			} else {
+				assert.Equal(t, actualRawPath, requestURI, "Unexpected request URI.")
+			}
+
 			if test.expectedHeader != "" {
 				assert.Equal(t, test.expectedHeader, actualHeader, "Unexpected '%s' header.", replacepath.ReplacedPathHeader)
 			}
-			}
 		})
 	}
 }