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

proposal: x/net/websocket: implement support for websockets over HTTP/2 #53209

Open
ethanpailes opened this issue Jun 2, 2022 · 4 comments
Open
Labels
Milestone

Comments

@ethanpailes
Copy link

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 new HTTP2Transport member of type *http2.Transport
x/net/websocket will gain two new public error types: ErrBadpath and ErrBadHandshake

Having http2 in the name/type of a public member feels a little strange, but storing a generic http.RoundTripper makes it impossible to make sure that we use config.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.

@ianlancetaylor
Copy link
Contributor

CC @neild @bradfitz

@aojea
Copy link
Contributor

aojea commented Jun 3, 2022

one limitation on the websocket connection is going to be that it will not be able to set deadlins

https://github.com/golang/net/blob/0fccb6fa2b5ce302a9da5afc2513d351bd175889/websocket/websocket.go#L268

var errSetDeadline = errors.New("websocket: cannot set deadline: not using a net.Conn")

I don't really know if this can be a problem, just something to take into consideration

@ethanpailes
Copy link
Author

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.

@masx200
Copy link

masx200 commented Feb 28, 2024

any update?

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

No branches or pull requests

5 participants