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

Implement http.Hijacker to support interaction with Websockets #131

Closed
durdn opened this issue Jan 5, 2017 · 3 comments
Closed

Implement http.Hijacker to support interaction with Websockets #131

durdn opened this issue Jan 5, 2017 · 3 comments

Comments

@durdn
Copy link

durdn commented Jan 5, 2017

Hey there,

You wrote a fantastic library and I've been using it with pleasure so far. Up until Yesterday I was using https://github.com/NYTimes/gziphandler to compress all the responses passing through my routes. Then I saw you already have implemented a middleware that does the exact same thing! RTFM right? :)

Anyway I have run into an issue when using the middleware.DefaultCompress and gorilla/websocket (and I saw reports of similar failures for people using code.google.com/p/go.net/websocket):

I run into this:

websocket: response does not implement http.Hijacker

It seems the response interface of your library does not implement http.Hijacker which is needed to use websockets.

The fix appears relatively easy and several other libraries have implemented a fix. For example:

caddyserver/caddy@05957b4

https://github.com/gin-gonic/gin/pull/105/files

Also the gziphandler project mentioned above has just committed the same change just the other day after I reported the problem:
nytimes/gziphandler#26

Other useful thread:
gin-gonic/gin#51

I can definitely work around my problem by not using the Compress middleware in bulk for all my routes but I thought you'd be interested in the problem and possibly copy the fixes above.

I am not on a quest for getting this issue fixed everywhere btw, I just found out chi has the capability I need and I always try to reduce my dependencies whenever possible.

Thanks for listening!
Cheers!
Nick

@pkieltyka
Copy link
Member

@durdn thanks for the report. we'll fix it :)

cc: @VojtechVitek - whichever one of us gets to it first. I'll try to look at this tomorrow.

@pkieltyka
Copy link
Member

fixed in 49a0ea6

@durdn
Copy link
Author

durdn commented Jan 6, 2017

@pkieltyka Amazing responsiveness! Thanks a lot!

kujenga added a commit to kujenga/go-stdlib that referenced this issue Jul 16, 2018
This is needed in order for users of this package to implement websocket
support, as without it, the statusCodeTracker struct does not implement
the correct interface because it knows nothing about the underlying
implementation of it's embedded http.ResponseWriter.

For reference, many other project have had this same issue, here's a
list of some found in a quick search for the issue, I'm sure there are
others as well:
- golang/go#14797
- census-instrumentation/opencensus-go#642
- go-chi/chi#131
- nytimes/gziphandler#26
- caddyserver/caddy#133
- emicklei/go-restful#354
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

2 participants