golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.19k stars 17.57k forks source link

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

Open ethanpailes opened 2 years ago

ethanpailes commented 2 years ago

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 https://github.com/golang/go/issues/53208 can be found at https://github.com/ethanpailes/golang-org-x-net/tree/ep/websocket-http2-try3. The main commit is https://github.com/ethanpailes/golang-org-x-net/commit/df38e5d6ac95add5640e29389b50d380277ed1f0, 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

https://github.com/golang/go/issues/49918 contains initial discussion about HTTP/2 websocket support

https://github.com/golang/go/issues/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 commented 2 years ago

CC @neild @bradfitz

aojea commented 2 years ago

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 commented 2 years ago

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 commented 7 months ago

any update?