-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
proposal: x/net/websocket: implement support for websockets over HTTP/2 #53209
Comments
one limitation on the websocket connection is going to be that it will not be able to set deadlins
I don't really know if this can be a problem, just something to take into consideration |
Yeah, that's a good point. I think ideally, the websocket APIs would accept a context to allow support for cancellation, but it's a bit tricky to implement given that HTTP/2 requires sharing a single TCP connection between multiple goroutines. The real fix for this likely involves extending x/net/http2 with a mechanism for setting deadlines for individual reads by adding some machinery so that all reads get routed through a central point that knows how to retry the read if it wasn't supposed to time out. Having a heap of wake times seems like it would work for this. That's also a pretty big change, so it seems like it would make sense to address it in a separate effort. |
any update? |
Overview
I propose extending the x/net/websocket package to support connections made over HTTP/2. This will require no external changes for the server, but the client will need to expose a new mechanism for allowing users to request that HTTP/2 is used to make a connection.
Public Interface Changes
websocket.Config
will gain a newHTTP2Transport
member of type*http2.Transport
x/net/websocket
will gain two new public error types:ErrBadpath
andErrBadHandshake
Having
http2
in the name/type of a public member feels a little strange, but storing a generichttp.RoundTripper
makes it impossible to make sure that we useconfig.TlsConfig
if the user provides one but the transport they provide does not have a TLSClientConfig, and additionally opens up the question of what happens if we are passed a http.Transport rather than a http2.Transport. If someone has even modernly strong opinions about a different color to paint this particular shed, I would love to hear them.Implementation Notes
A strawman implementation based on the changes I'm proposing in #53208 can be found at https://github.com/ethanpailes/golang-org-x-net/tree/ep/websocket-http2-try3. The main commit is ethanpailes/golang-org-x-net@df38e5d, but I may push little tweaks on top of it.
Enough about the handshake has changed from HTTP/1.1 that the implementation I came up rolls entirely new handshake subroutines for the new transport version. Once the handshake is done, the framing mechanisms are all the same so we can just reuse all the existing infrastructure for that by wrapping out HTTP/2 streams in the right interfaces.
Related Issues
#49918 contains initial discussion about HTTP/2 websocket support
#53208 contains a proposal for adding SETTINGS_ENABLE_CONNECT_PROTOCOL support to x/net/http2, which is required to implement websocket HTTP/2 support.
The text was updated successfully, but these errors were encountered: