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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 [Bug]: Middleware proxy.Do overwrote the wrong scheme #2001

Closed
3 tasks done
bofeng opened this issue Aug 7, 2022 · 1 comment · Fixed by #2004
Closed
3 tasks done

🐛 [Bug]: Middleware proxy.Do overwrote the wrong scheme #2001

bofeng opened this issue Aug 7, 2022 · 1 comment · Fixed by #2004

Comments

@bofeng
Copy link

bofeng commented Aug 7, 2022

Bug Description

I am making a simple proxy that you can append the proxied url after /, for example, you can request http://localhost:3333/http://ip-api.com/json/1.1.1.1 , which will proxy request to http://ip-api.com/json/1.1.1.1.

Here is my code:

package main

import (
	"strings"

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

func main() {
	app := fiber.New()
	app.Get("/*", func(c *fiber.Ctx) error {
		path := c.OriginalURL()
		url := strings.TrimPrefix(path, "/")
		if err := proxy.Do(c, url); err != nil {
			return err
		}
		c.Response().Header.Del(fiber.HeaderServer)
		return nil
	})
	app.Listen(":3333")
}

When I request the url like: http://localhost:3333/http://ip-api.com/json/1.1.1.1 , it will show this error:

unsupported protocol "ttp". http and https are supported

By debugging and following the code, the scheme has been overwritten to "ttp" at this line: https://github.com/gofiber/fiber/blob/master/middleware/proxy/proxy.go#L131 , since at some point the addr variable has changed to "ttp://xxxxx" (without the leading "h"), the parsed scheme changed to "ttp". It seems related to the memory allocation, the problem will be gone, if in my code I either:

  • change the fiber config to use Immutable: true, or
  • change path := c.OriginalURL() to path := utils.CopyString(c.OriginalURL()) .

But since the code above doesn't violate fiber's rule:

As a rule of thumb, you must only use context values within the handler, and you must not keep any references.

Even if I don't use Immutable: true or utils.CopyString, the problem shouldn't be there.

Also, this bug doesn't exist when I was using version v2.10.0, which doesn't have these 3 lines: https://github.com/gofiber/fiber/blob/master/middleware/proxy/proxy.go#L130

How to Reproduce

Copy the above code and run it with fiber v2.36.0, then visit: http://localhost:3333/http://ip-api.com/json/1.1.1.1 .
You should be able to see the error:

unsupported protocol "ttp". http and https are supported

I can reproduce this in my 3 laptops: Macbook m1, Mackbook m2 and Macbook Intel.

Expected Behavior

The error shouldn't be there (the scheme shouldn't be incorrectly overwritten) even without "Immutable: true" config or CopyString

Fiber Version

v2.36.0

Code Snippet (optional)

See above

Checklist:

  • I agree to follow Fiber's Code of Conduct.
  • I have checked for existing issues that describe my problem prior to opening this one.
  • I understand that improperly formatted bug reports may be closed without explanation.
@ReneWerner87
Copy link
Member

thanks we will look at the problem, unfortunately everyone is pretty busy right now with version 3 and other old content

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.

2 participants