Support RFC 7239: HTTP Forwarded header#1017
Conversation
[RFC 7239][] is an IETF proposed standard for a HTTP `Forwarded` request
header that aims to replace non-standard `X-Forwarded-*` headers.
This commit supports all relevant tokens that may be set in the
Forwarded header:
- proto
- host
- for
I have prioritised the Forwarded header over the non-standard headers
as I think it's preferable to use the standard header when specified.
Multiple values for each token are allowed by the standard. When
multiple values are supplied, I've followed these rules:
- for the `proto` token, use the first value specified as it's most
likely to be the protocol used by the original client
- for the `host` token, use the last value specified following the
convention already used by Rack for multiple `X-Forwarded-For`
headers
When both `X-Forwarded-For` and `Forwarded` headers are supplied and the
latter includes `for` values, I've concatenated both sets of values with
the `Forwarded` values coming last.
I've been careful to support mixed case in the header and optional
double quotes encapsulating the token value as specified in the RFC.
[RFC 7239]: https://tools.ietf.org/html/rfc7239
|
Is this a useful addition? I'd be really grateful for any feedback. 😎 |
|
|
||
| msg = FORMAT % [ | ||
| env['HTTP_X_FORWARDED_FOR'] || env["REMOTE_ADDR"] || "-", | ||
| forwarded_for || env['HTTP_X_FORWARDED_FOR'] || env["REMOTE_ADDR"] || "-", |
There was a problem hiding this comment.
Seems this is kinda-implementing Rack::Request#ip, maybe nicer to reuse that.
|
I think this will need to be made configurable now. The problem here is that proxies that have yet to be updated will not strip these headers, and also will not know to overwrite them. A failure to protect against outside spoofing can have important security implications. Users must manage the transition with care. |
|
Sorry for the lack of activity and thanks for the feedback. Hopefully I'll get time to work some more on this in the next couple of weeks. @raggi: Would you suggest disabling this functionality by default? |
|
I would not recommend making major API changes without changing major versions, ideally. This situation is unfortunate and tricky, but I don't think we can safely just break this. |
|
What's the status of this? |
|
Can we rebase this and land it in Rack 3.x? |
|
@ioquatix I will move this forward if @mattbostock don't want (have time) to. |
|
I haven't reviewed this PR for a while, but feel free to rebase it and we can go from there. |
|
It seems more likely we will use the updated version of this (#1423) than this, so I'm going to close this now. |
RFC 7239 is an IETF proposed standard for a HTTP
Forwardedrequestheader that aims to replace non-standard
X-Forwarded-*headers.This commit supports all relevant tokens that may be set in the
Forwarded header:
I have prioritised the Forwarded header over the non-standard headers
as I think it's preferable to use the standard header when specified.
Multiple values for each token are allowed by the standard. When
multiple values are supplied, I've followed these rules:
prototoken, use the first value specified as it's mostlikely to be the protocol used by the original client
hosttoken, use the last value specified following theconvention already used by Rack for multiple
X-Forwarded-Forheaders
When both
X-Forwarded-ForandForwardedheaders are supplied and thelatter includes
forvalues, I've concatenated both sets of values withthe
Forwardedvalues coming last.I've been careful to support mixed case in the header and optional
double quotes encapsulating the token value as specified in the RFC.