Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 proxy.Do is changing the originalURL() and not restoring it to the originalURL of the handler #1786

Closed
geet opened this issue Feb 19, 2022 · 2 comments 路 Fixed by #1788

Comments

@geet
Copy link
Contributor

geet commented Feb 19, 2022

Fiber version
github.com/gofiber/fiber/v2 v2.27.0
Issue description
I am invoking proxy.Do(c,proxy_url) inside a fiber handler. Inside the handler,

c.OriginalURL() 

returns a non-immutable string due to zero allocation in fiber.
Now, when i pass the fiber context to the proxy.Do(ctx,addr), the originalURL is modified due to the setRequestURI inside proxy.go
Here is the code snippet showing this.
Code snippet

package main

import (
	"fmt"

	"github.com/gofiber/fiber/v2"
	"github.com/gofiber/fiber/v2/middleware/proxy"
)

func main() {

	app := fiber.New()
	app.Get("/test", func(c *fiber.Ctx) error {
		originalUrl := c.OriginalURL()
		fmt.Println(originalUrl) // '/test'
		if err := proxy.Do(c, "http://www.google.com/search"); err != nil {
			return err
		}
                 fmt.Println(c.OriginalURL()) // `/search'
		fmt.Println("And here: ", originalUrl) // '/sear'
		return c.SendString("tested")
	})
	app.Listen(":3002")
}

From the browser try hitting localhost:3002/test two times. In the first occurrence, the results are expected due to zero allocation implementation of c.OriginalURL() method in fiber.

The above problem can be resolved by using the immutable method provided by the fiber utils to persist values.

	originalURL := utils.ImmutableString(c.OriginalURL())

But c.OriginalURL() will still point to the URL of the proxy and not to the original handler.

here is the snippet.

package main

import (
	"fmt"

	"github.com/gofiber/fiber/v2"
	"github.com/gofiber/fiber/v2/middleware/proxy"
	"github.com/gofiber/utils"
)

func main() {

	app := fiber.New()
	app.Get("/test", func(c *fiber.Ctx) error {
		//originalUrl := c.OriginalURL()
		originalUrl := utils.ImmutableString(c.OriginalURL())
		fmt.Println(originalUrl) // '/test'
		if err := proxy.Do(c, "http://www.google.com/search"); err != nil {
			return err
		}
		fmt.Println(c.OriginalURL()) // `/search`
		fmt.Println("And here: ", originalUrl) // '/sear'
		return c.SendString("tested")
	})
	app.Listen(":3002")
}

Shouldn't the proxy.Do() method restore the context after hitting the proxy URL since the doc says that this method can be used in a. handler ?

@welcome
Copy link

welcome bot commented Feb 19, 2022

Thanks for opening your first issue here! 馃帀 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

@geet
Copy link
Contributor Author

geet commented Feb 20, 2022

There was a similar issue raised and a fix was provided. However, I find that the fix provided won't work and the test is missing the essential edge case.
I think the methods proxy.Do() and sendFile() should restore the orginalURL() to avoid side effects which maybe caused in the original Handler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants