warp icon indicating copy to clipboard operation
warp copied to clipboard

addr::remote() always returns None with externally-provided TcpListener

Open mcginty opened this issue 4 years ago • 6 comments

there's no way currently to run a warp service that depends on the addr::remote() filter when served from a custom-initialized TCP socket, as neither Server::run_incoming2() nor FilteredService::call_with_addr() are accessible from outside the crate, so even trying the warp::service() route doesn't help.

https://github.com/seanmonstar/warp/pull/713 attempts to solve this one way, but it seems to have not gotten any activity.

In its simplest form, is there any reason not to simply make FilteredService::call_with_addr() public?

mcginty avatar Apr 05 '21 09:04 mcginty

You can do it by jumping through some hoops using hyper:

use futures::{Future, TryFutureExt};
use hyper::{server::Server, service::make_service_fn};
use std::convert::Infallible;
use std::net::SocketAddr;
use warp::filters::BoxedFilter;
use warp::Reply;

/// Create a hyper server with the provided `filter`, binding to `addr`. This also sets the
/// `TCP_NODELAY` flag on incoming connections.
pub(crate) fn serve_it<T: Into<SocketAddr>>(
    addr: T,
    filter: BoxedFilter<(impl Reply + 'static,)>,
) -> anyhow::Result<(SocketAddr, impl Future<Output = anyhow::Result<()>>)> {
    let filtered_service = warp::service(filter);

    let make_svc = make_service_fn(move |_| {
        let filtered_service = filtered_service.clone();
        async move { Ok::<_, Infallible>(filtered_service) }
    });

    let listener: std::net::TcpListener = make_listener(addr)?;
    let bound_to = listener.local_addr()?;
    let builder = Server::from_tcp(listener)?;
    let fut = builder.tcp_nodelay(true).serve(make_svc).map_err(|e| e.into());
    Ok((bound_to, fut))
}

wngr avatar Apr 06 '21 09:04 wngr

@wngr I had code similar to this - unless I'm mistaken, you can't then access the remote_addr in warp with this setup - it always returns None, which is the problem.

mcginty avatar Apr 06 '21 14:04 mcginty

You're right, you can't. The original issue description is misleading then though, maybe you could clarify the missing behaviour a bit more.

wngr avatar Apr 06 '21 14:04 wngr

Oh wow, yeah that description was unreadable looking back - sorry about that. Hopefully the updated one is a bit more readable, thanks for looking into it.

mcginty avatar Apr 06 '21 14:04 mcginty

Hope we can merge #713 before v0.4 release.

valbendan avatar Jun 03 '21 07:06 valbendan

So, what is the status on this? When is v0.4 coming?

I see there are actually two concurrent implementations in PR https://github.com/seanmonstar/warp/pull/964 and #713. Which will be accepted? I see that #964 does not break any public interface (allegedly). Can it be shipped in a minor version?

Asking this because I need it for a current project of mine.

tokahuke avatar May 29 '22 00:05 tokahuke