caddy icon indicating copy to clipboard operation
caddy copied to clipboard

Enable HTTP/3 by default

Open mholt opened this issue 3 years ago • 7 comments

This PR enables HTTP/3 by default (#3833), and implements the AcceptToken callback as recommended in #3055.

~~The AcceptToken callback is only simulated, however, not using actual load data yet.~~ (UPDATE: Now it's using a very simple load statistic.) @marten-seemann: am I on the right track? Do I have the basic idea correct? I am a little fuzzy on the semantics here, so if you could double-check that I have it right, I can find a way to determine load statistics.

Also removes experimental_http3 options/configuration except it does leave it in the JSON config for now, but marks it as deprecated.

Still need to add a way to disable HTTP/3, probably through the use of the existing protocols configuration parameter.

We will likely merge this sometime after the v2.5.0 release.

mholt avatar Apr 15 '22 16:04 mholt

@marten-seemann I went ahead and implemented a very simple load statistic (number of currently active requests). Maybe it's not great, but it's so dang cheap and simple that I want to start with it and see how it goes. Let me know if I'm on the right track in terms of implementation, and making sure I have the logic right for AcceptToken. Thanks!

Edit: Or, I wonder if the logic should be more like this (i.e. only use load statistic when it's a RetryToken specifically)?

AcceptToken: func(clientAddr net.Addr, token *quic.Token) bool {
	if token == nil {
		return false
	}
	if token.IsRetryToken && activeRequests != nil {
		highLoad := atomic.LoadInt64(activeRequests) > 1000 // TODO: make tunable
		return highLoad
	}
	return true
},

mholt avatar Apr 15 '22 17:04 mholt

I've pushed a commit that polishes this feature a bit more:

  • Deprecate the server { protocol sub-option in global options, including allow_h2c and strict_sni_host.
  • Move strict_sni_host to sub-option of server
  • Add server { protocols sub-option, which allows setting which HTTP versions to enable: h1 h2 h2c h3 are allowed values.

This lets the user toggle precisely which HTTP versions they want to support, including HTTP/3-only, which is currently our oldest open issue: #1614. (Yay!)

Because the Go standard library does not let us disable HTTP/1.1, we would probably have to implement our own server to support only HTTP/2 (or h2c) without HTTP/1.1 fallback. I actually tried it:

// http2Server is a server that speaks only HTTP/2.
type http2Server struct {
	h1server *http.Server
	server   *http2.Server
	handler  http.Handler
	logger   *zap.Logger
}

func (h2 http2Server) Serve(ln net.Listener) error {
	h2.server = new(http2.Server)
	opts := &http2.ServeConnOpts{
		BaseConfig: h2.h1server,
		Handler:    h2.handler,
	}
	for {
		conn, err := ln.Accept()
		if err != nil {
			return err
		}
		go h2.serve(conn, opts)
	}
}

func (h2 http2Server) serve(conn net.Conn, opts *http2.ServeConnOpts) {
	defer func() {
		if r := recover(); r != nil {
			buf := make([]byte, 64<<10)
			buf = buf[:runtime.Stack(buf, false)]
			h2.logger.Error("panic",
				zap.String("remote", conn.RemoteAddr().String()),
				zap.Any("error", r),
				zap.String("stack", string(buf)))
		}
	}()
	defer conn.Close()
	h2.server.ServeConn(conn, opts)
}

but I could not get it to work. Curl complained, curl: (1) Received HTTP/0.9 when not allowed - which I don't understand, but I didn't spend much time looking into why. Anyway, this seems like a huge maintenance burden & undertaking, so I decided to not support HTTP/2-only servers for now. However, HTTP/1-only and HTTP/3-only configurations work fine!

This means HTTP/3 can be disabled, and we've made protocol configuration easy and consistent.


In summary: I believe have finished this feature, all except for the AcceptToken tweak mentioned above. (I would still like quic.defaultAcceptToken() either exported or always called even if we set our own. /cc @marten-seemann :smiley:)

Once that is taken care of, I think we can merge this and ship Caddy with HTTP/3 enabled by default.

mholt avatar Aug 03 '22 20:08 mholt

but I could not get it to work. Curl complained, curl: (1) Received HTTP/0.9 when not allowed - which I don't understand, but did I spend much time looking into why.

try request with curl --http2-prior-knowledge, curl will try to do http upgrade from http1 first even with --http2 option. h2c server's response has no http1 header, will be treated as http0.9. I tested the code, it works on my machine.

image

TNQOYxNU avatar Aug 03 '22 21:08 TNQOYxNU

@TNQOYxNU Ah, that's good to know! Thank you. I will give that a try next time I revisit this (soon).

mholt avatar Aug 04 '22 00:08 mholt

Linking this upstream issue as it is the last item remaining before we merge: https://github.com/lucas-clemente/quic-go/issues/3494 -- really appreciate how thoughtful Marten is being about it. :slightly_smiling_face:

mholt avatar Aug 04 '22 18:08 mholt

https://github.com/lucas-clemente/quic-go/issues/2877 sounds like a possible bottleneck for servers with more traffic.

bt90 avatar Aug 10 '22 05:08 bt90

@bt90 Possibly! Although the upstream blockers appear to have been merged since then, so it's probably just a matter of time.

mholt avatar Aug 10 '22 05:08 mholt

This branch is ready for final checks; I think we are ready to merge. Thank you @marten-seemann for your help and congrats on the progress!

mholt avatar Aug 13 '22 17:08 mholt

@TNQOYxNU I will go ahead and merge this as a starting point. If we have requests for HTTP/2-only servers that have a legitimate need, we can revisit that feature. But for now I'll just see how it goes. Thanks again for verifying that!

mholt avatar Aug 15 '22 18:08 mholt