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

Server Protocol Upgrades #1323

Closed
seanmonstar opened this issue Sep 16, 2017 · 6 comments
Closed

Server Protocol Upgrades #1323

seanmonstar opened this issue Sep 16, 2017 · 6 comments
Labels
A-server Area: server. C-feature Category: feature. This is adding a new feature. E-medium Effort: medium. Some knowledge of how hyper internal works would be useful.

Comments

@seanmonstar
Copy link
Member

Add support for servers to receive requests that wish to upgrade to a different protocol, such as websockets. A proposed API follows:

Proposal

  • Response
    • pub fn upgrade(proto: header::Upgrade) -> (Response, server::Upgrade)

      This sets the StatusCode::SwitchingProtocols, and the Upgrade header provided. It returns the Response, in case other headers need to be added, and a server::Upgrade type, explained next.

  • server::Upgrade
    • impl Future for server::Upgrade
    • This type is a Future that resolves to the underlying IO object that is being upgraded.

Usage

// inside a `Service`
fn call(&self, req: Request) -> Self::Future {
    if wants_ws_upgrade(&req) {
        let (res, upgrade) = Response::upgrade(Upgrade::ws());
        self.ws_queue.send(res);
        future::ok(res)
    } else {
        self.handle_http(req)
    } 
}

A theoretical websocket server would also exist, and a channel, the ws_queue, would be used to give the socket to the websocket server.

Implementation

The server::Upgrade could wrap a futures::sync::oneshot::Receiver. The purpose for wrapping one is to hide the implementation details.

When upgrading protocols, the response is only supposed to send headers. A blank newline after a header block with a 101 response code is supposed to be for the already upgraded protocol. That means sending a body with an upgrade response is an error.

If it is possible with some Into impls to allow Response::upgrade to return a Response<()>, that would probably be ideal. I imagine it might not be doable, in which case, it will need to be enforced at runtime. This will still work, as the server::Upgrade can be sent a hyper::Error noting that a body was illegal.

It would be nice if the type could be server::Upgrade<T>, where T is the specific IO type being used internally. However, that might not be possible either, as the Service may not have a way to know what type is being used (it might be possible for a user to have encoded the type in their new_service, and thus Service, and be able to statically guarantee the type, though!). If not, then the return type will need to be a Box<AsyncRead + AsyncWrite>.

Any buffered bytes internally associated with the socket should be returned as well, as a Chunk.

@seanmonstar seanmonstar added the A-server Area: server. label Sep 16, 2017
@seanmonstar seanmonstar added E-medium Effort: medium. Some knowledge of how hyper internal works would be useful. C-feature Category: feature. This is adding a new feature. labels Sep 16, 2017
@lqf96
Copy link

lqf96 commented Sep 29, 2017

This is a great idea, but what about handling CONNECT method at the server side (like in a HTTP proxy server)? I think the upgrade function should only provide the underlying I/O object, and it is the user's responsibility to return a proper HTTP response.

@seanmonstar
Copy link
Member Author

@lqf96 I would definitely like to also offer a way of handling CONNECT requests.

However, it's not quite the same, as HTTP/2 defines CONNECT a little differently. The original socket does not get upgraded out of h2, it just sends and receives DATA frames on the same stream, and it should be forwarded to the target host. So, when hyper gains HTTP/2 support, it would be wrong to pull the socket out because some stream sent a CONNECT. It would need to continue to just stream bytes back and forth.

As for handling the response yourself, the thing is that you need to return a Response anyways, so it seems useful to be able to fill in the status code and headers, and let hyper write that to the socket.

@lqf96
Copy link

lqf96 commented Sep 30, 2017

Yes, that is indeed a problem. Maybe for HTTP/2 we need two kinds of upgrade, one for the TCP connection and another one for the H2 stream containing the HTTP request. The former one can be used to upgrade from HTTP/2 to something else, and the latter one can be used for CONNECT or (possibly) running WebSocket over H2 stream.

@seanmonstar
Copy link
Member Author

The former one can be used to upgrade from HTTP/2 to something else

HTTP/2 does not define a way to upgrade to another protocol. You cannot upgrade from HTTP/2 to websockets.

@idanarye
Copy link

Just want to note that the websocket crate already supported upgrading with old sync Hyper. It could do it because the old Request exposed the underlying IO stream. The new async version doesn't.

@nayato
Copy link

nayato commented Jan 4, 2018

@seanmonstar it all sounds reasonable. I'm not sure I get the part about returning Chunk of buffered bytes at the end though. I'd expect any buffered bytes be "put back" and made available through reading from returned IO object. I.e. return implementation that first depletes buffered bytes, then proxies calls to actual underlying IO object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-server Area: server. C-feature Category: feature. This is adding a new feature. E-medium Effort: medium. Some knowledge of how hyper internal works would be useful.
Projects
None yet
Development

No branches or pull requests

4 participants