https://github.com/caddyserver/caddy/pull/4707 Skip to content Toggle navigation Sign up * Product + Actions Automate any workflow + Packages Host and manage packages + Security Find and fix vulnerabilities + Codespaces Instant dev environments + Copilot Write better code with AI + Code review Manage code changes + Issues Plan and track work + Discussions Collaborate outside of code + Explore + All features + Documentation + GitHub Skills + Changelog * Solutions + By Size + Enterprise + Teams + Compare all + By Solution + CI/CD & Automation + DevOps + DevSecOps + Case Studies + Customer Stories + Resources * Open Source + GitHub Sponsors Fund open source developers + The ReadMe Project GitHub community articles + Repositories + Topics + Trending + Collections * Pricing [ ] * # In this repository All GitHub | Jump to | * No suggested jump to results * # In this repository All GitHub | Jump to | * # In this organization All GitHub | Jump to | * # In this repository All GitHub | Jump to | Sign in Sign up {{ message }} caddyserver / caddy Public * * Notifications * Fork 3.4k * Star 42.7k * Code * Issues 79 * Pull requests 15 * Actions * Wiki * Security * Insights More * Code * Issues * Pull requests * Actions * Wiki * Security * Insights 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. Pick a username [ ] Email Address [ ] Password [ ] [ ] Sign up for GitHub 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 Jump to bottom caddyhttp: Enable HTTP/3 by default #4707 Merged mholt merged 8 commits into master from http3-default Aug 15, 2022 Merged caddyhttp: Enable HTTP/3 by default #4707 mholt merged 8 commits into master from http3-default Aug 15, 2022 +210 -191 Conversation 18 Commits 8 Checks 22 Files changed 8 Conversation This file contains 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 Show hidden characters mholt Copy link Member @mholt mholt commented Apr 15, 2022 * edited 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. Sorry, something went wrong. 23 marten-seemann, Plorenzo, 0x1a8510f2, mohammed90, demifiend9, brunojppb, AtomBaf, davidspiess, guillermo, agucova, and 13 more reacted with hooray emoji [?] 10 roland-rollo, dunglas, JeDaYoshi, 0x1a8510f2, rumpelsepp, mohammed90, brunojppb, davidspiess, jeffmccune, and MisterDuval reacted with heart emoji All reactions * 23 reactions * [?] 10 reactions mholt added 2 commits Apr 15, 2022 @mholt caddyhttp: Enable HTTP/3 by default 315308c @mholt core: WIP AcceptToken implementation in ListenQUIC 400ec5d @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 mentioned this pull request Apr 15, 2022 consider enabling QUIC by default #3833 Closed @mholt caddyhttp: Count active requests to optimize QUIC ... 49e30d8 Related to #3055 @mholt Copy link Member Author mholt commented Apr 15, 2022 * edited @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 }, All reactions Sorry, something went wrong. francislavoie francislavoie reviewed Apr 15, 2022 View changes modules/caddyhttp/app.go Show resolved Hide resolved marten-seemann marten-seemann reviewed Apr 15, 2022 View changes Copy link Contributor @marten-seemann marten-seemann left a comment Super excited to see this happening! Let me know if you need anything else, happy to help! Sorry, something went wrong. 1 mholt reacted with thumbs up emoji All reactions * 1 reaction listeners.go Outdated Show resolved Hide resolved @mholt Slightly revise AcceptToken based on feedback 920dd34 @mholt mholt added this to the v2.6.0 milestone Jul 25, 2022 @mholt Merge branch 'master' into http3-default 312458d @mholt mholt force-pushed the http3-default branch from 07233cd to 312458d Compare Aug 3, 2022 @mholt Configurable protocols 481bc51 @mholt Copy link Member Author mholt commented Aug 3, 2022 * edited 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. [?] 3 roland-rollo, 0x1a8510f2, and demifiend9 reacted with heart emoji 2 roland-rollo and Plorenzo reacted with rocket emoji All reactions * [?] 3 reactions * 2 reactions Sorry, something went wrong. @mholt mholt mentioned this pull request Aug 3, 2022 QUIC-only Mode #1614 Closed @TNQOYxNU Copy link TNQOYxNU commented Aug 3, 2022 * edited 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 2 mholt and roland-rollo reacted with hooray emoji [?] 2 mholt and roland-rollo reacted with heart emoji 1 mholt reacted with eyes emoji All reactions * 2 reactions * [?] 2 reactions * 1 reaction Sorry, something went wrong. @mholt Copy link Member Author mholt commented Aug 4, 2022 @TNQOYxNU Ah, that's good to know! Thank you. I will give that a try next time I revisit this (soon). [?] 1 TNQOYxNU reacted with heart emoji All reactions * [?] 1 reaction Sorry, something went wrong. francislavoie francislavoie reviewed Aug 4, 2022 View changes caddyconfig/httpcaddyfile/serveroptions.go Show resolved Hide resolved @mholt Copy link Member Author mholt commented Aug 4, 2022 Linking this upstream issue as it is the last item remaining before we merge: lucas-clemente/quic-go#3494 -- really appreciate how thoughtful Marten is being about it. 1 roland-rollo reacted with rocket emoji All reactions * 1 reaction Sorry, something went wrong. @bt90 Copy link bt90 commented Aug 10, 2022 lucas-clemente/quic-go#2877 sounds like a possible bottleneck for servers with more traffic. All reactions Sorry, something went wrong. @mholt Copy link Member Author mholt commented Aug 10, 2022 @bt90 Possibly! Although the upstream blockers appear to have been merged since then, so it's probably just a matter of time. All reactions Sorry, something went wrong. @mholt Update quic-go, implement RequireAddressValidation 6627ec8 @mholt mholt mentioned this pull request Aug 13, 2022 implement a new API to let servers control client address verification lucas-clemente/quic-go#3501 Merged @mholt mholt removed the do not merge Not ready yet! label Aug 13, 2022 @mholt Merge branch 'master' into http3-default a2f9925 @mholt Copy link Member Author mholt commented Aug 13, 2022 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! [?] 3 marten-seemann, 0x1a8510f2, and lidel reacted with heart emoji All reactions * [?] 3 reactions Sorry, something went wrong. @mholt mholt marked this pull request as ready for review Aug 13, 2022 @mholt mholt removed the in progress Being actively worked on label Aug 13, 2022 @mholt mholt mentioned this pull request Aug 13, 2022 disable QUIC Retries when not under load #3055 Closed @mholt Copy link Member Author mholt commented Aug 15, 2022 @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! All reactions Sorry, something went wrong. @mholt mholt changed the title [DEL:Enable HTTP/3 by default:DEL] [INS:caddyhttp: Enable HTTP/3 by default:INS] Aug 15, 2022 Hide details View details @mholt mholt merged commit c79c086 into master Aug 15, 2022 23 checks passed @mholt mholt deleted the http3-default branch Aug 15, 2022 @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 issue Aug 17, 2022 @mholt @WilczynskiT caddyhttp: Enable HTTP/3 by default (caddyserver#4707) f30a56d @mholt mholt mentioned this pull request Aug 18, 2022 Serve http2 when listener wrapper doesn't return *tls.Conn #4929 Open @francislavoie francislavoie modified the milestones: v2.6.0, v2.6.0-beta.1 Aug 21, 2022 Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment Reviewers @marten-seemann marten-seemann @francislavoie francislavoie Assignees @mholt mholt Labels feature New feature or request Projects None yet Milestone v2.6.0-beta.1 Development Successfully merging this pull request may close these issues. None yet 5 participants @mholt @TNQOYxNU @bt90 @marten-seemann @francislavoie 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. Footer (c) 2022 GitHub, Inc. Footer navigation * Terms * Privacy * Security * Status * Docs * Contact GitHub * Pricing * API * Training * Blog * About You can't perform that action at this time. You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.