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

Splitting port from the remote address #129

Closed
wants to merge 1 commit into from
Closed

Splitting port from the remote address #129

wants to merge 1 commit into from

Conversation

abramovic
Copy link

Remote Address goes from "127.0.0.1:8000" to "127.0.0.1"

@zenazn
Copy link
Owner

zenazn commented May 24, 2015

Neither the X-Forwarded-For header nor the X-Real-IP header should have ports in them. Could you elaborate on why you think this patch is needed?

@abramovic
Copy link
Author

Right, X-Forwarded-For and X-Real-IP shouldn't have ports in them. This is if those headers are not present (ie: where the app isn't behind a load balancer) so this parses the original req.RemoteAddr which might include ports.

If the X-Forwarded-For and X-Real-IP headers are present then req.RemoteAddr won't get parsed (line 45 and line 47).

@zenazn
Copy link
Owner

zenazn commented May 24, 2015

Err, RemoteAddr is set by net/http, and this middleware doesn't touch RemoteAddr unless one of those two headers is set. Are you perhaps printing out the port of RemoteAddr when you don't intend to?

@zenazn
Copy link
Owner

zenazn commented Jun 23, 2015

I don't think this is a bug in Goji so I'm going to close this PR. Please feel free to update this PR if you're still having issues!

@zenazn zenazn closed this Jun 23, 2015
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.

2 participants