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

Update to support Hyper 0.11? #132

Open
generalelectrix opened this issue Jun 16, 2017 · 24 comments
Open

Update to support Hyper 0.11? #132

generalelectrix opened this issue Jun 16, 2017 · 24 comments

Comments

@generalelectrix
Copy link

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Author

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
Copy link
Collaborator

illegalprime commented Jun 19, 2017

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 🙂

@illegalprime
Copy link
Collaborator

@generalelectrix I already ran into issues ^

@chrismacklin
Copy link

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
Copy link

dthul commented Jun 25, 2017

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
Copy link
Collaborator

@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
Copy link
Collaborator

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
Copy link

dthul commented Jun 27, 2017

@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
Copy link
Collaborator

@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
Copy link
Collaborator

@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
Copy link

dthul commented Jun 28, 2017

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
Copy link
Collaborator

@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
Copy link

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

@maxlapshin
Copy link

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

@illegalprime
Copy link
Collaborator

@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
Copy link
Contributor

spinda commented Aug 31, 2017

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

@spinda
Copy link
Contributor

spinda commented Aug 31, 2017

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
Copy link
Contributor

Eijebong commented Sep 16, 2017

Anything new on that ? :)

@illegalprime
Copy link
Collaborator

@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
Copy link
Contributor

Is hyperium/hyper#1563 relevant here?

@saethlin
Copy link

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
Copy link
Member

vi commented Nov 29, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests