x-forwarded-port not being set correctly... maybe #341

Closed
blahed opened this Issue Nov 27, 2012 · 4 comments

Comments

Projects
None yet
4 participants
@blahed
Contributor

blahed commented Nov 27, 2012

My understanding is that x-forwarded-port should be set to the server port that the request came in on: 80, 443, etc.

If that's the case, it's not happening. req.connection.remotePort is the requesting client's remote port. I didn't see an obvious way to get the server from the request, but it seems easy enough to determine it from the host header/protocol.

Assuming I'm understanding x-forwarded-port correctly, I've got a branch that fixes the issue here:

https://github.com/blahed/node-http-proxy/compare/x-forwarded-port

@hmalphettes

This comment has been minimized.

Show comment
Hide comment
@hmalphettes

hmalphettes Jan 2, 2013

+1
I have used x-forwarded-port with nginx; the expected value of the header is the port of the original request before it was proxied.

Say my node server is running on 3000 and is proxied by node-http-proxy on 3001.
I would like to know when crafting URLs for the original client that the public port is 3001.
At the moment node-http-proxy would give me something like 62112.

I have tested @blahed's patch and it works for me.

+1
I have used x-forwarded-port with nginx; the expected value of the header is the port of the original request before it was proxied.

Say my node server is running on 3000 and is proxied by node-http-proxy on 3001.
I would like to know when crafting URLs for the original client that the public port is 3001.
At the moment node-http-proxy would give me something like 62112.

I have tested @blahed's patch and it works for me.

blahed added a commit to blahed/node-http-proxy that referenced this issue May 14, 2013

determine x-forwarded-port from host header
`req.remotePort' returns the ephemeral port, which is not useful.
node v0.10.0 added `req.localPort' which returns what we want, but
we want to maintain backwards compatibility. Fixes #341 & #227
@fideloper

This comment has been minimized.

Show comment
Hide comment
@fideloper

fideloper Dec 14, 2013

Hello!

I'm just coming across this now. I was wondering if there was a planned resolution, or something I missed?

Installing node v0.10.23 and node-http-proxy 0.10.3 shows the same behavior for me.

Edit: The caronte branch exhibits this behavior as well.

Hello!

I'm just coming across this now. I was wondering if there was a planned resolution, or something I missed?

Installing node v0.10.23 and node-http-proxy 0.10.3 shows the same behavior for me.

Edit: The caronte branch exhibits this behavior as well.

yawnt added a commit that referenced this issue Dec 18, 2013

@yawnt

This comment has been minimized.

Show comment
Hide comment
@yawnt

yawnt Dec 18, 2013

Contributor

@fideloper fixed in 97e4600 :)

Contributor

yawnt commented Dec 18, 2013

@fideloper fixed in 97e4600 :)

@yawnt yawnt closed this Dec 18, 2013

@fideloper

This comment has been minimized.

Show comment
Hide comment
@fideloper

fideloper Dec 18, 2013

Awesome, thanks!

Awesome, thanks!

indexzero added a commit that referenced this issue Dec 27, 2013

determine x-forwarded-port from host header
`req.remotePort' returns the ephemeral port, which is not useful.
node v0.10.0 added `req.localPort' which returns what we want, but
we want to maintain backwards compatibility. Fixes #341 & #227
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment