websockets-rs / rust-websocket

A WebSocket (RFC6455) library written in Rust
http://websockets-rs.github.io/rust-websocket/
MIT License
1.55k stars 223 forks source link

Update to support Hyper 0.11? #132

Open generalelectrix opened 7 years ago

generalelectrix commented 7 years ago

Hyper broke a lot of stuff moving to 0.11, and all of the contemporary Hyper examples are incompatible with rust-websocket. Any horizon on a patch to support the latest Hyper?

illegalprime commented 7 years ago

Party! That means they're finally async! That should remove a lot of weird glue I used to hold the two together lol. This is planned, are you hoping to just have it use 0.11 or are you looking for something in particular?

generalelectrix commented 7 years ago

Just looking for the 0.11 update. And yes, all of the examples are fundamentally async and are definitely a lot cleaner looking than they were pre-0.11.

generalelectrix commented 7 years ago

Do you happen to have a vague sense of when this might happen? It would be helpful to me in my project planning, as I can defer development of my websocket-related piece if it means not having to refactor later to get up to the newest hyper. If its more like a months-from-now kind of thing then I will go ahead and implement using the current version and hyper 0.10 and perhaps refactor later. Thank you!

illegalprime commented 7 years ago

I'll try to see what I can do this week and I'll let you know, I feel like it's going to be a small change but I'm always wrong about these things :slightly_smiling_face:

illegalprime commented 7 years ago

@generalelectrix I already ran into issues ^

chrismacklin commented 7 years ago

Ok, thanks for the update! I'll move ahead running on hyper 0.10 for the time being. Thank you for your work on this library, it is working nicely and is easy to use.

dthul commented 7 years ago

Yes, that would be awesome! I started a new project like two hours ago and already got stuck on this issue. My setup: Hyper (0.11) listens on Port 80 for incoming connections. If the hyper request matches a certain path (like "/websocket") I try to upgrade the connection. At the moment this fails with:

error[E0308]: mismatched types
  --> src/main.rs:60:36
   |
60 |                 match HyperRequest(req).into_ws() {
   |                                    ^^^ expected struct `hyper::server::request::Request`, found struct `hyper::Request`
   |
   = note: expected type `hyper::server::request::Request<'_, '_>`
              found type `hyper::Request`

Since the request types of the two hyper versions don't match.

illegalprime commented 7 years ago

@dthul Yeah it's expecting the hyper 0.10 version of that request. For now you can upgrade it manually by implementing IntoWs for a hyper request.

illegalprime commented 7 years ago

I think the move is to ditch hyper in favor of making it an optional feature that allows integrating it into the lib. That will be pretty tough though, but it'll make the crate leaner.

dthul commented 7 years ago

@illegalprime Thanks for the tip with IntoWs. I didn't put too much time into it but it looks like it is not totally straightforward to implement this for a hyper 0.11 request. IntoWs requires a stream that implements AsyncRead and AsyncWrite but Hyper gives you a futures::future::Stream which implements neither of those.

illegalprime commented 7 years ago

@dthul I'll take a look when I get home, it gets down to how do you extract the stream that hyper is giving us.

illegalprime commented 7 years ago

@dthul I still need to test this, but I think what you have to do is verify the headers yourself, create a TcpConnection connecting to https://docs.rs/hyper/0.11.0/hyper/struct.Request.html#method.remote_addr , send the rest of the handshake, and finally wrap the connection in the websocket codec. This is too involved for a normal user so I want to push a hotfix soon that will implement IntoWs for an (AsyncTcpStream, Headers). This hyper version changes a lot, so I want to get rid of the hyper dependency soon.

dthul commented 7 years ago

I get a "connection refused" error when I try to connect to the remote_addr that I get from the Hyper request. Does TCP even allow you to connect to an existing client socket?

illegalprime commented 7 years ago

@dthul oh I see. Yeah this is really tough, unless we can get access to the live connection hyper is using we can't transform it into websockets. If there's no way to do it I'll open a bug in hyper.

nicoburns commented 7 years ago

@illegalprime Did you ever figure out a way to do this?

maxlapshin commented 7 years ago

also interested how to put websocket handler into latest hyper webserver with tokio and async.

illegalprime commented 7 years ago

@nicoburns @maxlapshin I basically have to rip out hyper and use an http parser, so we'll likely lose support for using hyper headers in making connections. Is that a feature that would be missed?

spinda commented 7 years ago

I (independently) wrote https://github.com/spinda/hyper-websocket and https://github.com/hyperium/hyper/pull/1287 to solve this problem.

spinda commented 7 years ago

Also, the option to build without hyper would be really nice for my use case (hyper-based HTTP server which upgrades connections to WebSockets, all using async I/O with Tokio), as long as something like WsUpgrade is still present to be able to either accept or reject a handshake that was parsed outside the websocket library. Currently I have to build with two different versions of hyper in the same project, an 0.10 series release for websocket and an 0.11 series release to actually use with Tokio.

Eijebong commented 7 years ago

Anything new on that ? :)

illegalprime commented 6 years ago

@spinda about to finish finals, so I'm excited to get some time. Biggest thing is moving to httparse and ditching hyper as a dependency, then re-adding it as a feature that implements IntoWs.

SimonSapin commented 6 years ago

Is https://github.com/hyperium/hyper/pull/1563 relevant here?

saethlin commented 5 years ago

Is there any progress here? This crate is slowly drifting out of sync with the rest of the web ecosystem; the current version of hyper is 0.12.16 and this issue to upgrade to 0.11 hasn't seen any signs of life in nearly a year.

I'm willing to take a shot at the upgrade, but I'll need some assistance because I'm far from an expert on this subject.

vi commented 5 years ago

The closest thing is this fork. I don't know how ready is it for a crates.io release...

rust-websocket still depends on tokio-core instead of tokio (there is already a pull request, but I haven't reviewed it yet) and only recently stopped depending on old tokio-tls preventing it to be compiled for new OpenSSL 1.1.1.

If you want to contribute hyper update, you should probably assume that #199 is merged and also look at Enzious fork where things are supposedly done.