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

net/http: new protocol registration mechanism #69649

Open
Tracked by #67810
neild opened this issue Sep 26, 2024 · 2 comments
Open
Tracked by #67810

net/http: new protocol registration mechanism #69649

neild opened this issue Sep 26, 2024 · 2 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@neild
Copy link
Contributor

neild commented Sep 26, 2024

The following is not quite a proposal. It is a declaration of an intent to commit shenanigans, and an apology (in the sense of "a reasoned argument or justification of something") for them.

The net/http package allows an external package (practically: golang.org/x/net/http2) to register an HTTP/2 server and client implementation with it. (This is Server.TLSNextProto and Transport.TLSNextProto, plus Transport.RegisterProtocol.)

The current registration mechanism is cumbersome and can't easily be extended to support some features we want to add to the package. For example, we want to add support for unencrypted HTTP/2 (#67816), but the current extension mechanism assumes all HTTP/2 connections are a *tls.Conn. We have no way to pass an unencrypted net.Conn from net/http to the HTTP/2 implementation.

We have a plan to move x/net/http2 into std (#67810), but this involves a complex sequence of steps in which adding unencrypted HTTP/2 support is supposed to occur before the package move.

Therefore, it would be very convenient in the short term to have a better connection between net/http and golang.org/x/net/http2.

Ideally, this connection would be extensible if/when we discover additional ways the two packages need to communicate. It should also add little to no exported API surface to net/http, since it will have few-to-no users.

I propose, therefore, to add the following two unexported functions to net/http, using //go:linkname to make them visible to x/net/http2 (and only x/net/http2):

//go:linkname serverRegisterProtocolImplementation golang.org/x/net/http2.nethttp_serverRegisterProtocolImplementation
func serverRegisterProtocolImplementation(s *Server, proto string, impl any) error

//go:linkname transportRegisterProtocolImplementation golang.org/x/net/http2.nethttp_transportRegisterProtocolImplementation
func transportRegisterProtocolImplementation(t *Transport, proto string, impl any) error

This is implemented in https://go.dev/cl/616097 (net/http) and https://go.dev/cl/616118 (x/net/http2).

The interface passed in the impl parameters is fiddly, low-level, contains no user-serviceable parts, and is subject to change to in the future. (We pass it as an "any" to make it easier to evolve if necessary.) See the above CLs for the details.

It is likely that we will want to expose a user-visible protocol registration mechanism in the future to support HTTP/3, since there exists at least one existing third-party HTTP/3 implementation. We could do this by converting the above unexported functions to exported methods and defining an appropriate interface for HTTP/3 server/client implementations:

func (*Server) RegisterProtocolImplementation(proto string, impl any)
func (*Transport) RegisterProtocolImplementation(proto string, impl any)

That's a separate proposal, though.

@neild
Copy link
Contributor Author

neild commented Sep 26, 2024

/cc @marten-seemann because this is a step in the direction of defining a good way for HTTP/3 implementations to register themselves with net/http.

@mknyszek mknyszek added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 30, 2024
@mknyszek mknyszek added this to the Backlog milestone Sep 30, 2024
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 30, 2024
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants