Skip to content

Commit

Permalink
fix: URL encoding fixed while forwarding the request to the upstream …
Browse files Browse the repository at this point in the history
…in proxy mode (#716)
  • Loading branch information
dadrus authored Jun 24, 2023
1 parent 353453f commit 9234ea1
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 25 deletions.
29 changes: 16 additions & 13 deletions internal/fiber/middleware/xfmphu/request_url.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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, "/")
}
}

Expand All @@ -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 },
Expand Down
12 changes: 6 additions & 6 deletions internal/handler/proxy/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -326,15 +326,15 @@ 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) {
t.Helper()

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")
Expand Down Expand Up @@ -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
},
Expand All @@ -399,15 +399,15 @@ 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) {
t.Helper()

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")
Expand Down
9 changes: 3 additions & 6 deletions internal/handler/requestcontext/request_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down

0 comments on commit 9234ea1

Please sign in to comment.