Skip to content

caddyhttp: Enable HTTP/3 by default#4707

Merged
mholt merged 8 commits into
masterfrom
http3-default
Aug 15, 2022
Merged

caddyhttp: Enable HTTP/3 by default#4707
mholt merged 8 commits into
masterfrom
http3-default

Conversation

@mholt

@mholt mholt commented Apr 15, 2022

Copy link
Copy Markdown
Member

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

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 mholt added in progress 🏃‍♂️ Being actively worked on under review 🧐 Review is pending before merging do not merge ⛔ Not ready yet! labels Apr 15, 2022
@mholt mholt self-assigned this Apr 15, 2022
@mholt

mholt commented Apr 15, 2022

Copy link
Copy Markdown
Member Author

@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
},

Comment thread modules/caddyhttp/app.go

@marten-seemann marten-seemann left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super excited to see this happening! Let me know if you need anything else, happy to help!

Comment thread listeners.go Outdated
@mholt mholt added this to the v2.6.0 milestone Jul 25, 2022
@mholt

mholt commented Aug 3, 2022

Copy link
Copy Markdown
Member Author

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 😃)

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

@mholt mholt mentioned this pull request Aug 3, 2022
@TNQOYxNU

TNQOYxNU commented Aug 3, 2022

Copy link
Copy Markdown

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

@mholt

mholt commented Aug 4, 2022

Copy link
Copy Markdown
Member Author

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

Comment thread caddyconfig/httpcaddyfile/serveroptions.go
@mholt

mholt commented Aug 4, 2022

Copy link
Copy Markdown
Member Author

Linking this upstream issue as it is the last item remaining before we merge: quic-go/quic-go#3494 -- really appreciate how thoughtful Marten is being about it. 🙂

@bt90

bt90 commented Aug 10, 2022

Copy link
Copy Markdown
Contributor

quic-go/quic-go#2877 sounds like a possible bottleneck for servers with more traffic.

@mholt

mholt commented Aug 10, 2022

Copy link
Copy Markdown
Member Author

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

@mholt

mholt commented Aug 13, 2022

Copy link
Copy Markdown
Member Author

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 mholt marked this pull request as ready for review August 13, 2022 17:35
@mholt mholt removed the in progress 🏃‍♂️ Being actively worked on label Aug 13, 2022
@mholt

mholt commented Aug 15, 2022

Copy link
Copy Markdown
Member Author

@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 mholt changed the title Enable HTTP/3 by default caddyhttp: Enable HTTP/3 by default Aug 15, 2022
@mholt mholt merged commit c79c086 into master Aug 15, 2022
@mholt mholt deleted the http3-default branch August 15, 2022 18:02
@mholt mholt added feature ⚙️ New feature or request and removed under review 🧐 Review is pending before merging labels Aug 15, 2022
WilczynskiT pushed a commit to WilczynskiT/caddy that referenced this pull request Aug 17, 2022
@francislavoie francislavoie modified the milestones: v2.6.0, v2.6.0-beta.1 Aug 21, 2022
@mholt mholt modified the milestones: v2.6.0-beta.1, v2.6.0 Sep 13, 2022
toby-griffiths added a commit to toby-griffiths/api-platform that referenced this pull request Sep 21, 2022
The experimental_http3 server protocol option is now failing in
Caddyfile as the option has been removed.

HTTP/3 support is now standard.

See caddyserver/caddy#4707
dunglas pushed a commit to api-platform/api-platform that referenced this pull request Sep 21, 2022
* Removes experimental_http3 as now standard

The experimental_http3 server protocol option is now failing in
Caddyfile as the option has been removed.

HTTP/3 support is now standard.

See caddyserver/caddy#4707

* Removes entire server block & comment

This block is now, no longer required.
xiayesuifeng added a commit to xiayesuifeng/gopanel that referenced this pull request Sep 23, 2022
DarkKirb added a commit to DarkKirb/nixos-config that referenced this pull request Sep 23, 2022
cool-software pushed a commit to devscope-cmd/Rest-API that referenced this pull request Jun 30, 2025
* Removes experimental_http3 as now standard

The experimental_http3 server protocol option is now failing in
Caddyfile as the option has been removed.

HTTP/3 support is now standard.

See caddyserver/caddy#4707

* Removes entire server block & comment

This block is now, no longer required.
dmytromokliak added a commit to dmytromokliak/API-management-platform that referenced this pull request Sep 16, 2025
* Removes experimental_http3 as now standard

The experimental_http3 server protocol option is now failing in
Caddyfile as the option has been removed.

HTTP/3 support is now standard.

See caddyserver/caddy#4707

* Removes entire server block & comment

This block is now, no longer required.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature ⚙️ New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants