From 0125ae39cccfdf9b6fdfb16d5a59f3ad37a2caf6 Mon Sep 17 00:00:00 2001 From: Brett Bethke <10068296+bb4242@users.noreply.github.com> Date: Wed, 20 May 2026 01:19:11 -0500 Subject: [PATCH 1/3] caddyhttp: omit Last-Modified for unusable mod times (#7740) See #5548 and #7730 --- modules/caddyhttp/fileserver/staticfiles.go | 25 ++++++++- .../caddyhttp/fileserver/staticfiles_test.go | 56 +++++++++++++++++++ .../caddyhttp/fileserver/testdata/modtime.txt | 0 3 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 modules/caddyhttp/fileserver/testdata/modtime.txt diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go index 507321ad6..70fbd6192 100644 --- a/modules/caddyhttp/fileserver/staticfiles.go +++ b/modules/caddyhttp/fileserver/staticfiles.go @@ -29,6 +29,7 @@ import ( "runtime" "strconv" "strings" + "time" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -579,7 +580,17 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c // that errors generated by ServeContent are written immediately // to the response, so we cannot handle them (but errors there // are rare) - http.ServeContent(w, r, info.Name(), info.ModTime(), file.(io.ReadSeeker)) + // + // There are a few file modification times that aren't useful + // to send in Last-Modified headers, but the golang http library only + // omits Last-Modified headers for the Unix epoch time. So, force + // the modification time to the epoch time if it's not useful. + zeroTime := time.Time{} + modTime := info.ModTime() + if !usefulModTime(modTime) { + modTime = zeroTime + } + http.ServeContent(w, r, info.Name(), modTime, file.(io.ReadSeeker)) return nil } @@ -726,6 +737,14 @@ func (fsrv *FileServer) notFound(w http.ResponseWriter, r *http.Request, next ca return caddyhttp.Error(http.StatusNotFound, nil) } +// Indicates whether a file's modification time is useful for validator +// generation purposes (i.e. inclusion in ETag and Last-Modified headers). +// See issues #5548 and #7730. +func usefulModTime(modTime time.Time) bool { + mtimeunix := modTime.Unix() + return mtimeunix != 0 && mtimeunix != 1 +} + // calculateEtag computes an entity tag using a strong validator // without consuming the contents of the file. It requires the // file info contain the correct size and modification time. @@ -743,8 +762,8 @@ func (fsrv *FileServer) notFound(w http.ResponseWriter, r *http.Request, next ca // which we consider precise enough to qualify as a strong validator. func calculateEtag(d os.FileInfo) string { mtime := d.ModTime() - if mtimeUnix := mtime.Unix(); mtimeUnix == 0 || mtimeUnix == 1 { - return "" // not useful anyway; see issue #5548 + if !usefulModTime(mtime) { + return "" } var sb strings.Builder sb.WriteRune('"') diff --git a/modules/caddyhttp/fileserver/staticfiles_test.go b/modules/caddyhttp/fileserver/staticfiles_test.go index 5d6133c73..5d3bcbd06 100644 --- a/modules/caddyhttp/fileserver/staticfiles_test.go +++ b/modules/caddyhttp/fileserver/staticfiles_test.go @@ -15,10 +15,17 @@ package fileserver import ( + "context" + "net/http" + "net/http/httptest" + "os" "path/filepath" "runtime" "strings" "testing" + "time" + + "github.com/caddyserver/caddy/v2" ) func TestFileHidden(t *testing.T) { @@ -128,3 +135,52 @@ func TestFileHidden(t *testing.T) { } } } + +// Check to make sure that we don't serve ETag and Last-Modified headers +// for files with invalid modification times +func TestModTimeHeaders(t *testing.T) { + check_validator_headers(time.Now(), true, t) + check_validator_headers(time.Unix(0, 0), false, t) + check_validator_headers(time.Unix(1, 0), false, t) + check_validator_headers(time.Unix(2, 0), true, t) +} + +func check_validator_headers(modTime time.Time, expect_headers bool, t *testing.T) { + f := false + fsrv := FileServer{ + Root: "./testdata", + CanonicalURIs: &f, + } + w := httptest.NewRecorder() + r, err := http.NewRequest("GET", "/modtime.txt", nil) + if err != nil { + t.Fatal(err) + } + repl := caddy.NewReplacer() + ctx := context.WithValue(r.Context(), caddy.ReplacerCtxKey, repl) + r = r.WithContext(ctx) + + ctx2, _ := caddy.NewContext(caddy.Context{Context: context.Background()}) // module will be nil by default + fsrv.Provision(ctx2) + + path := "testdata/modtime.txt" + os.Chtimes(path, modTime, modTime) + + fsrv.ServeHTTP(w, r, nil) + + if expect_headers { + if w.Header().Get("ETag") == "" { + t.Errorf("Didn't get ETag header for file with valid mod time %s", modTime) + } + if w.Header().Get("Last-Modified") == "" { + t.Errorf("Didn't get Last-Modified header for file with valid mod time %s", modTime) + } + } else { + if w.Header().Get("ETag") != "" { + t.Errorf("Got ETag header for file with invalid mod time %s", modTime) + } + if w.Header().Get("Last-Modified") != "" { + t.Errorf("Got Last-Modified header for file with invalid mod time %s", modTime) + } + } +} diff --git a/modules/caddyhttp/fileserver/testdata/modtime.txt b/modules/caddyhttp/fileserver/testdata/modtime.txt new file mode 100644 index 000000000..e69de29bb From 325c244ea71a645c224afa1d0b46296ed76ef9fd Mon Sep 17 00:00:00 2001 From: cbro Date: Wed, 20 May 2026 02:35:40 -0400 Subject: [PATCH 2/3] caddytls: fix TLS state races and ECH rotation retry (#7756) * caddytls: fix data race in session ticket key rotation stayUpdated copies the map header (configs := s.configs) under the lock, then iterates the original map after releasing it. Concurrent calls to register/unregister mutate the same map. Hold the lock for the entire iteration instead. * caddytls: fix data race in AllMatchingCertificates AllMatchingCertificates reads the package-level certCache without acquiring certCacheMu, while Cleanup sets certCache to nil under the write lock. The adjacent HasCertificateForSubject correctly acquires certCacheMu.RLock. Add the missing RLock/RUnlock to match. * caddytls: fix ECH key rotation stopping permanently on error When rotateECHKeys returns an error, the rotation goroutine returns immediately, stopping all future key rotation for the lifetime of the process. Change return to continue, matching the error handling for publishECHConfigs two lines below. --- modules/caddytls/sessiontickets.go | 5 ++--- modules/caddytls/tls.go | 4 +++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/modules/caddytls/sessiontickets.go b/modules/caddytls/sessiontickets.go index bfc5628ac..7ebca4604 100644 --- a/modules/caddytls/sessiontickets.go +++ b/modules/caddytls/sessiontickets.go @@ -137,11 +137,10 @@ func (s *SessionTicketService) stayUpdated() { case newKeys := <-keysChan: s.mu.Lock() s.currentKeys = newKeys - configs := s.configs - s.mu.Unlock() - for cfg := range configs { + for cfg := range s.configs { cfg.SetSessionTicketKeys(newKeys) } + s.mu.Unlock() case <-s.stopChan: return } diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go index 928e109e6..b993cba6e 100644 --- a/modules/caddytls/tls.go +++ b/modules/caddytls/tls.go @@ -440,7 +440,7 @@ func (t *TLS) Start() error { t.EncryptedClientHello.configsMu.Unlock() if err != nil { echLogger.Error("rotating ECH configs failed", zap.Error(err)) - return + continue } err := t.publishECHConfigs(echLogger) if err != nil { @@ -879,6 +879,8 @@ func (t *TLS) getAutomationPolicyForName(name string) *AutomationPolicy { // AllMatchingCertificates returns the list of all certificates in // the cache which could be used to satisfy the given SAN. func AllMatchingCertificates(san string) []certmagic.Certificate { + certCacheMu.RLock() + defer certCacheMu.RUnlock() return certCache.AllMatchingCertificates(san) } From 88037f1666eb9ce1b26453d32dc861cb3a87a4c7 Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Wed, 20 May 2026 16:36:30 +1000 Subject: [PATCH 3/3] chore: clean up wording and typo fixes (#7745) * chore: clean up wording and typo fixes * chore: ASCII -> alphanumeric in lexer for heredoc marker --- caddyconfig/caddyfile/lexer.go | 4 ++-- caddyconfig/caddyfile/lexer_test.go | 2 +- caddyconfig/caddyfile/parse.go | 2 +- caddytest/caddytest.go | 12 ++++++------ .../heredoc_invalid_marker.caddyfiletest | 2 +- caddytest/integration/stream_test.go | 2 +- cmd/commands.go | 2 +- modules/caddyhttp/celmatcher.go | 6 +++--- modules/caddyhttp/celmatcher_test.go | 2 +- modules/caddyhttp/encode/encode.go | 4 ++-- modules/caddyhttp/fileserver/staticfiles.go | 2 +- modules/caddyhttp/http2listener.go | 2 +- modules/caddyhttp/httpredirectlistener.go | 2 +- modules/caddyhttp/reverseproxy/fastcgi/client.go | 10 +++++----- modules/caddyhttp/reverseproxy/healthchecks.go | 2 +- .../caddyhttp/reverseproxy/selectionpolicies_test.go | 4 ++-- modules/caddytls/automation.go | 2 +- modules/logging/filters.go | 4 ++-- 18 files changed, 33 insertions(+), 33 deletions(-) diff --git a/caddyconfig/caddyfile/lexer.go b/caddyconfig/caddyfile/lexer.go index 60dabe43d..40ea2e5f7 100644 --- a/caddyconfig/caddyfile/lexer.go +++ b/caddyconfig/caddyfile/lexer.go @@ -155,7 +155,7 @@ func (l *lexer) next() (bool, error) { // want to keep. if ch == '\n' { if len(val) == 2 { - return false, fmt.Errorf("missing opening heredoc marker on line #%d; must contain only alpha-numeric characters, dashes and underscores; got empty string", l.line) + return false, fmt.Errorf("missing opening heredoc marker on line #%d; must contain only alphanumeric characters, dashes and underscores; got empty string", l.line) } // check if there's too many < @@ -165,7 +165,7 @@ func (l *lexer) next() (bool, error) { heredocMarker = string(val[2:]) if !heredocMarkerRegexp.Match([]byte(heredocMarker)) { - return false, fmt.Errorf("heredoc marker on line #%d must contain only alpha-numeric characters, dashes and underscores; got '%s'", l.line, heredocMarker) + return false, fmt.Errorf("heredoc marker on line #%d must contain only alphanumeric characters, dashes and underscores; got '%s'", l.line, heredocMarker) } inHeredoc = true diff --git a/caddyconfig/caddyfile/lexer_test.go b/caddyconfig/caddyfile/lexer_test.go index 7389af79b..89dde2d9f 100644 --- a/caddyconfig/caddyfile/lexer_test.go +++ b/caddyconfig/caddyfile/lexer_test.go @@ -424,7 +424,7 @@ EOF { input: []byte("not-a-heredoc <<\n"), expectErr: true, - errorMessage: "missing opening heredoc marker on line #1; must contain only alpha-numeric characters, dashes and underscores; got empty string", + errorMessage: "missing opening heredoc marker on line #1; must contain only alphanumeric characters, dashes and underscores; got empty string", }, { input: []byte(`heredoc <<HTTPS redirect on the same +// like an HTTP request, then we perform an HTTP->HTTPS redirect on the same // port as the original connection. func (c *httpRedirectConn) Read(p []byte) (int, error) { if c.once { diff --git a/modules/caddyhttp/reverseproxy/fastcgi/client.go b/modules/caddyhttp/reverseproxy/fastcgi/client.go index 48599c27f..7811ae234 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/client.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/client.go @@ -135,8 +135,8 @@ type client struct { logger *zap.Logger } -// Do made the request and returns a io.Reader that translates the data read -// from fcgi responder out of fcgi packet before returning it. +// Do makes the request and returns an io.Reader that translates the data read +// from the FastCGI responder out of FastCGI packets before returning it. func (c *client) Do(p map[string]string, req io.Reader) (r io.Reader, err error) { // check for CONTENT_LENGTH, since the lack of it or wrong value will cause the backend to hang if clStr, ok := p["CONTENT_LENGTH"]; !ok { @@ -179,7 +179,7 @@ func (c *client) Do(p map[string]string, req io.Reader) (r io.Reader, err error) return r, err } -// clientCloser is a io.ReadCloser. It wraps a io.Reader with a Closer +// clientCloser is an io.ReadCloser. It wraps an io.Reader with a Closer // that closes the client connection. type clientCloser struct { rwc net.Conn @@ -208,8 +208,8 @@ func (f clientCloser) Close() error { return f.rwc.Close() } -// Request returns a HTTP Response with Header and Body -// from fcgi responder +// Request returns an HTTP response with header and body +// from the FastCGI responder. func (c *client) Request(p map[string]string, req io.Reader) (resp *http.Response, err error) { r, err := c.Do(p, req) if err != nil { diff --git a/modules/caddyhttp/reverseproxy/healthchecks.go b/modules/caddyhttp/reverseproxy/healthchecks.go index 73604f916..a737f116e 100644 --- a/modules/caddyhttp/reverseproxy/healthchecks.go +++ b/modules/caddyhttp/reverseproxy/healthchecks.go @@ -522,7 +522,7 @@ func (h *Handler) doActiveHealthCheck(dialInfo DialInfo, hostAddr string, networ body = io.LimitReader(body, h.HealthChecks.Active.MaxSize) } defer func() { - // drain any remaining body so connection could be re-used + // drain any remaining body so connection could be reused _, _ = io.Copy(io.Discard, body) resp.Body.Close() }() diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies_test.go b/modules/caddyhttp/reverseproxy/selectionpolicies_test.go index 580abbdde..f915b1467 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies_test.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies_test.go @@ -568,7 +568,7 @@ func TestQueryHashPolicy(t *testing.T) { pool[1].setHealthy(false) h = queryPolicy.Select(pool, request, nil) if h != nil { - t.Error("Expected query policy policy host to be nil.") + t.Error("Expected query policy host to be nil.") } request = httptest.NewRequest(http.MethodGet, "/?foo=aa11&foo=bb22", nil) @@ -630,7 +630,7 @@ func TestURIHashPolicy(t *testing.T) { pool[1].setHealthy(false) h = uriPolicy.Select(pool, request, nil) if h != nil { - t.Error("Expected uri policy policy host to be nil.") + t.Error("Expected uri policy host to be nil.") } } diff --git a/modules/caddytls/automation.go b/modules/caddytls/automation.go index 5b7a4ed5d..918a58b40 100644 --- a/modules/caddytls/automation.go +++ b/modules/caddytls/automation.go @@ -158,7 +158,7 @@ type AutomationPolicy struct { DisableOCSPStapling bool `json:"disable_ocsp_stapling,omitempty"` // Overrides the URLs of OCSP responders embedded in certificates. - // Each key is a OCSP server URL to override, and its value is the + // Each key is an OCSP server URL to override, and its value is the // replacement. An empty value will disable querying of that server. // EXPERIMENTAL. Subject to change. OCSPOverrides map[string]string `json:"ocsp_overrides,omitempty"` diff --git a/modules/logging/filters.go b/modules/logging/filters.go index 087b872e7..b863e72ea 100644 --- a/modules/logging/filters.go +++ b/modules/logging/filters.go @@ -149,10 +149,10 @@ func (f *ReplaceFilter) Filter(in zapcore.Field) zapcore.Field { // list of IP addresses, where all of the values // will be masked. type IPMaskFilter struct { - // The IPv4 mask, as an subnet size CIDR. + // The IPv4 mask, as a subnet size CIDR. IPv4MaskRaw int `json:"ipv4_cidr,omitempty"` - // The IPv6 mask, as an subnet size CIDR. + // The IPv6 mask, as a subnet size CIDR. IPv6MaskRaw int `json:"ipv6_cidr,omitempty"` v4Mask net.IPMask