-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
caddyhttp: use the new http.Protocols to handle h1, h2 and h2c requests #6961
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
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
f4b57b0
use the new http.Protocols to handle h1, h2 and h2c requests
WeidiDeng ca256e3
fix lint
WeidiDeng 16e184a
keep ConnCtxKey for now
WeidiDeng 6c1d6b0
fix handling for h2c
WeidiDeng c537d9b
check http version while reading the connection
WeidiDeng ff4aae6
check if connection implements connectionStater when it should
WeidiDeng 337fc17
add comments about either h1 or h2 must be used in the listener
WeidiDeng 4c19224
fix if check
WeidiDeng 92ea784
return a net.Conn that implements connectionStater if applicable
WeidiDeng c28bab9
remove http/1.1 from alpn if h1 is disabled
WeidiDeng 4158f9e
fix matching if only h1 is enabled
WeidiDeng b242ca6
Merge branch 'master' into server-h2-handling-using-protocols
WeidiDeng 2293c3f
Merge branch 'master' into server-h2-handling-using-protocols
mholt File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,102 +1,110 @@ | ||
| package caddyhttp | ||
|
|
||
| import ( | ||
| "context" | ||
| "crypto/tls" | ||
| weakrand "math/rand" | ||
| "io" | ||
| "net" | ||
| "net/http" | ||
| "sync/atomic" | ||
| "time" | ||
|
|
||
| "go.uber.org/zap" | ||
| "golang.org/x/net/http2" | ||
| ) | ||
|
|
||
| type connectionStater interface { | ||
| ConnectionState() tls.ConnectionState | ||
| } | ||
|
|
||
| // http2Listener wraps the listener to solve the following problems: | ||
| // 1. server h2 natively without using h2c hack when listener handles tls connection but | ||
| // don't return *tls.Conn | ||
| // 2. graceful shutdown. the shutdown logic is copied from stdlib http.Server, it's an extra maintenance burden but | ||
| // whatever, the shutdown logic maybe extracted to be used with h2c graceful shutdown. http2.Server supports graceful shutdown | ||
| // sending GO_AWAY frame to connected clients, but doesn't track connection status. It requires explicit call of http2.ConfigureServer | ||
| // 1. prevent genuine h2c connections from succeeding if h2c is not enabled | ||
| // and the connection doesn't implment connectionStater or the resulting NegotiatedProtocol | ||
| // isn't http2. | ||
| // This does allow a connection to pass as tls enabled even if it's not, listener wrappers | ||
| // can do this. | ||
| // 2. After wrapping the connection doesn't implement connectionStater, emit a warning so that listener | ||
| // wrapper authors will hopefully implement it. | ||
| // 3. check if the connection matches a specific http version. h2/h2c has a distinct preface. | ||
| type http2Listener struct { | ||
| cnt uint64 | ||
| useTLS bool | ||
| useH1 bool | ||
| useH2 bool | ||
| net.Listener | ||
| server *http.Server | ||
| h2server *http2.Server | ||
| } | ||
|
|
||
| type connectionStateConn interface { | ||
| net.Conn | ||
| ConnectionState() tls.ConnectionState | ||
| logger *zap.Logger | ||
| } | ||
|
|
||
| func (h *http2Listener) Accept() (net.Conn, error) { | ||
| for { | ||
| conn, err := h.Listener.Accept() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| conn, err := h.Listener.Accept() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if csc, ok := conn.(connectionStateConn); ok { | ||
| // *tls.Conn will return empty string because it's only populated after handshake is complete | ||
| if csc.ConnectionState().NegotiatedProtocol == http2.NextProtoTLS { | ||
| go h.serveHttp2(csc) | ||
| continue | ||
| } | ||
| } | ||
| _, isConnectionStater := conn.(connectionStater) | ||
| // emit a warning | ||
| if h.useTLS && !isConnectionStater { | ||
| h.logger.Warn("tls is enabled, but listener wrapper returns a connection that doesn't implement connectionStater") | ||
| } else if !h.useTLS && isConnectionStater { | ||
| h.logger.Warn("tls is disabled, but listener wrapper returns a connection that implements connectionStater") | ||
| } | ||
|
|
||
| // if both h1 and h2 are enabled, we don't need to check the preface | ||
| if h.useH1 && h.useH2 { | ||
| return conn, nil | ||
| } | ||
|
|
||
| // impossible both are false, either useH1 or useH2 must be true, | ||
| // or else the listener wouldn't be created | ||
| h2Conn := &http2Conn{ | ||
| h2Expected: h.useH2, | ||
| Conn: conn, | ||
| } | ||
| if isConnectionStater { | ||
| return http2StateConn{h2Conn}, nil | ||
| } | ||
| return h2Conn, nil | ||
| } | ||
|
|
||
| func (h *http2Listener) serveHttp2(csc connectionStateConn) { | ||
| atomic.AddUint64(&h.cnt, 1) | ||
| h.runHook(csc, http.StateNew) | ||
| defer func() { | ||
| csc.Close() | ||
| atomic.AddUint64(&h.cnt, ^uint64(0)) | ||
| h.runHook(csc, http.StateClosed) | ||
| }() | ||
| h.h2server.ServeConn(csc, &http2.ServeConnOpts{ | ||
| Context: h.server.ConnContext(context.Background(), csc), | ||
| BaseConfig: h.server, | ||
| Handler: h.server.Handler, | ||
| }) | ||
| type http2StateConn struct { | ||
| *http2Conn | ||
| } | ||
|
|
||
| const shutdownPollIntervalMax = 500 * time.Millisecond | ||
| func (conn http2StateConn) ConnectionState() tls.ConnectionState { | ||
| return conn.Conn.(connectionStater).ConnectionState() | ||
| } | ||
|
|
||
| func (h *http2Listener) Shutdown(ctx context.Context) error { | ||
| pollIntervalBase := time.Millisecond | ||
| nextPollInterval := func() time.Duration { | ||
| // Add 10% jitter. | ||
| //nolint:gosec | ||
| interval := pollIntervalBase + time.Duration(weakrand.Intn(int(pollIntervalBase/10))) | ||
| // Double and clamp for next time. | ||
| pollIntervalBase *= 2 | ||
| if pollIntervalBase > shutdownPollIntervalMax { | ||
| pollIntervalBase = shutdownPollIntervalMax | ||
| } | ||
| return interval | ||
| } | ||
| type http2Conn struct { | ||
| // current index where the preface should match, | ||
| // no matching is done if idx is >= len(http2.ClientPreface) | ||
| idx int | ||
| // whether the connection is expected to be h2/h2c | ||
| h2Expected bool | ||
| // log if one such connection is detected | ||
| logger *zap.Logger | ||
| net.Conn | ||
| } | ||
|
|
||
| timer := time.NewTimer(nextPollInterval()) | ||
| defer timer.Stop() | ||
| for { | ||
| if atomic.LoadUint64(&h.cnt) == 0 { | ||
| return nil | ||
| func (c *http2Conn) Read(p []byte) (int, error) { | ||
| if c.idx >= len(http2.ClientPreface) { | ||
| return c.Conn.Read(p) | ||
| } | ||
| n, err := c.Conn.Read(p) | ||
| for i := range n { | ||
| // first mismatch | ||
| if p[i] != http2.ClientPreface[c.idx] { | ||
| // close the connection if h2 is expected | ||
| if c.h2Expected { | ||
| c.logger.Debug("h1 connection detected, but h1 is not enabled") | ||
| _ = c.Conn.Close() | ||
| return 0, io.EOF | ||
| } | ||
| // no need to continue matching anymore | ||
| c.idx = len(http2.ClientPreface) | ||
| return n, err | ||
| } | ||
| select { | ||
| case <-ctx.Done(): | ||
| return ctx.Err() | ||
| case <-timer.C: | ||
| timer.Reset(nextPollInterval()) | ||
| c.idx++ | ||
| // matching complete | ||
| if c.idx == len(http2.ClientPreface) && !c.h2Expected { | ||
| c.logger.Debug("h2/h2c connection detected, but h2/h2c is not enabled") | ||
| _ = c.Conn.Close() | ||
| return 0, io.EOF | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func (h *http2Listener) runHook(conn net.Conn, state http.ConnState) { | ||
| if h.server.ConnState != nil { | ||
| h.server.ConnState(conn, state) | ||
| } | ||
| return n, err | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this only be set if
h2c, but noth2?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is deliberate. if the handshake succeeds with
h2, it will be handled accordingly.h2cis enabled to handle listener wrappers.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But if the user only configured
h2and noth2c, it'll still haveSetUnencryptedHTTP2(true), right? That doesn't seem correct.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.