Use X-Forwared-Host for resolving host

[frankbille]

X-Forwarded-For is normally used for getting the original client IP, whereas X-Forwarded-Host is normally used to get the original host requested by the client.

X-Forwarded-For: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For
X-Forwarded-Host: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host

[tboerger]

Even if that is correct this can be a breaking change for various installations.

[tboerger]

And beside that, this the "for" header works totally fine with reverse proxies like traefik, caddy or haproxy.

[frankbille]

It was actually because of traefik I found this. The latest traefik version sets X-Forwarded-For to an IP address, and X-Forwarded-Host to the original requested hostname.

I understand the point about backward compatibility. Maybe if both were supported?

[tboerger]

Maybe it should be better to check for host and as a fallback check the for header.

The traefik change have been integrated within the last release?

[frankbille]

I am fairly new to Traefik, but I think they have been using Oxy for a long time (It is the library handling the proxying). And Oxy has been using X-Forwarded-For for client ip for at least 3 years. So this is how traefik has been handling this always.

I can't think of a good way to support both headers for the same thing other than configuration. If we handle both, then it has to be either:

1. X-Forwarded-Host
2. X-Forwarded-For

or

1. X-Forwarded-For
2. X-Forwarded-Host

But since both fields will most likely be filled out in most cases, you would always take the first one in both cases.

[tboerger]

I'm using traefik already quite long and it always worked fine for me in combination with this middleware

[frankbille]

That is quite odd. Have you checked that the X-Forwarded-For header is set? And it is an actual hostname and not an IP?
I'm also reading up on this now again, and I can't find any that uses that header for anything but client ip.

[tboerger]

There is an pass host Header Option I'm using with traefik

[frankbille]

But that passes the "Host" header, and according to the resolveHost algorithm it will never get to that if the X-Forwarded-For header is set.

The algorithm is:

1. X-Forwarded-For header (I think this is incorrect, as it should be X-Forwarded-Host)
2. X-Host header (a fine use of older header)
3. request.Host (This uses host from request url and if not present uses the "Host" header)
4. Host from request url (repetition from third step and thus superfluous)
5. Default host from config

So in your case with traefik you have to somehow suppress the passing of X-Forwarded-For or do something else instead. Have you tried to debug what headers you get from the traefik request?

[tboerger]

I will prepare a docker-compose scenario to see the exact behavior

[jdolitsky]

Can we instead add a field to the Config struct, such as ForwardingHeader, that is used for this? If not provided, just defaults to "X-Forwarded-For" so as not to break anyones setup?

location.go:

...
	// ForwardingHeader is the expected header for identifying the original host
	// requested by the client in the Host HTTP request header
	ForwardingHeader string
}

config.go:

func newLocation(config Config) *location {
	forwardingHeader := config.ForwardingHeader
	if forwardingHeader == "" {
		forwardingHeader = "X-Forwarded-For"
	}
	return &location{
		scheme: config.Scheme,
		host:   config.Host,
		base:   config.Base,
		headers: headers{
			scheme: "X-Forwarded-Proto",
			host:   forwardingHeader,
		},
	}
}

cc @hho

[kinnou02]

What have to be done to make this PR go forward?
The solution proposed by @jdolitsky seems to solve everyone problems.
I can write a bit of documentation if it is the last thing needed.

[eric]

I just ran into this with traefik as well and was very confused by what I was seeing.

[ihipop]

WHY THIS IS NOT MERGED?
we can use incompatible version change via go mod

[TJM]

I agree, bump the major version and get this released, but for anyone else who is using this module and needs a quick workaround:

	// location - detects the scheme (http/https) and hostname of the server via the http.Request
	locConfig := location.DefaultConfig()
	locConfig.Headers.Host = "X-Forwarded-Host" // Workaround BUG in location middleware
	router.Use(location.New(locConfig))

[neumantm]

Agree. Just ran into the same thing with kubernetes ingress-nginx.

In my opinion using X-Forwaeded-For is a plain bug.
I can see no scenario where this is/was really what a developer wants (except if the reverse proxy has also a matching bug, but then that should also be addressed there.)
But if you want to be really safe to not break existing things: Bump the major version