caddyhttp: use the new http.Protocols to handle h1, h2 and h2c requests #6961
Conversation
|
Can you link the relevant Go PRs/commits? Curious to see how that evolved. |
| // enabling h2c because some listener wrapper will wrap the connection that is no longer *tls.Conn | ||
| // However, we need to handle the case that if the connection is h2c but h2c is not enabled. We identify | ||
| // this type of connection by checking if it's behind a TLS listener wrapper or if it implements tls.ConnectionState. | ||
| srv.server.Protocols.SetUnencryptedHTTP2(true) |
There was a problem hiding this comment.
Shouldn't this only be set if h2c, but not h2?
There was a problem hiding this comment.
This is deliberate. if the handshake succeeds with h2, it will be handled accordingly. h2c is enabled to handle listener wrappers.
There was a problem hiding this comment.
But if the user only configured h2 and not h2c, it'll still have SetUnencryptedHTTP2(true), right? That doesn't seem correct.
There was a problem hiding this comment.
Or wait, you mean if there's LWs then it could appear as though the conn is no longer encrypted cause it's already been decrypted? Is that what you mean? Tricky.
| @@ -264,18 +263,6 @@ type Server struct { | |||
|
|
|||
| // ServeHTTP is the entry point for all HTTP requests. | |||
| func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |||
| // If there are listener wrappers that process tls connections but don't return a *tls.Conn, this field will be nil. | |||
| // TODO: Can be removed if https://github.com/golang/go/pull/56110 is ever merged. | |||
There was a problem hiding this comment.
Whoa, 3 years later, it's finally in! 👏 Is it too early to drop this though, isn't it only in Go 1.25?
There was a problem hiding this comment.
That's why I leave this as a draft. I have yet to test this thoroughly.
There was a problem hiding this comment.
Yeah, this should be left in for now. Other parts seem fine though.
|
Is there anything in reverseproxy that can be simplified with the new h2c stuff? Or did they only improve the server side but not client side? |
|
I want to wait until 6955 is merged to work on the reverse proxy protocol stuff. |
WeidiDeng
force-pushed
the
server-h2-handling-using-protocols
branch
from
aa4bb70 to
e5ca8db
Compare
|
This is still in draft state; would you like another review? |
WeidiDeng
force-pushed
the
server-h2-handling-using-protocols
branch
from
42fb301 to
c28bab9
Compare
WeidiDeng
force-pushed
the
server-h2-handling-using-protocols
branch
from
4158f9e to
a76d005
Compare
…ts (caddyserver#6961) * use the new http.Protocols to handle h1, h2 and h2c requests * fix lint * keep ConnCtxKey for now * fix handling for h2c * check http version while reading the connection * check if connection implements connectionStater when it should * add comments about either h1 or h2 must be used in the listener * fix if check * return a net.Conn that implements connectionStater if applicable * remove http/1.1 from alpn if h1 is disabled * fix matching if only h1 is enabled --------- Co-authored-by: Matt Holt <[email protected]>
Golang 1.24 introduced a new type that can be used to specify the versions of http caddy can handle. Now caddy can support h2/h2c without h1.
This new mechanism also means graceful shutdown for h2 is easier and we no longer need to track the number of h2 connections. It does require another hack to prevent unintentional h2c request handling, but that hack is minimal.
H2c via protocol upgrade is no longer supported. As far as I know, only curl with use this upgrade if
--http2-prior-knowledgeis not specified.