golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.86k stars 17.65k forks source link

proposal: net/http/httputil: support RFC 7239 Forwarded header in ReverseProxy #20526

Closed c960657 closed 7 years ago

c960657 commented 7 years ago

ReverseProxy injects the de facto standard header X-Forwarded-For.

RFC 7239 defines a new header, Forwarded, that offers the same functionality as X-Forwarded-For and more.

I suggest that ReverseProxy get support for Forwarded: for=... in addition to X-Forwarded-For.

odeke-em commented 7 years ago

/cc @bradfitz @dsnet

dsnet commented 7 years ago

The suggestion seems fine, but @bradfitz has the final say. A consideration is whether the addition of a "Forwarded" header is worth the extra bytes we dump out on the network. The internet at large already understands "X-Forwarded-For". It will be a long time (if ever) before "Forwarded" replaces "X-Forwarded-For", so people will be forced to implement understanding both. It doesn't seem like the utility is that great.

c960657 commented 7 years ago

Adoption of the Forwarded header does seem to suffer from an chicken-and-egg problem.

One reason why Forwarded is increasingly useful is that it also allows forwarding the original hostname and protocol used by the client. The hostname is already available in the Host header, but the protocol is not. With all the rage about HTTPS these days, one has to resort to other non-standard headers such as X-Forwarded-Proto, so one could imagine that this would generate an increasing demand for a standardized header. However, this is just speculation.

rsc commented 7 years ago

What software does recognize Forwarded today?

There's a question of recognizing the new header vs generating the new header. Recognizing both seems OK.

There's no point in generating a header that nothing else recognizes though. Should we really generate both?

Should we translate X-Forwarded-For into Forwarded?

Should we translate Forwarded into X-Forwarded-For?

rsc commented 7 years ago

Still waiting for answers to the above. Ping @c960657 ?

c960657 commented 7 years ago

The Forwarded header is supported by the Symfony framework and thus also by other projects based on that including Drupal and Laravel.

It is also supported by the Gorilla web toolkit and forwarded-http Node library.

The header is not supported by Apache mod_proxy_http, Ruby on Rails, WordPress or MediaWiki.

All of the above support X-Forwarded-For and to some extend the other X-Forwarded-xxx headers. So no, the Forwarded header does not have much traction, and currently offers no advantages over X-Forwarded-For.

rsc commented 7 years ago

If we just don't do this, and we wait until there's a more compelling need, what bad things would happen? Is there some minimal amount we should do today, or should we just postpone everything until later?

rsc commented 7 years ago

Ping? Should we just postpone this?

bradfitz commented 7 years ago

Sounds like there's no compelling reason to do this. We can reconsider in the future if things change.

c960657 commented 7 years ago

If we want to do something, but our main concern is not adding more bytes to our request payload, we could parse incoming Forwarded headers in requests with no X-Forwarded-For header and add the IP addresses in for=… to the X-Forwarded-For. This is similar to the transition mechanism described in section 7.4 of the RFC, just in the other direction (the RFC suggests copying from X-Forwarded-For to Forwarded).

This would allow downstream proxies to support only Forwarded, while downstream proxies and the HTTP server can still use X-Forwarded-For. This is probably not a big win, so I agree that at the moment, there is no compelling need to add support for RFC 7239.

So I don’t object to closing this issue :-)

bradfitz commented 7 years ago

I'd be fine with recognizing Forwarded if X-Forwarded-For was absent. Feel free to send a change for Go 1.10.