x-forwarded-host overwrite for mutli level proxies - #1267
Conversation
Sorry, something went wrong.
With more than 1 proxy the original host was lost, now it will be passed down the proxy chain
jcrugzz
left a comment
There was a problem hiding this comment.
LGTM 👍
Sorry, something went wrong.
|
@jaggernoth host is probably most important but should we also default the other values so we are consistent? |
Sorry, something went wrong.
|
@jcrugzz : From what I can see in the code for other x-forwarded header entries new levels are concatenated as CSV values and I was considering making x-forwarded-host a CSV value as well but it would be much riskier as it may cause compatibility issues (CSV requires additional parsing). |
Sorry, something went wrong.
indexzero
left a comment
There was a problem hiding this comment.
Agree with @jcrugzz comment, but this PR is still 👍
As to the CSV value question @jaggernoth the research I briefly looked at did not suggest that as a format:
Sorry, something went wrong.
With more than 1 proxy the original host was lost, now it will be passed down the proxy chain
Let's consider a scenario
proxy (Internet-facing) -> proxy(load balancer) -> application where both proxies will be based on node-http-proxy, with current logic original host to be lost
With updated logic the original forwarded host will be passed down the proxy chain