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

Forwarding headers fixes/improvements #338

Merged
merged 3 commits into from
Jun 16, 2023

Conversation

beerriot
Copy link
Contributor

@beerriot beerriot commented Mar 8, 2023

This branch started with a fix for #315. webmachine_request:get_peer now returns the earliest non-private IP, which is the one closest to the client, and webmachine_request:get_sock returns the latest non-private IP, which is the one closest to the server.

While making the first change, I noticed that IPv6 wasn't well-supported. Localhost, link-local, and private v4 IPs translated to v6 IPs are now treated as private by get_peer and get_sock.

Finally, support is added for the standardized Forwarded header. X-Forwarded-For is checked only if no answer was found in Forwarded.

This is all implemented based on reading specs and other guides. I don't have a proper setup to test the behaviors of real-world proxies.

@beerriot
Copy link
Contributor Author

Rebased to resolve conflict with tuple-module removal.

Resolves webmachine#315. `wrq:peer` is documented as returning the client IP. That's
the first non-local element in the first X-Forwarded-For header. See
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For

`webmachine_request:get_sock` continues to return the last entry in
X-Forwarded-For. So basically "get_sock" is "how is this server connected
to the internet" and "get_peer" is "how is this client connected to the
internet"?
@seancribbs seancribbs merged commit 0bdcc77 into webmachine:main Jun 16, 2023
6 checks passed
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