fix: avoid unescaping slashes when proxying URLs

Doing all the path operations on URL.Path means that they're being
performed on an unescaped URL. Unfortunately, this URL is *not*
guaranteed to be the directly unescaped version of URL.EscapedPath(); in
particular, slashes will be unescaped in URL.Path, e.g. given:

/abc%2Fdef

URL.Path will be:

/abc/def

In these cases, URL.RawPath is also set with the escaped version of the
path. However, once URL.Path is modified, URL.RawPath is ignored, and
URL.EscapedPath() returns the path-escaped version of URL.Path...which,
in this case, is now /abc/def, not the correct /abc%2Fdef.

As a result, when performing any path modifications during proxying
(i.e. proxying to an upstream URL with a path component, or applying
StripPath), this results in any *escaped* slashes in the proxied URL
being unescaped.

In order to work around this, rewriting operations need to take place on
the *escaped* path, not the unescaped one, and then an intermediate URL
can be used to determine the Path and RawPath values to set on the
forward URL.

This incurs a small breaking change in that StripPath is now applied to
the *escaped* URL, not the unescaped URL. These semantics arguably make
more sense, since StripPath otherwise also cannot distinguish between
unencoded slashes within a path segment vs those that are separating
path segments, but it's still a breaking change nonetheless.

Signed-off-by: Ryan Gonzalez <[email protected]>

proxy/proxy.go

@@ -184,20 +184,46 @@ func ConfigureBackendURL(r *http.Request, rl *rule.Rule) error {
 	}
 
 	proxyHost := r.Host
-	proxyPath := r.URL.Path
+	proxyPath := r.URL.EscapedPath()
 
 	backendHost := p.Host
-	backendPath := p.Path
+	backendPath := p.EscapedPath()
 	backendScheme := p.Scheme
 
+	// We need to build a new path from the incoming *escaped* path components,
+	// because just relying on the already-escaped URL.Path results in escaped
+	// slashes (%2F) being irreversibly converted to actual slashes.
+	//
+	// However, once the new escaped path is built, we can't immediately assign it
+	// to the resulting URL:
+	// - Assigning to URL.Path would mean assigning an escaped path to
+	//   the unescaped path attribute, which would end up double-escaping
+	//   everything.
+	// - Setting RawPath on its own will result in its value being ignored,
+	//   because URL.EscapedPath() (used by URL.String()) will ignore RawPath
+	//   if it does not match an escaped version of Path.
+	// - Directly escaping the path ourselves first to assign it to Path isn't
+	//   possible, because net/url exposes *no* way to escape an entire path,
+	//   only path segments.
+	//
+	// In order to be able to set both URL.Path and URL.RawPath, we re-parse the
+	// escaped path as an intermediate URL. This gives us the correct Path and
+	// RawPath values that can then be set on the forward URL.
+	newEscapedPath := "/" + strings.TrimLeft("/"+strings.Trim(backendPath, "/")+"/"+strings.TrimLeft(proxyPath, "/"), "/")
+	if rl.Upstream.StripPath != "" {
+		newEscapedPath = strings.Replace(newEscapedPath, "/"+strings.Trim(rl.Upstream.StripPath, "/"), "", 1)
+	}
+
+	intermediatePathURL, err := url.Parse(newEscapedPath)
+	if err != nil {
+		return errors.WithStack(err)
+	}
+
 	forwardURL := r.URL
 	forwardURL.Scheme = backendScheme
 	forwardURL.Host = backendHost
-	forwardURL.Path = "/" + strings.TrimLeft("/"+strings.Trim(backendPath, "/")+"/"+strings.TrimLeft(proxyPath, "/"), "/")
-
-	if rl.Upstream.StripPath != "" {
-		forwardURL.Path = strings.Replace(forwardURL.Path, "/"+strings.Trim(rl.Upstream.StripPath, "/"), "", 1)
-	}
+	forwardURL.RawPath = intermediatePathURL.RawPath
+	forwardURL.Path = intermediatePathURL.Path
 
 	r.Host = backendHost
 	if rl.Upstream.PreserveHost {

proxy/proxy_test.go

@@ -501,6 +501,12 @@ func TestConfigureBackendURL(t *testing.T) {
 			eURL:  "http://localhost:4000/foo/users/1234",
 			eHost: "localhost:3000",
 		},
+		{
+			r:     &http.Request{Host: "localhost:3000", URL: &url.URL{RawPath: "/api/users/12%2F34", Path: "/api/users/12/34", Scheme: "http"}},
+			rl:    &rule.Rule{Upstream: rule.Upstream{URL: "http://localhost:4000/foo/", PreserveHost: true, StripPath: "api"}},
+			eURL:  "http://localhost:4000/foo/users/12%2F34",
+			eHost: "localhost:3000",
+		},
 	} {
 		t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
 			require.NoError(t, proxy.ConfigureBackendURL(tc.r, tc.rl))