From 2d323f900df420a29de29cbab949eea08e3d1a61 Mon Sep 17 00:00:00 2001 From: Daniel Kumor Date: Tue, 7 Jan 2020 02:16:40 +0000 Subject: [PATCH] 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: 7be6b8d421c63928639f499b984a821585992c2b GitHub-Pull-Request: golang/go#36378 Reviewed-on: https://go-review.googlesource.com/c/go/+/213257 Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/net/http/httputil/reverseproxy.go | 23 ++++++++++++++++- src/net/http/httputil/reverseproxy_test.go | 29 +++++++++++++++++++++- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go index 6e5bc4753e..70de7b107d 100644 --- a/src/net/http/httputil/reverseproxy.go +++ b/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 { diff --git a/src/net/http/httputil/reverseproxy_test.go b/src/net/http/httputil/reverseproxy_test.go index be5531951a..6a3a1c54fc 100644 --- a/src/net/http/httputil/reverseproxy_test.go +++ b/src/net/http/httputil/reverseproxy_test.go @@ -1357,7 +1357,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, @@ -1365,3 +1365,30 @@ func TestSingleJoinSlash(t *testing.T) { } } } + +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) + } + } +}