Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 69 additions & 0 deletions internal/testenv/pagesdeploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -536,3 +536,72 @@
}
}
}

// 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) {

Check failure on line 561 in internal/testenv/pagesdeploy_test.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 19 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=rainmanjam_polyemesis&issues=AZ_-Qu9ifOdhf-t9yPjU&open=AZ_-Qu9ifOdhf-t9yPjU&pullRequest=334
Comment on lines +555 to +561
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)
}
}
}
26 changes: 23 additions & 3 deletions web/public/_headers
Original file line number Diff line number Diff line change
Expand Up @@ -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 /*.
Comment on lines +31 to +33
/*
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
Expand Down
Loading