From f67619d3e004e64a178d21bbef83bc754fa76638 Mon Sep 17 00:00:00 2001 From: Daniel Kumor Date: Fri, 3 Jan 2020 20:33:53 -0500 Subject: [PATCH 1/2] 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 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 --- src/net/http/httputil/reverseproxy.go | 23 +++++++++++++++++- src/net/http/httputil/reverseproxy_test.go | 27 ++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go index e8f7df29a14d41..626a3f4fd3e24e 100644 --- a/src/net/http/httputil/reverseproxy.go +++ b/src/net/http/httputil/reverseproxy.go @@ -97,6 +97,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", @@ -109,7 +130,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 { diff --git a/src/net/http/httputil/reverseproxy_test.go b/src/net/http/httputil/reverseproxy_test.go index f58e08867f6dfd..53c010cbc8685a 100644 --- a/src/net/http/httputil/reverseproxy_test.go +++ b/src/net/http/httputil/reverseproxy_test.go @@ -1210,3 +1210,30 @@ func TestSingleJoinSlash(t *testing.T) { } } } + +func TestJoinURLPath(t *testing.T) { + tests := []struct { + a *url.URL + b *url.URL + path string + rawpath 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.path || rp != tt.rawpath { + t.Errorf("joinURLPath(URL(%s,%s),URL(%s,%s)) want (%s,%s) got (%s,%s)", + tt.a.Path, tt.a.RawPath, + tt.b.Path, tt.b.RawPath, + tt.path, tt.rawpath, + p, rp) + } + } +} From 7be6b8d421c63928639f499b984a821585992c2b Mon Sep 17 00:00:00 2001 From: Daniel Kumor Date: Mon, 6 Jan 2020 21:10:55 -0500 Subject: [PATCH 2/2] Added changes suggested by code review. The comment on using %q instead of %s in formatting strings in errors is also applicable to the output of TestSingleJoinSlash, so I took the liberty of fixing that also. --- src/net/http/httputil/reverseproxy_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/net/http/httputil/reverseproxy_test.go b/src/net/http/httputil/reverseproxy_test.go index 53c010cbc8685a..ce1d16fbc4e4cf 100644 --- a/src/net/http/httputil/reverseproxy_test.go +++ b/src/net/http/httputil/reverseproxy_test.go @@ -1202,7 +1202,7 @@ 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, @@ -1213,10 +1213,10 @@ func TestSingleJoinSlash(t *testing.T) { func TestJoinURLPath(t *testing.T) { tests := []struct { - a *url.URL - b *url.URL - path string - rawpath string + 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"}, @@ -1228,11 +1228,11 @@ func TestJoinURLPath(t *testing.T) { for _, tt := range tests { p, rp := joinURLPath(tt.a, tt.b) - if p != tt.path || rp != tt.rawpath { - t.Errorf("joinURLPath(URL(%s,%s),URL(%s,%s)) want (%s,%s) got (%s,%s)", + 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.path, tt.rawpath, + tt.wantPath, tt.wantRaw, p, rp) } }