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

There should be a way to get a peer's IP address #71

Open
miquels opened this issue Aug 20, 2018 · 6 comments
Open

There should be a way to get a peer's IP address #71

miquels opened this issue Aug 20, 2018 · 6 comments
Labels
enhancement New feature or request

Comments

@miquels
Copy link

miquels commented Aug 20, 2018

There should be a way to get the peer's IP address, e.g. tcpstream.peer_addr(). A peer_addr() or client_addr() method should be added to the request struct.

If this issue should be filed with hyper instead let me know.

@jwilm
Copy link
Contributor

jwilm commented Aug 20, 2018

As a work-around, if you're behind a reverse proxy, you can have the proxy server add an x-forwarded-for or x-real-ip header

@shepmaster shepmaster added the enhancement New feature or request label Aug 22, 2018
@shepmaster shepmaster changed the title peer IP address There should be a way to get a peer's IP address Aug 22, 2018
@carllerche
Copy link
Owner

There are a few ways that the client IP address can be exposed:

  1. Storing it in the http Extensions map.
  2. Defining a new Request type that has additional fields.
  3. Adding IP address to http::Request
  4. Storing it in the x-real-ip (or other) header.

You have thoughts @seanmonstar?

@seanmonstar
Copy link
Collaborator

Storing it in the http Extensions map.

Probably best option, but the sad part is that it means everyone pays to store the value even though only some need access to it...

Defining a new Request type that has additional fields.

Definitely the least attractive option (why have http::Request if we have to newtype over it?).

Adding IP address to http::Request

The reason for it not being there in the first place is because it doesn't always make sense; it's mixing HTTP with TCP/IP. Some transports have no concept of an IP address (like UDS).

Storing it in the x-real-ip (or other) header.

I think users should probably look for that header, since in most cases, the client isn't directly connected to their server, but there's usually several steps of load balancers in the way. But I don't think a framework should be inserting this header if it didn't already exist.

@shepmaster
Copy link
Collaborator

a framework

And I'd expect a framework to provide an abstraction over these (x-real-ip || ... || TCP address).

no concept of an IP address (like UDS).

Great point! This feels something that Go takes advantage of and makes sense. Seems like this will always have to be an Option.

@carllerche
Copy link
Owner

@seanmonstar I agree with your summary.

Though, a thought occurs. I wonder if additional data could be passed via the generic in Request<T>.

Then, there would need to be an additional trait bound: BufStream + SomethingElse to access the addr. Middleware would have to be able to preserve that extra data too somehow.

@miquels
Copy link
Author

miquels commented Aug 28, 2018

Storing it in the http Extensions map.

Probably best option, but the sad part is that it means everyone pays to store the value even though only some need access to it...

How about doing that only when asked for, say by adding a method to the Builder like pub fn save_client_ip_address(self, val: bool) -> Self ?

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

No branches or pull requests

5 participants