Add `address` filter that gives the client / peer's address about warp

[seanmonstar]

For reference, here's that branch I started: https://github.com/seanmonstar/warp/compare/addr?expand=1

It doesn't need the local hyper dependency any more, and doesn't actually add a new filter yet, just makes the address available in the Route. Running a pipelined benchmark against the branch suggests some slow down, which is not desirable.

[seanmonstar]

Note that hyper gained the ability for a MakeService to receive a reference to the connection when making a new Service, so it could be fairly simple to implement in warp now.

A blocker is that warp allows any transport type that implements AsyncRead + AsyncWrite, which doesn't provide a way to access the remote address. We could add a trait like Transport: AsyncRead + AsyncWrite, and new server constructors with the new bounds.

[seanmonstar]

Good idea! Hm, should this try to be smarter, by checking X-Forwarded-For and such, or simply return the value from the TcpStream?

[markcol]

If you don't use the x-forwarded-for header contents, aren't you going to be returning the address of the last proxy that touched the request, rather than the actual client address?

[kamalmarhubi]

I think a collection of filters would make sense here:

• one that gets the address of the connected socket
• one that gets the address from X-Forwarded-For (returns an iterator?)
• one that gets the address from Forwarded (https://tools.ietf.org/html/rfc7239)

I don't think a decision should be made for the user on whether they trust the proxy header or not. Some friendly default combinations could be useful though, eg Forwarded or X-Forwarded-For if available, else socket.

[seanmonstar]

@markcol yes, but that's only an issue if proxies are involved. The other side is that you're trusting that a proxy was in place and it would have had to look at the remote address of its TcpStream. If it's done automatically and there is no proxy, you'd be trusting an end user that could forge a fake header.

It seems like both are useful, depending on your deployment.

[seanmonstar]

I have a branch a branch exploring adding this that I can push tomorrow. It might be hurting performance slightly, making me wish for a solution that doesn't do anything unless the filter was actually being used...

[kamalmarhubi]

Is this possible without changes to hyper? I didn't see any way to get at the address from Request that you get as a hyper::Service, which is why I started poking around hyper's code and issues.

[seanmonstar]

Yes it's possible, just not with hyper::Server. It requires creating the listener and using Http::serve_connection, so that the bound service could have grabbed the socket address of the connection first.

[kamalmarhubi]

@seanmonstar how are you benchmarking? in case I get to experiment a bit this weekend...

[seanmonstar]

The server program:

extern crate warp;

use warp::Filter;

fn main() {
    let text = warp::path("plaintext")
        .map(|| "Hello, World!");
    let json = warp::path("json")
        .map(|| warp::reply::json(&["Hello, World"]));
    let routes = text.or(json);

    warp::serve(routes)
        .unstable_pipeline()
        .run(([127, 0, 0, 1], 3030));
}

And then, using wrk with a pipeline script, something like wrk -t1 -d10 -c50 -s pipeline.lua http://127.0.0.1:3030/plaintext -- 16. You can fiddle with the -c argument some to find a good baseline on your machine for warp master, and then compare with the branch.

[andywwright]

@seanmonstar would you consider to make it as a feature? Those, for whom a client IP based routing is critical (like me) would switch it on and pay the penalty, and it wouldn't affect anyone else that way.