hyperium / hyper

An HTTP library for Rust
https://hyper.rs
MIT License
14.64k stars 1.6k forks source link

CONNECT requests not really doable in the current client setup #2560

Open nox opened 3 years ago

nox commented 3 years ago

CONNECT requests, be them h1 or h2, are not really doable through the Hyper client, with the current connector design.

hyper::client::Client::request takes a request, extracts the (scheme, host, port) triple from its URI, and then passes that to its inner connector.

That means that for a CONNECT example.org:80 request, the connector will try to connect to example.org:80 to then send the CONNECT request to it. That's obviously not what you want when doing CONNECT requests.

I ended up making a new ProxyConnector<C> connector that completely ignores the uri argument of <Service<Uri>>::call, but that sounds like a workaround and there is probably a better way to handle that in Hyper itself.

seanmonstar commented 3 years ago

The solution you used is in fact the current recommended way. As another example, reqwest creates a custom connector that keeps track of all configured "proxies", and checks it when a new connection is needed to determine whether it needs to connect to the proxy or to the original destination.

Another design possibility could be to include some hyper::client::connect::Tunnel or Proxy or what-have-you, that works as an extension for the Request::extensions().

nox commented 3 years ago

Rather than extensions, I've thought of a more general mechanism which would probably help for other things than proxy connections.

We could equip Client with a connect_to method:

impl<C, B> Client<C, B> where
    C: Connect + Clone + Send + Sync + 'static,
    B: HttpBody + Send + 'static,
    B::Data: Send,
    B::Error: Into<Box<dyn StdError + Send + Sync>>,
{
    pub fn connect_to(&self, uri: Uri) -> hyper::Result<hyper::client::conn::SendRequest<B>>;
}

Basically, this method would either return a SendRequest<B> just like hyper::client::conn::handshake, or a user error if the provided uri was not the kind of URI expected by the connector service, or a connect error if the connection failed.

In addition to allowing to make CONNECT requests without the ProxyConnector<C> trick, this method could also be a way to provide the caller with information about the connection more cleanly than through writing Connected::extra into the response extensions (which are not always available if the connection succeeded but the actual request failed), by making the return type be hyper::Result<(hyper::client::conn::SendRequest<B>, hyper::client::connect::Connected)> or something like that.

kanpov commented 4 months ago

My solution to this that was somewhat simple was to wrap a TCP or UNIX socket, and when connecting to it, issue the CONNECT request in plaintext. This was needed for me since I was doing a firecracker host-initiated connection to guest VM, where a CONNECT needs to be issued first with the port of the guest vsock and firecracker returns OK if the host -> guest tunnel is established successfully, but the principle works for any other CONNECT request. In this case, the CONNECT destination is not even a URI, but a guest vsock port (so a u32)

Here's the complete code in my project, hyper-client-sockets: https://github.com/kanpov/hyper-client-sockets/blob/master/src/firecracker.rs