Skip to content
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

Fix for x-forwarded-for not being applied #533

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

notepid
Copy link
Contributor

@notepid notepid commented Mar 19, 2024

this.proxied is not set before calling setClient() resulting in a "undefined" value for this.proxied. This in turn prevents the proxy headers to be used when resolving the client IP.

In a scenario where a reverse proxy is used the client IP will always show as 127.0.0.1 instead of the X-Forwarded-For header.

Also consider using request.socket instead of connection in setClient() Ref: https://stackoverflow.com/questions/8107856/how-to-determine-a-users-ip-address-in-node

…ed is set so its set during constructor call or the proxy headers will not be applied
@NuSkooler
Copy link
Owner

@notepid Thanks for this!

RE: request.socket, is the current method causing issues?

@notepid
Copy link
Contributor Author

notepid commented Mar 19, 2024

@notepid Thanks for this!

You're welcome :)

RE: request.socket, is the current method causing issues?

No issues as I see. Just noted while looking into this that connection is being deprecated.

I tested with this code and it looked to work but that was just a quick test:
this.resolvedRemoteAddress = (this.client.proxied && httpRequest.headers['x-forwarded-for'] || '').split(',').pop().trim() || httpRequest.socket.remoteAddress;

Apparently x-forwarded-for can be a list of addresses.

@NuSkooler
Copy link
Owner

@notepid Thanks for this!
No issues as I see. Just noted while looking into this that connection is being deprecated.

Ah gotcha, feel free to open a PR or update here. I'm happy either way if you want to update it. It can land in a ticket otherwise. I have a bit list of things I need to get back to when I get some more time on my plate.

Apparently x-forwarded-for can be a list of addresses.

Never can be just "that easy" :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants