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

Add method to "upgrade" hyper request to a WebSocket. #53

Closed
kevincox opened this issue Oct 26, 2015 · 10 comments
Closed

Add method to "upgrade" hyper request to a WebSocket. #53

kevincox opened this issue Oct 26, 2015 · 10 comments

Comments

@kevincox
Copy link

Currently the HTTP request is parsed inside of the library. It would be nice if there was a way to check if an existing hyper Request is a websocket request, and if so provide a way to "upgrade" it into a websocket connection. This allows using hyper as a HTTP server and accepting websockets on the same port.

This is similar to how the ruby library faye-websocket works. You can check the env (essentially a standard request format) if it is a websocket, and if so "upgrade" it into a websocket connection.

@illegalprime
Copy link
Collaborator

+1 seems like a good idea. This is like #46 (which never got fixed I'm pretty sure). I was leaning towards getting rid of hyper as a dependency so whatevery the fix is it should be able to fit with every http implementation out there.

@illegalprime
Copy link
Collaborator

@kevincox working on it now on my fork. Do you have any opinions about what the API should be?

@kevincox
Copy link
Author

I imagine something like taking a hyper Request and Response and returning a websocket Client. I imagine setting response headers can be done before passing the Response to the method.

@illegalprime
Copy link
Collaborator

It's not enough to pass a Response to make a client, you need the socket too. The two methods I'm thinking of are passing in a stream (like a TcpStream) and returning a client if the stream is asking for a websocket connection, or returning the original stream.

So like into_ws(self) -> Result<Client, (Self, Err)>
But this requires owning self, to mitigate this I have a method to check if a response/request can be made into a websocket connection.

Is that API good for your use case?

@kevincox
Copy link
Author

I thought the Response had an output stream? Or is that not enough?

The problem with that API is that (IIUC) it reads the request itself (or at least the Headers). The reason I wanted to "upgrade" the request manually is so that I can do routing, authentication and whatever else I want based on the headers. Your method allows me to route requests if it isn't a websocket connection but doesn't give me the option if it is.

@illegalprime
Copy link
Collaborator

Your right, I wasn't familiar with hyper's Requests. As long as the network stream is bundled with the request data it should be possible. There just needs to be a definitive way to match the upgrade request with the network stream (there is in hyper's Request). As for the method signature:

impl IntoWebSocket for Reqeust {
    fn into_ws(self) -> Result<(&mut Self, Client), (Self, Err)> {
        // ...
    }
}

Would you rather Client own the network stream / Request or just borrow it? If you just want to route requests then I would assume it would be OK to let the Client own it.

Since I'm not familiar with hyper it would help to know what code you would want to write to do this.

@kevincox
Copy link
Author

I imagine owning it would be fine. I don't think there is any legitimate use of using it after a websocket connection has ended.

@dns2utf8
Copy link

Is there any progress on this?

@illegalprime
Copy link
Collaborator

illegalprime commented Jul 5, 2016

@dns2utf8, @octplane I just made #80 , does that API seem like it would be useful to you?

@illegalprime
Copy link
Collaborator

@kevincox @dns2utf8 this is implemented now.

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

3 participants