From 2e8733b1523449fba4a3ff8754be428f5512c1af Mon Sep 17 00:00:00 2001 From: Alexey Nesterov Date: Mon, 6 Dec 2021 14:50:34 +0000 Subject: [PATCH] Correctly handle URL escaped paths Consider proxy configuration `*,/test/(.*),https://dest/$1`. When reproxy accepts a request with URL encoded path, i.e. '/test/a%205%25%20b/' which the encoded form of '/test/a 5% b', it is using request.URL.Path which is already URL decoded by Golang. This causes an error in proxy.go while it is trying to validate the destination with `url.Parse(match.Destination)` as, strictly speaking, destination URL is not a valid URL anymore, it is `https://target-dest/a 5% b`. With this fix, the original escaped URL stays as is, correctly passes the validation and then it is up to destination server to URL decode and correctly handle the URL. --- app/proxy/proxy.go | 2 +- app/proxy/proxy_test.go | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/proxy/proxy.go b/app/proxy/proxy.go index a69a76c..b82032d 100644 --- a/app/proxy/proxy.go +++ b/app/proxy/proxy.go @@ -301,7 +301,7 @@ func (h *Http) matchHandler(next http.Handler) http.Handler { if server == "" { server = strings.Split(r.Host, ":")[0] // drop port } - matches := h.Match(server, r.URL.Path) // get all matches for the server:path pair + matches := h.Match(server, r.URL.EscapedPath()) // get all matches for the server:path pair match, ok := getMatch(matches, h.LBSelector) if ok { ctx := context.WithValue(r.Context(), ctxMatch, match) // set match info diff --git a/app/proxy/proxy_test.go b/app/proxy/proxy_test.go index 890518d..7b53999 100644 --- a/app/proxy/proxy_test.go +++ b/app/proxy/proxy_test.go @@ -101,6 +101,20 @@ func TestHttp_Do(t *testing.T) { assert.Contains(t, string(b), "Sorry for the inconvenience") assert.Equal(t, "text/html; charset=utf-8", resp.Header.Get("Content-Type")) } + + { + resp, err := client.Get("http://localhost:" + strconv.Itoa(port) + "/api/test%20%25%20and%20&,%20and%20other%20characters%20@%28%29%5E%21") + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode) + t.Logf("%+v", resp.Header) + + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + assert.Equal(t, "response /123/test%20%25%20and%20&,%20and%20other%20characters%20@%28%29%5E%21", string(body)) + assert.Equal(t, "reproxy", resp.Header.Get("App-Name")) + assert.Equal(t, "v1", resp.Header.Get("h1")) + } } func TestHttp_DoWithAssets(t *testing.T) {