diff --git a/internal/testenv/pagesdeploy_test.go b/internal/testenv/pagesdeploy_test.go index cb65782a..1607ebfb 100644 --- a/internal/testenv/pagesdeploy_test.go +++ b/internal/testenv/pagesdeploy_test.go @@ -536,3 +536,72 @@ func TestTheDocumentedDNSRecordsNameTheHostsTheSiteIsBuiltFor(t *testing.T) { } } } + +// Cloudflare Workers Static Assets CONCATENATES header values across matching +// _headers rules. Cloudflare Pages replaced them. This file was written for +// Pages, and its own comment said so: "a later rule overrides an earlier one for +// the same header name, which is why the cache rules come after the catch-all." +// +// After the migration to Workers that stopped being true, and the live site +// served every content-hashed asset as +// +// Cache-Control: no-cache, public, max-age=31536000, immutable +// +// where RFC 9111 gives `no-cache` the last word. Eleven of eleven assets +// revalidated on every navigation, and the render-blocking stylesheet cost about +// 588ms in front of first paint each time. Nothing failed; it was merely slow, +// which is why it took a measurement rather than a test to find. +// +// The rule this enforces is narrow and mechanical: no two rules may set the same +// header, because on Workers the second does not win -- it joins. +// +// Proven able to fail against the committed tree by adding `Cache-Control: +// no-cache` back to the `/*` block: this reports /_astro/* and /* both setting +// Cache-Control. +func TestNoTwoHeaderRulesSetTheSameHeader(t *testing.T) { + raw := readRepoFile(t, "web", "public/_headers") + + var path string + // header name -> the paths that set it + setters := map[string][]string{} + for _, line := range strings.Split(raw, "\n") { + trimmed := strings.TrimSpace(line) + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + if !strings.HasPrefix(line, " ") && !strings.HasPrefix(line, "\t") { + path = trimmed + continue + } + name, _, ok := strings.Cut(trimmed, ":") + if !ok { + continue + } + name = strings.ToLower(strings.TrimSpace(name)) + setters[name] = append(setters[name], path) + } + if len(setters) == 0 { + t.Fatal("no header rules parsed out of web/public/_headers; this guard " + + "would pass just as happily on an empty file") + } + + for name, paths := range setters { + // A header set by several DISJOINT path rules is fine -- /fonts/* and + // /shots/* both setting Cache-Control never match one request. What is + // not fine is a rule that also matches everything. + hasCatchAll := false + for _, p := range paths { + if p == "/*" { + hasCatchAll = true + } + } + if hasCatchAll && len(paths) > 1 { + t.Errorf("%q is set on /* AND on %v.\n"+ + "On Workers Static Assets those values are CONCATENATED, not "+ + "replaced, so the /* value survives into every more specific "+ + "rule. For Cache-Control that means a `no-cache` on /* defeats "+ + "every `immutable` below it.", + name, paths) + } + } +} diff --git a/web/public/_headers b/web/public/_headers index a380cabd..4bb7d611 100644 --- a/web/public/_headers +++ b/web/public/_headers @@ -13,14 +13,34 @@ # side only. While both files exist they must say the same thing. # # Astro copies public/ verbatim into dist/, so this lands at dist/_headers, which -# is where Pages looks. Pages consumes it and does not serve it as an asset. +# is where Cloudflare looks. It is consumed rather than served as an asset. # -# Rules are applied in order and a later rule overrides an earlier one for the -# same header name, which is why the cache rules come after the catch-all. +# WORKERS APPENDS, PAGES OVERRODE, AND THAT DIFFERENCE WAS A LIVE BUG. +# +# This file used to set `Cache-Control: no-cache` on /* and rely on the later, +# more specific rules replacing it. That is Pages behaviour. On Workers Static +# Assets the values are CONCATENATED, so a hashed asset was served as +# +# Cache-Control: no-cache, public, max-age=31536000, immutable +# +# and RFC 9111 gives `no-cache` the last word: revalidate before use. Every +# immutable asset asked the origin again on every navigation. Measured on the +# live site: 11 of 11 assets returning 304, and the render-blocking stylesheet +# costing ~588ms in front of first paint on each internal navigation. +# +# So no rule may set a header that a later rule also sets. The security headers +# stay on /* because they belong on everything; Cache-Control is now stated once +# per path class, and the HTML rule is scoped to the HTML rather than to /*. /* X-Content-Type-Options: nosniff Referrer-Policy: strict-origin-when-cross-origin Content-Security-Policy: default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-inline'; font-src 'self'; base-uri 'self'; frame-ancestors 'none' + +# HTML must revalidate, or a deploy is invisible until caches expire. Scoped to +# the built pages rather than /*, so it cannot leak onto a hashed asset. +/ + Cache-Control: no-cache +/*.html Cache-Control: no-cache # Content-hashed by the build, so it can be cached hard and forever. Same