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/http2: support for WebSockets over HTTP/2 #49918

Open
mitar opened this issue Dec 2, 2021 · 5 comments
Open

proposal: x/net/http2: support for WebSockets over HTTP/2 #49918

mitar opened this issue Dec 2, 2021 · 5 comments
Labels
Projects
Milestone

Comments

@mitar
Copy link

@mitar mitar commented Dec 2, 2021

What version of Go are you using (go version)?

go version go1.17.3 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What did you do?

I wanted to use Websockets over HTTP2.

What did you expect to see?

Support for it in Go standard library.

What did you see instead?

It does not support it yet.

See #46319 for background.

WebSockets over HTTP2 have been standardized as RFC 8441 and Firefox and Chromium supports that. Thus, I suggest that support is added for Websockets over HTTP2. The server should also be able to specify using HTTP/2 SETTINGS parameter that it supports Websockets over HTTP2.

@aojea
Copy link

@aojea aojea commented Dec 21, 2021

also related #27244

@aojea
Copy link

@aojea aojea commented Dec 21, 2021

Just for reference https://datatracker.ietf.org/doc/html/rfc8441 it seems this requires (please correct me if I'm wrong, not very familiar with the stdlib implemententation):

  1. Implement the SETTINGS_ENABLE_CONNECT_PROTOCOL SETTINGS

Requires adding an HTTP2 server option to enable the setting

  1. Implement the The Extended CONNECT Method

This requires adding a new field to the requests

   o  A new pseudo-header field :protocol MAY be included on request
      HEADERS indicating the desired protocol to be spoken on the tunnel
      created by CONNECT.  The pseudo-header field is single valued and
      contains a value from the "Hypertext Transfer Protocol (HTTP)
      Upgrade Token Registry" located at
      <https://www.iana.org/assignments/http-upgrade-tokens/>

and adding the logic to use the field if SETTINGS_ENABLE_CONNECT_PROTOCOL has been received.

EDIT1 :/

go/src/net/http/request.go

Lines 107 to 109 in 90fb5a4

// Go's HTTP client does not support sending a request with
// the CONNECT method. See the documentation on Transport for
// details.

EDIT2, so this seems to be on purpose

#22554 (comment)

So most users of CONNECT (including Go's build infrastructure in: https://github.com/golang/build/blob/ce623a5/cmd/buildlet/reverse.go#L213) either fmt.Fprintf the CONNECT request themselves, or do *http.Request.Write it to a self-dialed net.Conn, rather than using *http.Transport.

  1. Bootstrap the websocket protocol

HTTP/2 WebSockets need a way to read from and write to the HTTP/2 stream for a request, not a way to hijack the connection.

@aojea
Copy link

@aojea aojea commented Dec 22, 2021

I think that this will only require adding a new option on the http2 server to enable the SETTINGS_ENABLE_CONNECT_PROTOCOL option and another one in the client to let the user know if that setting is enable.

If I understand correctly how CONNECT works in golang, the rest of the logic can be done at a higher level in a similar way that is being done with https://github.com/golang/net/tree/master/websocket

@JeremyLoy
Copy link

@JeremyLoy JeremyLoy commented Jan 7, 2022

Considering the gorilla/websocket library is currently unmaintained, which is causing a lot of concern in the community, I think this is a great opportunity to solidify and complete websocket support in the standard library

@aojea
Copy link

@aojea aojea commented Jan 9, 2022

I did a prototype https://github.com/aojea/net/pull/1/files to get a better idea on what will be required to implement this.

My main concern is that golang seems to avoid implementing the CONNECT semantics in the stdlib #22554 (comment), leaving the implementation to the users.
The requirement of the Extended CONNECT Method will require to add new CONNECT semantics (including a new pseudoheader) to the stdlib, I don't know what the golang team think about this 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Proposals
Incoming
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants