Skip to content

Commit

Permalink
Restore Original URL of the context after changing it (#1788)
Browse files Browse the repository at this point in the history
* Restore original URL after the proxy

* Use the Immutable string to restore

* Changing deprecated ImmutableString to CopyString

Co-authored-by: Geet Manghnani <gmanghna@in.ibm.com>
  • Loading branch information
geet and gmanghna committed Feb 20, 2022
1 parent c5f11cc commit a746e5b
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 4 deletions.
2 changes: 1 addition & 1 deletion ctx.go
Expand Up @@ -1301,7 +1301,7 @@ func (c *Ctx) SendFile(file string, compress ...bool) error {
}
}
// Restore the original requested URL
originalURL := c.OriginalURL()
originalURL := utils.CopyString(c.OriginalURL())
defer c.fasthttp.Request.SetRequestURI(originalURL)
// Set new URI for fileHandler
c.fasthttp.Request.SetRequestURI(file)
Expand Down
10 changes: 7 additions & 3 deletions ctx_test.go
Expand Up @@ -1856,14 +1856,18 @@ func Test_Ctx_SendFile_RestoreOriginalURL(t *testing.T) {
t.Parallel()
app := New()
app.Get("/", func(c *Ctx) error {
originalURL := c.OriginalURL()
originalURL := utils.CopyString(c.OriginalURL())
err := c.SendFile("ctx.go")
utils.AssertEqual(t, originalURL, c.OriginalURL())
return err
})

_, err := app.Test(httptest.NewRequest("GET", "/?test=true", nil))
utils.AssertEqual(t, nil, err)
_, err1 := app.Test(httptest.NewRequest("GET", "/?test=true", nil))
// second request required to confirm with zero allocation
_, err2 := app.Test(httptest.NewRequest("GET", "/?test=true", nil))

utils.AssertEqual(t, nil, err1)
utils.AssertEqual(t, nil, err2)
}

// go test -run Test_Ctx_JSON
Expand Down
2 changes: 2 additions & 0 deletions middleware/proxy/proxy.go
Expand Up @@ -121,6 +121,8 @@ func Forward(addr string) fiber.Handler {
func Do(c *fiber.Ctx, addr string) error {
req := c.Request()
res := c.Response()
originalURL := utils.CopyString(c.OriginalURL())
defer req.SetRequestURI(originalURL)
req.SetRequestURI(addr)
// NOTE: if req.isTLS is true, SetRequestURI keeps the scheme as https.
// issue reference:
Expand Down
23 changes: 23 additions & 0 deletions middleware/proxy/proxy_test.go
Expand Up @@ -339,3 +339,26 @@ func Test_Proxy_Buffer_Size_Response(t *testing.T) {
utils.AssertEqual(t, nil, err)
utils.AssertEqual(t, fiber.StatusOK, resp.StatusCode)
}

// go test -race -run Test_Proxy_Do_RestoreOriginalURL
func Test_Proxy_Do_RestoreOriginalURL(t *testing.T) {
t.Parallel()
app := fiber.New()
app.Get("/proxy", func(c *fiber.Ctx) error {
return c.SendString("ok")
})
app.Get("/test", func(c *fiber.Ctx) error {
originalURL := utils.CopyString(c.OriginalURL())
if err := Do(c, "/proxy"); err != nil {
return err
}
utils.AssertEqual(t, originalURL, c.OriginalURL())
return c.SendString("ok")
})
_, err1 := app.Test(httptest.NewRequest("GET", "/test", nil))
// This test requires multiple requests due to zero allocation used in fiber
_, err2 := app.Test(httptest.NewRequest("GET", "/test", nil))

utils.AssertEqual(t, nil, err1)
utils.AssertEqual(t, nil, err2)
}

0 comments on commit a746e5b

Please sign in to comment.