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

websockets support #18

Closed
leavengood opened this issue Dec 10, 2015 · 8 comments
Closed

websockets support #18

leavengood opened this issue Dec 10, 2015 · 8 comments

Comments

@leavengood
Copy link

My coworkers and I are a big fan of fasthttp, it has proven very useful for a work project and we find it very ergonomic to use.

I now have need for websockets with a fasthttp.Server. What is the plan for websockets support in fasthttp?

There are two major Go packages for websockets:

The former is a bit easier to use and probably is considered the "standard" Go websockets package since it comes from the Go team. Nonetheless the gorilla websockets seems to support the standard better and it appears more flexible and not as tied to net/http as the golang/net version.

I plan to fork gorilla/websocket and add a fasthttp version of the Upgrade method:

https://godoc.org/github.com/gorilla/websocket#Upgrader.Upgrade

If anyone is working on this, or if there are other ideas on how to get websockets working with fasthttp, please reply.

@leavengood
Copy link
Author

I have a rough version working now. The gorilla Upgrade method is pretty tied in with how net/http does hijacking (which I must admit is pretty hard to use as alluded to in the README for fasthttp), and it was hard to even try to refactor it for fasthttp. So for now I ended up just adding an exported NewServerConn function to gorilla that takes a *net.Conn and just calls the existing newConn to create a *websocket.Conn. I then made my own fasthttp.Handler which does all the necessary checks of the HTTP request for websockets, calls ctx.Hijack() and returns the Switching Protocols status code.

This works fine with my simple JavaScript websockets code.

I still think this would be better if done in gorilla, so I will try adding that next. I made just have to add a new FastHTTPUpgrader struct or something like that, since Upgrader is pretty tied to net/http. The good news is that websocket.Conn doesn't depend on net/http at all, and it does most of the hard work for websockets.

I don't yet know if the gorilla project will be open to adding support for fasthttp though, even if I do all the work.

@valyala
Copy link
Owner

valyala commented Dec 11, 2015

If anyone is working on this, or if there are other ideas on how to get websockets working with fasthttp, please reply.

@leavengood , there are plans to add websockets support to fasthttp, but currently I'm busy with other stuff. So it would be great if somebody implements websockets on top of fasthttp.

@leavengood
Copy link
Author

I have implemented a first attempt at supporting fasthttp in the gorilla websocket package. It can be seen in my fork:

leavengood/websocket@a24f11b

A lot of the changes are just making various utility functions usable for both net/http and fasthttp, but the "meat" of it is here:

leavengood/websocket@a24f11b#diff-34c6b408d72845d076d47126c29948d1R222

I have copied the style of the normal Upgrader a bit, and it works, but I don't know how much it fits in with the style of fasthttp. Here is an example of what using it would look like:

handler := func(c *websocket.Conn) {
    // Do something with the connection
}
upgrader := &websocket.FastHTTPUpgrader{
    Handler:     handler,
    // Don't check the origin header
    CheckOrigin: func(ctx *fasthttp.RequestCtx) bool { return true },
}
if err := fasthttp.ListenAndServe(":12345", upgrader.UpgradeHandler); err != nil {
    log.Fatalf("Error in ListenAndServe: %s", err)
}

Let me know what you think of this, and I can see about adding some tests and then making a PR to the main gorilla websocket repo.

@leavengood
Copy link
Author

I made a PR and have made some adjustments to account for fasthttp needing Go 1.4 or above:

gorilla/websocket#98

@valyala
Copy link
Owner

valyala commented Dec 16, 2015

@leavengood , gorilla/websocket#98 looks cool! It would be great if it will be merged eventually into gorilla websockets.

@leavengood
Copy link
Author

It may need some more tweaking but they seem to be open to the idea. Worse case I could maintain a fork with fasthttp support. Their project seems pretty stable with a few commits now and then, so it wouldn't be hard to keep a fork up-to-date. For the moment if other people need this they can use my fork, though at the moment the new code is in a fasthttp branch.

@leavengood
Copy link
Author

Gorilla websockets does not currently want to merge in my changes, so I have closed the PR and merged that code into the master branch of my fork (https://github.com/leavengood/websocket). If anyone wishes to use websockets with fasthttp, they can import my fork. I'm going to close this issue as well.

@valyala
Copy link
Owner

valyala commented Dec 24, 2015

@leavengood , I mentioned your library in README.md.

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

No branches or pull requests

2 participants