net/http/httputil: handle escaped paths in SingleHostReverseProxy

When forwarding a request, a SingleHostReverseProxy appends the
request's path to the target URL's path. However, if certain path
elements are encoded, (such as %2F for slash in either the request or
target path), simply joining the URL.Path elements is not sufficient,
since the field holds the decoded path.

Since 87a605, the RawPath field was added which holds a decoding
hint for the URL. When joining URL paths, this decoding hint needs
to be taken into consideration.

As an example, if the target URL.Path is /a/b, and URL.RawPath
is /a%2Fb, joining the path with /c should result in /a/b/c
in URL.Path, and /a%2Fb/c in RawPath.

The added joinURLPath function combines the two URL's Paths,
while taking into account escaping, and replaces the previously used
singleJoiningSlash in NewSingleHostReverseProxy.

Fixes #35908

Change-Id: I45886aee548431fe4031883ab1629a41e35f1727
GitHub-Last-Rev: 7be6b8d
GitHub-Pull-Request: #36378
Reviewed-on: https://go-review.googlesource.com/c/go/+/213257
Run-TryBot: Ian Lance Taylor <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>

Author: dkumor

src/net/http/httputil/reverseproxy.go

@@ -110,6 +110,27 @@ func singleJoiningSlash(a, b string) string {
 	return a + b
 }
 
+func joinURLPath(a, b *url.URL) (path, rawpath string) {
+	if a.RawPath == "" && b.RawPath == "" {
+		return singleJoiningSlash(a.Path, b.Path), ""
+	}
+	// Same as singleJoiningSlash, but uses EscapedPath to determine
+	// whether a slash should be added
+	apath := a.EscapedPath()
+	bpath := b.EscapedPath()
+
+	aslash := strings.HasSuffix(apath, "/")
+	bslash := strings.HasPrefix(bpath, "/")
+
+	switch {
+	case aslash && bslash:
+		return a.Path + b.Path[1:], apath + bpath[1:]
+	case !aslash && !bslash:
+		return a.Path + "/" + b.Path, apath + "/" + bpath
+	}
+	return a.Path + b.Path, apath + bpath
+}
+
 // NewSingleHostReverseProxy returns a new ReverseProxy that routes
 // URLs to the scheme, host, and base path provided in target. If the
 // target's path is "/base" and the incoming request was for "/dir",
@@ -122,7 +143,7 @@ func NewSingleHostReverseProxy(target *url.URL) *ReverseProxy {
 	director := func(req *http.Request) {
 		req.URL.Scheme = target.Scheme
 		req.URL.Host = target.Host
-		req.URL.Path = singleJoiningSlash(target.Path, req.URL.Path)
+		req.URL.Path, req.URL.RawPath = joinURLPath(target, req.URL)
 		if targetQuery == "" || req.URL.RawQuery == "" {
 			req.URL.RawQuery = targetQuery + req.URL.RawQuery
 		} else {

src/net/http/httputil/reverseproxy_test.go

@@ -1357,11 +1357,38 @@ func TestSingleJoinSlash(t *testing.T) {
 	}
 	for _, tt := range tests {
 		if got := singleJoiningSlash(tt.slasha, tt.slashb); got != tt.expected {
-			t.Errorf("singleJoiningSlash(%s,%s) want %s got %s",
+			t.Errorf("singleJoiningSlash(%q,%q) want %q got %q",
 				tt.slasha,
 				tt.slashb,
 				tt.expected,
 				got)
 		}
 	}
 }
+
+func TestJoinURLPath(t *testing.T) {
+	tests := []struct {
+		a        *url.URL
+		b        *url.URL
+		wantPath string
+		wantRaw  string
+	}{
+		{&url.URL{Path: "/a/b"}, &url.URL{Path: "/c"}, "/a/b/c", ""},
+		{&url.URL{Path: "/a/b", RawPath: "badpath"}, &url.URL{Path: "c"}, "/a/b/c", "/a/b/c"},
+		{&url.URL{Path: "/a/b", RawPath: "/a%2Fb"}, &url.URL{Path: "/c"}, "/a/b/c", "/a%2Fb/c"},
+		{&url.URL{Path: "/a/b", RawPath: "/a%2Fb"}, &url.URL{Path: "/c"}, "/a/b/c", "/a%2Fb/c"},
+		{&url.URL{Path: "/a/b/", RawPath: "/a%2Fb%2F"}, &url.URL{Path: "c"}, "/a/b//c", "/a%2Fb%2F/c"},
+		{&url.URL{Path: "/a/b/", RawPath: "/a%2Fb/"}, &url.URL{Path: "/c/d", RawPath: "/c%2Fd"}, "/a/b/c/d", "/a%2Fb/c%2Fd"},
+	}
+
+	for _, tt := range tests {
+		p, rp := joinURLPath(tt.a, tt.b)
+		if p != tt.wantPath || rp != tt.wantRaw {
+			t.Errorf("joinURLPath(URL(%q,%q),URL(%q,%q)) want (%q,%q) got (%q,%q)",
+				tt.a.Path, tt.a.RawPath,
+				tt.b.Path, tt.b.RawPath,
+				tt.wantPath, tt.wantRaw,
+				p, rp)
+		}
+	}
+}