From 9234ea168cf94b32e4a4d5e1486ec6552d9b7d37 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Sat, 24 Jun 2023 19:22:09 +0200 Subject: [PATCH] fix: URL encoding fixed while forwarding the request to the upstream in proxy mode (#716) --- .../fiber/middleware/xfmphu/request_url.go | 29 ++++++++++--------- internal/handler/proxy/handler_test.go | 12 ++++---- .../handler/requestcontext/request_context.go | 9 ++---- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/internal/fiber/middleware/xfmphu/request_url.go b/internal/fiber/middleware/xfmphu/request_url.go index 6e1928151..67867c874 100644 --- a/internal/fiber/middleware/xfmphu/request_url.go +++ b/internal/fiber/middleware/xfmphu/request_url.go @@ -29,10 +29,11 @@ import ( func requestURL(c *fiber.Ctx) *url.URL { var ( - proto string - host string - path string - query string + proto string + host string + rawPath string + path string + query string ) if c.IsProxyTrusted() { @@ -41,25 +42,25 @@ func requestURL(c *fiber.Ctx) *url.URL { forwardedURI, _ := url.Parse(forwardedURIVal) proto = forwardedURI.Scheme host = forwardedURI.Host - path = forwardedURI.Path + rawPath = forwardedURI.Path query = forwardedURI.Query().Encode() } } - if len(path) == 0 && c.IsProxyTrusted() { - path = c.Get(xForwardedPath) + if len(rawPath) == 0 && c.IsProxyTrusted() { + rawPath = c.Get(xForwardedPath) } - if len(path) == 0 { - path = c.Params("*") - if len(path) != 0 { - path = fmt.Sprintf("/%s", path) + if len(rawPath) == 0 { + rawPath = c.Params("*") + if len(rawPath) != 0 { + rawPath = fmt.Sprintf("/%s", rawPath) } // there is a bug in the implementation of the nginx controller // see: https://github.com/kubernetes/ingress-nginx/issues/10114 - if c.Get(xSentFrom) == nginxIngressAgent && strings.HasPrefix(path, "//") { - path = strings.TrimPrefix(path, "/") + if c.Get(xSentFrom) == nginxIngressAgent && strings.HasPrefix(rawPath, "//") { + rawPath = strings.TrimPrefix(rawPath, "/") } } @@ -68,6 +69,8 @@ func requestURL(c *fiber.Ctx) *url.URL { query = stringx.ToString(origReqURL.QueryString()) } + path, _ = url.PathUnescape(rawPath) + return &url.URL{ Scheme: x.IfThenElseExec(len(proto) != 0, func() string { return proto }, diff --git a/internal/handler/proxy/handler_test.go b/internal/handler/proxy/handler_test.go index 2b603c496..e13dfd621 100644 --- a/internal/handler/proxy/handler_test.go +++ b/internal/handler/proxy/handler_test.go @@ -306,7 +306,7 @@ func TestHandleProxyEndpointRequest(t *testing.T) { req := httptest.NewRequest( http.MethodPost, - "http://heimdall.test.local/foobar", + "http://heimdall.test.local/%5Bid%5D/foobar", strings.NewReader("hello")) req.Header.Set("Content-Type", "text/html") @@ -326,7 +326,7 @@ func TestHandleProxyEndpointRequest(t *testing.T) { })).Return(upstreamURL, nil) repository.EXPECT().FindRule(mock.MatchedBy(func(reqURL *url.URL) bool { - return reqURL.String() == "http://heimdall.test.local/foobar" + return reqURL.String() == "http://heimdall.test.local/%5Bid%5D/foobar" })).Return(rule, nil) }, instructUpstream: func(t *testing.T) { @@ -334,7 +334,7 @@ func TestHandleProxyEndpointRequest(t *testing.T) { upstreamCheckRequest = func(req *http.Request) { assert.Equal(t, http.MethodGet, req.Method) - assert.Equal(t, "/foobar", req.URL.Path) + assert.Equal(t, "/[id]/foobar", req.URL.Path) assert.Equal(t, "baz", req.Header.Get("X-Foo-Bar")) cookie, err := req.Cookie("X-Bar-Foo") @@ -383,7 +383,7 @@ func TestHandleProxyEndpointRequest(t *testing.T) { strings.NewReader("hello")) req.Header.Set("Content-Type", "text/html") - req.Header.Set("X-Forwarded-Path", "/barfoo") + req.Header.Set("X-Forwarded-Path", "/%5Bbarfoo%5D") return req }, @@ -399,7 +399,7 @@ func TestHandleProxyEndpointRequest(t *testing.T) { })).Return(upstreamURL, nil) repository.EXPECT().FindRule(mock.MatchedBy(func(reqURL *url.URL) bool { - return reqURL.String() == "http://heimdall.test.local/barfoo" + return reqURL.String() == "http://heimdall.test.local/%5Bbarfoo%5D" })).Return(rule, nil) }, instructUpstream: func(t *testing.T) { @@ -407,7 +407,7 @@ func TestHandleProxyEndpointRequest(t *testing.T) { upstreamCheckRequest = func(req *http.Request) { assert.Equal(t, http.MethodPost, req.Method) - assert.Equal(t, "/barfoo", req.URL.Path) + assert.Equal(t, "/[barfoo]", req.URL.Path) assert.Equal(t, "baz", req.Header.Get("X-Foo-Bar")) cookie, err := req.Cookie("X-Bar-Foo") diff --git a/internal/handler/requestcontext/request_context.go b/internal/handler/requestcontext/request_context.go index 69b4a6f1e..92922788c 100644 --- a/internal/handler/requestcontext/request_context.go +++ b/internal/handler/requestcontext/request_context.go @@ -120,12 +120,9 @@ func (s *RequestContext) FinalizeAndForward(upstreamURL *url.URL, timeout time.D s.c.Request().Header.Del(name) } - URL := &url.URL{ - Scheme: upstreamURL.Scheme, - Host: upstreamURL.Host, - Path: s.reqURL.Path, - RawQuery: s.reqURL.RawQuery, - } + URL := *s.reqURL + URL.Scheme = upstreamURL.Scheme + URL.Host = upstreamURL.Host s.c.Request().Header.SetMethod(s.reqMethod) s.c.Request().SetRequestURI(URL.String())