Skip to content

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

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

Closed
c960657 opened this issue May 29, 2017 · 11 comments
Closed

Comments

@c960657
Copy link

c960657 commented May 29, 2017

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 odeke-em changed the title net/http/httputil: Support RFC 7239 Forwarded header in ReverseProxy proposal: net/http/httputil: support RFC 7239 Forwarded header in ReverseProxy May 31, 2017
@gopherbot gopherbot added this to the Proposal milestone May 31, 2017
@odeke-em
Copy link
Member

/cc @bradfitz @dsnet

@dsnet
Copy link
Member

dsnet commented May 31, 2017

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
Copy link
Author

c960657 commented May 31, 2017

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
Copy link
Contributor

rsc commented Jun 5, 2017

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
Copy link
Contributor

rsc commented Jun 12, 2017

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

@c960657
Copy link
Author

c960657 commented Jun 12, 2017

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
Copy link
Contributor

rsc commented Jun 19, 2017

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
Copy link
Contributor

rsc commented Jun 26, 2017

Ping? Should we just postpone this?

@bradfitz
Copy link
Contributor

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

@c960657
Copy link
Author

c960657 commented Jun 27, 2017

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
Copy link
Contributor

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

@golang golang locked and limited conversation to collaborators Jun 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants