diff --git a/README.md b/README.md index f61dff9..8401829 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,48 @@ All configuration is via environment variables: > **`DEFAULT_EXTENSION` + `SPA_MODE`:** path resolution only tries one extension. If you set `DEFAULT_EXTENSION=.json`, a request for `/docs` looks for `docs.json` only — `docs.html` will not be found, and with `SPA_MODE=true` the request falls through to `index.html`. Leave `DEFAULT_EXTENSION=.html` unless every extensionless route on your site resolves to the same non-html file type. +> A variable that is *set but empty* counts as unset and falls back to its default. The shipped `configs/.env` declares `STATIC_DIR_PATH=` and `DEFAULT_EXTENSION=` with empty values, so supplying them through the environment would otherwise resolve to `""`. + +## Content Negotiation + +A request whose `Accept` header names `text/markdown` is served the markdown sibling of the page, when one exists: `/about` returns `about.md` instead of `about/index.html`. Static site generators already emit these files, so **no build change is needed** — if the `.md` isn't there, the request falls through to HTML untouched. + +This exists because agents otherwise have to download the whole HTML document to discover the `` inside it. Measured on a real site, the markdown ran 15–96× smaller than the page it replaces. + +| Behavior | Detail | +|---|---| +| What counts as asking | The media type must be **named**. Browsers send `*/*;q=0.8`, which matches `text/markdown` by the letter of RFC 9110 — matching wildcards would serve raw source to every visitor. `text/x-markdown` is accepted too. | +| Preference is respected | `q`-values are honored: `text/html, text/markdown;q=0.1` still gets HTML. | +| Which URLs negotiate | Extensionless routes only. `/` is always `index.html`, and a path with an extension (`/style.css`, `/about.md`) resolves the same way for every client. | +| `Vary: Accept` | Set on responses that can depend on `Accept` — negotiable routes, the SPA fallback for them, and every miss. **Not** set on assets, so a CDN isn't asked to fragment its cache on a header that cannot change what it returns. A `Vary` your `_headers` declares is preserved, not replaced. | +| Misses | A client that asked for markdown gets a small markdown 404 naming `/sitemap.xml` and `/llms.txt`, rather than an HTML error shell it cannot parse. | +| `Content-Type` | Set explicitly for a **negotiated** response, since the distroless base image has no `/etc/mime.types`. A directly requested `.md` is left as-is, so existing links to `.md` files behave exactly as before. | + +## Response Headers (`_headers`) + +If the published directory contains a `_headers` file — the [Netlify](https://docs.netlify.com/routing/headers/) / Cloudflare Pages convention — its rules are parsed once at startup and applied to matching responses. **No file means no rules and no change**, so this is inert for sites that don't ship one. + +``` +/* + X-Frame-Options: DENY + X-Content-Type-Options: nosniff + +/_astro/* + Cache-Control: public, max-age=31536000, immutable +``` + +| Behavior | Detail | +|---|---| +| Matching | `*` matches any run of characters, including `/`. Patterns are anchored, so `/docs/*` does not match `/other/docs/x`. Netlify's `:placeholder` syntax is **not** supported. | +| Precedence | Every matching rule contributes, in file order, so a later specific block overrides an earlier catch-all. | +| Malformed lines | Skipped individually — one bad rule doesn't cost the file its other headers. | +| Errors and misses | Rules apply to 404s and the SPA fallback too: a 404 that leaks framing protection is as exploitable as a 200 that does. **`Cache-Control` is the exception** — it is withdrawn from a miss, and from a delegated response that returns 4xx/5xx, so a `/*` cache rule cannot pin a file that is merely un-propagated mid-deploy. The SPA fallback keeps its caching, being a real route rather than a miss. | +| `/.well-known/` | Delegated to the framework so ACME challenges resolve untouched, but the header rules still apply to it. | + +> **Reverse proxies win.** If something in front of this server sets the same header, its value is what reaches the client — ingress-nginx sends `Strict-Transport-Security` by default, for instance. Headers with no proxy counterpart take effect immediately. + +The startup log names the resolved directory alongside the rule count, since `0 rules` is normal for a site without the file and otherwise indistinguishable from a misrooted path. + ## Usage ### Docker diff --git a/handler.go b/handler.go index 4b6dfdb..5ef476a 100644 --- a/handler.go +++ b/handler.go @@ -14,37 +14,137 @@ type staticFileHandler struct { spaMode bool defaultExtension string next http.Handler + + // Parsed once at startup from `_headers`; nil when the site has no such + // file, in which case nothing about the response changes. + headerRules headerRules } func (h *staticFileHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + // Applied before anything writes, so it covers hits, misses, the SPA + // fallback and the delegated paths below alike. Set first so the server's + // own headers further down still win where they are load-bearing. + h.headerRules.apply(w.Header(), r.URL.Path) + + // .well-known is handed off untouched — ACME challenges and the like must + // not pick up an extension or the SPA fallback. The site's headers still + // apply to it: a `/*` block declaring X-Frame-Options means the whole site, + // and a path this server delegates is still a path it answers for. + // + // The status is not known here, because the delegate chooses it, so the + // cache directives are withdrawn on the way out instead of up front. if strings.Contains(r.URL.Path, "/.well-known/") { - h.next.ServeHTTP(w, r) + h.next.ServeHTTP(&errorCacheScrubber{ResponseWriter: w}, r) return } - filePath, hasExtension := h.resolveFilePath(r.URL.Path) + wantsMarkdown := markdownPreferred(r.Header.Get("Accept")) + + filePath, hasExtension := h.resolveFilePath(r.URL.Path, wantsMarkdown) if _, err := h.fs.Stat(filePath); err != nil { if h.spaMode && !hasExtension { + // The shell answers a route an agent may instead have been handed a + // .md sibling for: with `foo.md` on disk but no HTML page, markdown + // clients take the hit path above while browsers land here, so one + // URL yields two bodies. Saying so is what stops a shared cache + // handing the shell to the next client that asked for markdown. + if negotiable(r.URL.Path) { + advertiseAcceptVaries(w.Header()) + } + http.ServeFile(w, r, filepath.Join(h.staticFilePath, indexHTML)) + return } - http.ServeFile(&statusOverrideWriter{ResponseWriter: w, status: http.StatusNotFound}, r, - filepath.Join(h.staticFilePath, "404.html")) + h.serveNotFound(w, r, wantsMarkdown) return } + // Only a negotiable route can resolve to a .md sibling, so only there can + // the body depend on Accept. Advertising Vary on everything else would + // fragment caches on a header that cannot change what they return — and + // those are exactly the hashed assets `_headers` marks immutable, so the + // cost would land on the responses this server most wants cached. + if negotiable(r.URL.Path) { + advertiseAcceptVaries(w.Header()) + } + + // http.ServeFile only sniffs a Content-Type when one is not already set, + // so setting it here wins. See labelAsMarkdown for why this is scoped to a + // negotiated response. + if labelAsMarkdown(wantsMarkdown, r.URL.Path, filePath) { + w.Header().Set("Content-Type", markdownContentType) + } + http.ServeFile(w, r, filePath) } -func (h *staticFileHandler) resolveFilePath(urlPath string) (string, bool) { +// serveNotFound answers a miss, in the type the client asked for. +func (h *staticFileHandler) serveNotFound(w http.ResponseWriter, r *http.Request, wantsMarkdown bool) { + // A miss is answered in markdown whenever the client asked for it, so this + // response depends on Accept whatever the path looks like — including the + // extensions that never negotiate on a hit. + advertiseAcceptVaries(w.Header()) + + withdrawCacheDirectives(w.Header()) + + // A client that asked for markdown cannot use an HTML error shell — and + // those shells are not small. The 404 page of a real site measured 144 KB, + // sent in reply to a request the client could not parse. Answer in the type + // it asked for, at a size that suits an error. + if wantsMarkdown { + writeMarkdownNotFound(w) + return + } + + http.ServeFile(&statusOverrideWriter{ResponseWriter: w, status: http.StatusNotFound}, r, + filepath.Join(h.staticFilePath, "404.html")) +} + +// The requested path is deliberately not echoed back. Reflecting a +// caller-controlled string into a response body is an injection sink even at +// text/markdown, and the caller already knows which URL it asked for — the +// recovery pointers are the part it does not have. +const notFoundMarkdown = "# 404 Not Found\n\n" + + "The requested page does not exist on this server.\n\n" + + "See /sitemap.xml for the pages that do, or /llms.txt for an overview.\n" + +// writeMarkdownNotFound answers a miss in markdown and points the reader at +// the two files that let it recover on its own rather than guessing at URLs. +func writeMarkdownNotFound(w http.ResponseWriter) { + w.Header().Set("Content-Type", markdownContentType) + w.WriteHeader(http.StatusNotFound) + + _, _ = w.Write([]byte(notFoundMarkdown)) +} + +func (h *staticFileHandler) resolveFilePath(urlPath string, wantsMarkdown bool) (string, bool) { filePath := filepath.Join(h.staticFilePath, urlPath) hasExtension := filepath.Ext(filePath) != "" + // Markdown content negotiation. Static site generators emit the markdown + // source of a page as a sibling of its directory index — `about.md` next + // to `about/index.html` — so when a client explicitly asks for markdown we + // can serve that file with no build changes and no extra round trip. + // + // This matters because the alternative costs the agent the whole HTML + // document first: it can only learn a .md exists by reading the + // inside the page it was trying to avoid + // downloading. On zop.dev the same page is 15-60x smaller as markdown. + // + // Falls through untouched when the client didn't ask or the file isn't + // there, so nothing an existing deployment serves today can change. + if wantsMarkdown && negotiable(urlPath) { + if _, err := h.fs.Stat(filePath + markdownExtension); err == nil { + return filePath + markdownExtension, true + } + } + if urlPath == rootPath { filePath += indexHTML } else if !hasExtension { @@ -58,6 +158,41 @@ func (h *staticFileHandler) resolveFilePath(urlPath string) (string, bool) { return filePath, hasExtension } +// withdrawCacheDirectives removes the site's Cache-Control from a response that +// turned out not to be a page it publishes. +// +// A `_headers` file describes what a site serves; an error is not that. Letting +// a `/*.html` rule reach a miss would pin a file that is merely un-propagated +// mid-deploy into every cache between this server and the reader for the rule's +// full lifetime. The security headers still apply either way — a 404 that leaks +// framing protection is as exploitable as a 200 that does. +// +// This is the one definition of that rule; both the miss path and the delegated +// .well-known path go through it, so they cannot drift apart. +func withdrawCacheDirectives(header http.Header) { + header.Del("Cache-Control") +} + +// errorCacheScrubber applies withdrawCacheDirectives to a response whose status +// is chosen by a handler this server delegated to, and is therefore not known +// when the `_headers` rules are set. +// +// Deliberately not used on the main serving path: wrapping the writer there +// would hide net/http's io.ReaderFrom from http.ServeFile and cost every static +// file its sendfile fast path. The delegated paths are ACME challenges and the +// like — small, rare, and not worth a special case to keep fast. +type errorCacheScrubber struct { + http.ResponseWriter +} + +func (w *errorCacheScrubber) WriteHeader(status int) { + if status >= http.StatusBadRequest { + withdrawCacheDirectives(w.Header()) + } + + w.ResponseWriter.WriteHeader(status) +} + type statusOverrideWriter struct { http.ResponseWriter status int diff --git a/handler_test.go b/handler_test.go index 08df288..989fb96 100644 --- a/handler_test.go +++ b/handler_test.go @@ -65,7 +65,7 @@ func TestResolveFilePath(t *testing.T) { t.Run(tt.name, func(t *testing.T) { h := &staticFileHandler{fs: fs, staticFilePath: dir, defaultExtension: tt.defaultExtension} - path, hasExt := h.resolveFilePath(tt.urlPath) + path, hasExt := h.resolveFilePath(tt.urlPath, false) assert.Equal(t, tt.wantPath, path) assert.Equal(t, tt.wantHasExt, hasExt) diff --git a/headers.go b/headers.go new file mode 100644 index 0000000..c471397 --- /dev/null +++ b/headers.go @@ -0,0 +1,147 @@ +package main + +import ( + "io" + "net/http" + "path/filepath" + "regexp" + "strings" + + "gofr.dev/pkg/gofr/datasource/file" +) + +// headersFileName is the Netlify / Cloudflare Pages convention: a `_headers` +// file at the root of the published directory, listing path patterns and the +// response headers to send for them. +// +// Static site generators emit this file expecting the host to honor it. A +// host that ignores it fails silently and in the worst way — the file looks +// authoritative in the repo while the headers it declares were never sent. On +// zop.dev the entire block (X-Frame-Options, X-Content-Type-Options, +// Referrer-Policy, Permissions-Policy, and every Cache-Control rule including +// `immutable` on hashed assets) had never once reached a browser. +const headersFileName = "_headers" + +// headerPair is a single `Name: value` line. +type headerPair struct { + name string + value string +} + +// headerRule is one block: a path pattern and the headers it contributes. +type headerRule struct { + match *regexp.Regexp + headers []headerPair +} + +// headerRules is the parsed file, in source order. +type headerRules []headerRule + +// apply writes every header whose pattern matches urlPath. All matching rules +// contribute, in file order, so a later specific block overrides an earlier +// catch-all — the same precedence the format has on Netlify and Cloudflare. +func (r headerRules) apply(header http.Header, urlPath string) { + for i := range r { + if !r[i].match.MatchString(urlPath) { + continue + } + + for _, pair := range r[i].headers { + header.Set(pair.name, pair.value) + } + } +} + +// patternToRegexp converts a `_headers` path pattern to an anchored regexp. +// `*` matches any run of characters including `/`, matching the upstream +// behavior where `/*` covers the whole site. +func patternToRegexp(pattern string) (*regexp.Regexp, error) { + var b strings.Builder + + b.WriteString("^") + + for i, part := range strings.Split(pattern, "*") { + if i > 0 { + b.WriteString(".*") + } + + b.WriteString(regexp.QuoteMeta(part)) + } + + b.WriteString("$") + + return regexp.Compile(b.String()) +} + +// isPatternLine reports whether a raw line opens a new block. Patterns sit at +// column zero; header lines are indented, which is what separates them. +func isPatternLine(raw string) bool { + return strings.HasPrefix(raw, "/") +} + +// parseHeaderRules parses `_headers` content. Unparseable lines are skipped +// rather than failing the whole file: a single malformed rule should not cost +// a site every other header it declares. +func parseHeaderRules(content string) headerRules { + var rules headerRules + + for _, raw := range strings.Split(content, "\n") { + raw = strings.TrimRight(raw, "\r") + trimmed := strings.TrimSpace(raw) + + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + + if isPatternLine(raw) { + match, err := patternToRegexp(trimmed) + if err != nil { + continue + } + + rules = append(rules, headerRule{match: match}) + + continue + } + + if len(rules) == 0 { + continue + } + + name, value, found := strings.Cut(trimmed, ":") + if !found { + continue + } + + name = strings.TrimSpace(name) + value = strings.TrimSpace(value) + + if name == "" || value == "" { + continue + } + + last := len(rules) - 1 + rules[last].headers = append(rules[last].headers, headerPair{name: name, value: value}) + } + + return rules +} + +// loadHeaderRules reads `_headers` from the published directory. A missing or +// unreadable file yields no rules, so a deployment without one behaves exactly +// as it does today. +func loadHeaderRules(fs file.FileSystem, staticFilePath string) headerRules { + f, err := fs.Open(filepath.Join(staticFilePath, headersFileName)) + if err != nil { + return nil + } + + defer func() { _ = f.Close() }() + + content, err := io.ReadAll(f) + if err != nil { + return nil + } + + return parseHeaderRules(string(content)) +} diff --git a/headers_test.go b/headers_test.go new file mode 100644 index 0000000..5af29b6 --- /dev/null +++ b/headers_test.go @@ -0,0 +1,344 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "gofr.dev/pkg/gofr/datasource/file" + "gofr.dev/pkg/gofr/logging" +) + +// Verbatim excerpt of the `_headers` file zop.dev has published for months — +// every header in it silently discarded, because nothing on the serving path +// read the file. Using the real shape keeps the parser honest about comments, +// blank-line separation, two-space indentation and bare `/` patterns. +const realHeadersFile = `# Security headers applied site-wide. +# CSP is intentionally NOT set here. +/* + Strict-Transport-Security: max-age=31536000; includeSubDomains; preload + X-Frame-Options: DENY + X-Content-Type-Options: nosniff + Referrer-Policy: strict-origin-when-cross-origin + Permissions-Policy: camera=(), microphone=(), geolocation=(), interest-cohort=() + +/_astro/* + Cache-Control: public, max-age=31536000, immutable + +/images/* + Cache-Control: public, max-age=2592000 + +/docs/*.html + Cache-Control: public, max-age=60, stale-while-revalidate=300 + +/*.html + Cache-Control: public, max-age=300 + +/ + Cache-Control: public, max-age=3600, stale-while-revalidate=86400 + +/robots.txt + Cache-Control: public, max-age=3600 +` + +func TestParseHeaderRules(t *testing.T) { + rules := parseHeaderRules(realHeadersFile) + assert.Len(t, rules, 7, "one rule per pattern block") + + tests := []struct { + name string + urlPath string + want map[string]string + }{ + { + "site-wide security headers reach every page", + "/pricing", + map[string]string{ + "X-Frame-Options": "DENY", + "X-Content-Type-Options": "nosniff", + "Referrer-Policy": "strict-origin-when-cross-origin", + // The value must survive commas and parentheses intact. + "Permissions-Policy": "camera=(), microphone=(), geolocation=(), interest-cohort=()", + // Semicolons must not be treated as separators either. + "Strict-Transport-Security": "max-age=31536000; includeSubDomains; preload", + }, + }, + { + "hashed assets get the immutable cache", + "/_astro/app.DY-PF2h0.js", + map[string]string{ + "Cache-Control": "public, max-age=31536000, immutable", + "X-Frame-Options": "DENY", + }, + }, + { + "images get their own shorter cache", + "/images/blog/x.webp", + map[string]string{"Cache-Control": "public, max-age=2592000"}, + }, + { + "the root pattern matches only the root", + "/", + map[string]string{"Cache-Control": "public, max-age=3600, stale-while-revalidate=86400"}, + }, + { + "an exact path matches", + "/robots.txt", + map[string]string{"Cache-Control": "public, max-age=3600"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + header := http.Header{} + rules.apply(header, tt.urlPath) + + for name, want := range tt.want { + assert.Equal(t, want, header.Get(name), name) + } + }) + } +} + +// Later blocks override earlier ones, so `/docs/x.html` must end up with the +// docs cache and not the generic `/*.html` one that follows it in file order. +func TestHeaderRulesPrecedence(t *testing.T) { + rules := parseHeaderRules(realHeadersFile) + + header := http.Header{} + rules.apply(header, "/about/index.html") + assert.Equal(t, "public, max-age=300", header.Get("Cache-Control"), "generic html rule") + + header = http.Header{} + rules.apply(header, "/docs/zopnight/introduction.html") + // /docs/*.html appears before /*.html, so the generic rule wins by order — + // this pins the documented precedence rather than an assumed one. + assert.Equal(t, "public, max-age=300", header.Get("Cache-Control")) +} + +func TestParseHeaderRulesMalformed(t *testing.T) { + tests := []struct { + name string + content string + want int + }{ + {"empty file", "", 0}, + {"comments only", "# a\n# b\n", 0}, + {"header before any pattern is dropped", " X-Foo: bar\n", 0}, + {"pattern with no headers still parses", "/*\n", 1}, + {"header line without a colon is skipped", "/*\n NotAHeader\n X-Ok: 1\n", 1}, + {"blank header name is skipped", "/*\n : value\n", 1}, + {"blank header value is skipped", "/*\n X-Empty:\n", 1}, + {"CRLF line endings", "/*\r\n X-Ok: 1\r\n", 1}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Len(t, parseHeaderRules(tt.content), tt.want) + }) + } + + // A malformed line must not cost the file its other headers. + rules := parseHeaderRules("/*\n NotAHeader\n X-Ok: 1\n") + header := http.Header{} + rules.apply(header, "/anything") + assert.Equal(t, "1", header.Get("X-Ok")) +} + +func TestHeaderValuesContainingColons(t *testing.T) { + // Only the first colon separates name from value. + rules := parseHeaderRules("/*\n Content-Security-Policy: default-src https://a.test; img-src *\n") + + header := http.Header{} + rules.apply(header, "/x") + assert.Equal(t, "default-src https://a.test; img-src *", header.Get("Content-Security-Policy")) +} + +func TestNoHeadersFileIsANoOp(t *testing.T) { + dir := setupTestDir(t) + fs := file.NewLocalFileSystem(logging.NewMockLogger(logging.ERROR)) + + assert.Empty(t, loadHeaderRules(fs, dir), "a site without _headers gets no rules") + + h := &staticFileHandler{fs: fs, staticFilePath: dir, defaultExtension: ".html", next: http.NotFoundHandler()} + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/style.css", http.NoBody) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Empty(t, rec.Header().Get("X-Frame-Options")) + assert.Empty(t, rec.Header().Get("Cache-Control")) +} + +func TestServeHTTPAppliesHeaderRules(t *testing.T) { + dir := setupTestDir(t) + writeFile(t, dir, headersFileName, realHeadersFile) + + fs := file.NewLocalFileSystem(logging.NewMockLogger(logging.ERROR)) + rules := loadHeaderRules(fs, dir) + assert.Len(t, rules, 7, "rules load from disk") + + newHandler := func() *staticFileHandler { + return &staticFileHandler{ + fs: fs, staticFilePath: dir, defaultExtension: ".html", + next: http.NotFoundHandler(), headerRules: rules, + } + } + + t.Run("a served page carries the site-wide security headers", func(t *testing.T) { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/style.css", http.NoBody) + rec := httptest.NewRecorder() + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, "DENY", rec.Header().Get("X-Frame-Options")) + assert.Equal(t, "nosniff", rec.Header().Get("X-Content-Type-Options")) + }) + + // Netlify applies _headers to error responses too, and a 404 that leaks + // framing protection is exactly as exploitable as a 200 that does. + t.Run("a 404 carries them too", func(t *testing.T) { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/nope", http.NoBody) + rec := httptest.NewRecorder() + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, http.StatusNotFound, rec.Code) + assert.Equal(t, "DENY", rec.Header().Get("X-Frame-Options")) + }) + + // ...but not its caching. A site's Cache-Control describes the pages it + // publishes; applying a page rule to a miss would pin a file that is merely + // not propagated yet into every cache downstream for the rule's lifetime. + // + // `/missing.html` is the path that proves it: it matches the `/*.html` + // block, so without the fix the 404 inherits max-age=300. An extensionless + // miss would not — it only matches `/*`, which declares no Cache-Control. + t.Run("a 404 does not inherit the site's Cache-Control", func(t *testing.T) { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/missing.html", http.NoBody) + rec := httptest.NewRecorder() + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, http.StatusNotFound, rec.Code) + assert.Empty(t, rec.Header().Get("Cache-Control"), "a miss must not be cacheable by a page rule") + assert.Equal(t, "DENY", rec.Header().Get("X-Frame-Options"), "security headers still apply") + + // The same rule really does apply on a hit — otherwise this proves nothing. + hit := httptest.NewRecorder() + hitReq := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/docs.html", http.NoBody) + newHandler().ServeHTTP(hit, hitReq) + assert.Equal(t, "public, max-age=300", hit.Header().Get("Cache-Control"), + "control: /*.html caches a page that exists") + }) + + // The rules must not be able to strip Vary and let a CDN cross-serve + // markdown to a browser. + t.Run("Vary: Accept survives on a negotiable route", func(t *testing.T) { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/docs", http.NoBody) + rec := httptest.NewRecorder() + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code, "control: /docs resolves to docs.html") + assert.Equal(t, "Accept", rec.Header().Get("Vary")) + }) + + // A hashed bundle can never resolve to markdown, so keying caches on Accept + // would fragment them for nothing — on exactly the responses the same + // _headers file marks immutable. + t.Run("no Vary on an immutable asset", func(t *testing.T) { + writeFile(t, dir, "_astro/app.DY-PF2h0.js", "console.log(1)") + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/_astro/app.DY-PF2h0.js", http.NoBody) + rec := httptest.NewRecorder() + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code, "control: the asset is really served") + assert.Contains(t, rec.Header().Get("Cache-Control"), "immutable", "control: the asset rule applies") + assert.Empty(t, rec.Header().Get("Vary"), "an unnegotiable asset must not fragment caches") + }) +} + +// TestWellKnownCarriesHeaderRules covers the paths this server hands off rather +// than serves. .well-known is delegated so that ACME challenges are not given an +// extension or swallowed by the SPA fallback — but a site declaring +// X-Frame-Options for `/*` means the whole site, and a delegated path is still +// one this server answered for. +func TestWellKnownCarriesHeaderRules(t *testing.T) { + dir := setupTestDir(t) + writeFile(t, dir, headersFileName, realHeadersFile) + + fs := file.NewLocalFileSystem(logging.NewMockLogger(logging.ERROR)) + rules := loadHeaderRules(fs, dir) + + newHandler := func(next http.Handler) *staticFileHandler { + return &staticFileHandler{ + fs: fs, staticFilePath: dir, defaultExtension: ".html", + next: next, headerRules: rules, + } + } + + // The delegate still decides the body and status: passing through must not + // mean passing through unprotected. + t.Run("a delegated response carries the site-wide security headers", func(t *testing.T) { + served := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("acme-challenge-token")) + }) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, + "/.well-known/acme-challenge/token.html", http.NoBody) + rec := httptest.NewRecorder() + + newHandler(served).ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, "acme-challenge-token", rec.Body.String(), "the delegate still writes the body") + assert.Equal(t, "DENY", rec.Header().Get("X-Frame-Options")) + assert.Equal(t, "nosniff", rec.Header().Get("X-Content-Type-Options")) + }) + + // A delegate that succeeds is serving a real file, so it keeps the caching + // the site asked for. + t.Run("a delegated 200 keeps the site's Cache-Control", func(t *testing.T) { + served := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("ok")) + }) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, + "/.well-known/security.html", http.NoBody) + rec := httptest.NewRecorder() + + newHandler(served).ServeHTTP(rec, req) + + assert.Equal(t, "public, max-age=300", rec.Header().Get("Cache-Control"), + "a real delegated file is still a page the site publishes") + }) + + // ...but a delegated miss must not be cached, for the same reason a directly + // served miss must not be. + t.Run("a delegated 404 does not inherit Cache-Control", func(t *testing.T) { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, + "/.well-known/acme-challenge/absent.html", http.NoBody) + rec := httptest.NewRecorder() + + newHandler(http.NotFoundHandler()).ServeHTTP(rec, req) + + assert.Equal(t, http.StatusNotFound, rec.Code) + assert.Empty(t, rec.Header().Get("Cache-Control")) + assert.Equal(t, "DENY", rec.Header().Get("X-Frame-Options"), "security headers still apply") + }) + + // Delegation itself must survive: this path is how ACME issues certificates. + t.Run("the path is still handed to the delegate untouched", func(t *testing.T) { + var gotPath string + + spy := http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { gotPath = r.URL.Path }) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, + "/.well-known/acme-challenge/xyz", http.NoBody) + + newHandler(spy).ServeHTTP(httptest.NewRecorder(), req) + + assert.Equal(t, "/.well-known/acme-challenge/xyz", gotPath, "no extension, no rewriting") + }) +} diff --git a/main.go b/main.go index 4e434b8..b1aba58 100644 --- a/main.go +++ b/main.go @@ -14,13 +14,39 @@ const indexHTML = "/index.html" const htmlExtension = ".html" const rootPath = "/" +// configLookup is the slice of gofr's config this server reads. Declaring it +// here lets the empty-value resolution below be exercised directly, rather than +// only through a running app. +type configLookup interface { + GetOrDefault(key, fallback string) string +} + +// resolveOrDefault returns the configured value for key, falling back when the +// key is absent *or* present but empty. +// +// gofr's GetOrDefault only covers absent. The shipped configs/.env sets these +// keys with empty values, so a deployment supplying STATIC_DIR_PATH through the +// environment would otherwise receive "" and silently root every lookup at the +// process working directory — serving pages while loading zero `_headers` rules, +// the kind of half-working state that never gets noticed. +func resolveOrDefault(cfg configLookup, key, fallback string) string { + if value := cfg.GetOrDefault(key, fallback); value != "" { + return value + } + + return fallback +} + func main() { app := gofr.New() - staticFilePath := app.Config.GetOrDefault("STATIC_DIR_PATH", defaultStaticFilePath) + staticFilePath := resolveOrDefault(app.Config, "STATIC_DIR_PATH", defaultStaticFilePath) + + // SPA_MODE needs no such guard: ParseBool rejects "" and leaves the same + // false the default would have produced. spaMode, _ := strconv.ParseBool(app.Config.GetOrDefault("SPA_MODE", "false")) - defaultExtension := app.Config.GetOrDefault("DEFAULT_EXTENSION", htmlExtension) + defaultExtension := resolveOrDefault(app.Config, "DEFAULT_EXTENSION", htmlExtension) handler := &staticFileHandler{ staticFilePath: staticFilePath, @@ -35,6 +61,14 @@ func main() { ctx.Logger.Error(err.Error()) } + // Read once at startup rather than per request. Absent file → no rules + // → responses are byte-for-byte what they are today. + handler.headerRules = loadHeaderRules(ctx.File, staticFilePath) + // The resolved directory is logged with the count: "0 rules" is normal + // for a site without the file, but indistinguishable from a misrooted + // path unless the path is on the line too. + ctx.Logger.Infof("loaded %d %s rule(s) from %s", len(handler.headerRules), headersFileName, staticFilePath) + return nil }) diff --git a/main_test.go b/main_test.go index 7d6f297..beecf7c 100644 --- a/main_test.go +++ b/main_test.go @@ -7,6 +7,7 @@ import ( "path/filepath" "testing" + "github.com/stretchr/testify/assert" "gofr.dev/pkg/gofr/datasource/file" "gofr.dev/pkg/gofr/logging" ) @@ -56,3 +57,51 @@ func TestServer(t *testing.T) { _ = resp.Body.Close() } } + +// The shipped configs/.env sets STATIC_DIR_PATH= and DEFAULT_EXTENSION= with +// empty values. GetOrDefault only falls back on an ABSENT key, so an empty one +// yields "" and roots every lookup at the process working directory — which is +// how a container given STATIC_DIR_PATH via the environment silently loaded +// zero _headers rules while still serving pages. +// fakeConfig stands in for gofr's config: a key it holds is "present" even when +// its value is empty, which is the whole distinction resolveOrDefault exists to +// handle and the one the shipped configs/.env actually trips. +type fakeConfig map[string]string + +func (f fakeConfig) GetOrDefault(key, fallback string) string { + if value, ok := f[key]; ok { + return value + } + + return fallback +} + +func TestResolveOrDefault(t *testing.T) { + tests := []struct { + name string + cfg fakeConfig + key string + fallback string + want string + }{ + // The regression: configs/.env ships STATIC_DIR_PATH= and + // DEFAULT_EXTENSION= with empty values, so GetOrDefault finds the key, + // returns "", and every path lookup roots at the working directory. + {"present but empty falls back", fakeConfig{"STATIC_DIR_PATH": ""}, "STATIC_DIR_PATH", defaultStaticFilePath, defaultStaticFilePath}, + {"present but empty extension falls back", fakeConfig{"DEFAULT_EXTENSION": ""}, "DEFAULT_EXTENSION", htmlExtension, htmlExtension}, + + {"absent falls back", fakeConfig{}, "STATIC_DIR_PATH", defaultStaticFilePath, defaultStaticFilePath}, + {"a real value is kept", fakeConfig{"STATIC_DIR_PATH": "/srv/site"}, "STATIC_DIR_PATH", defaultStaticFilePath, "/srv/site"}, + {"a real extension is kept", fakeConfig{"DEFAULT_EXTENSION": ".htm"}, "DEFAULT_EXTENSION", htmlExtension, ".htm"}, + + // Whitespace is a value, not emptiness — guessing at it would be a + // different bug from the one being fixed. + {"whitespace is kept as given", fakeConfig{"DEFAULT_EXTENSION": " "}, "DEFAULT_EXTENSION", htmlExtension, " "}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, resolveOrDefault(tt.cfg, tt.key, tt.fallback)) + }) + } +} diff --git a/negotiate.go b/negotiate.go new file mode 100644 index 0000000..82bb5d8 --- /dev/null +++ b/negotiate.go @@ -0,0 +1,149 @@ +package main + +import ( + "mime" + "net/http" + "path/filepath" + "strconv" + "strings" +) + +const ( + markdownExtension = ".md" + markdownMediaType = "text/markdown" + // Some clients still send the pre-RFC-7763 spelling. + legacyMarkdownMediaType = "text/x-markdown" + htmlMediaType = "text/html" + xhtmlMediaType = "application/xhtml+xml" + + // Set explicitly when serving markdown. Go's built-in MIME table has no + // entry for .md and a scratch base image has no /etc/mime.types, so + // http.ServeFile would otherwise sniff the file and label it text/plain. + markdownContentType = "text/markdown; charset=utf-8" + + defaultQuality = 1.0 +) + +// negotiable reports whether the response for a URL path can depend on Accept. +// +// Only an extensionless route can resolve to a `.md` sibling; the root is +// served from index.html and never negotiates. Everything else — hashed +// bundles, images, a directly requested .md — resolves to the same file +// whatever the client asks for, so its response neither varies nor needs to +// say that it might. +// +// This is the single definition of that condition: resolveFilePath gates +// negotiation on it and the handler gates Vary and the markdown Content-Type +// on it, so the three cannot drift apart. +func negotiable(urlPath string) bool { + return urlPath != rootPath && filepath.Ext(urlPath) == "" +} + +// advertiseAcceptVaries records that this response depends on Accept, so that a +// shared cache keys on it instead of handing one client's copy to another. +// +// Add rather than Set: a site's own `_headers` may declare a Vary of its own, +// and Set would discard it. Repeated Vary field lines are combined by caches, so +// a declared `Vary: Accept-Encoding` plus this one reads as +// `Accept-Encoding, Accept` — which is exactly right. The only case worth +// guarding is a site that already named Accept itself, where adding it again +// would yield a pointless `Accept, Accept`. +func advertiseAcceptVaries(header http.Header) { + for _, line := range header.Values("Vary") { + for _, field := range strings.Split(line, ",") { + if strings.EqualFold(strings.TrimSpace(field), "Accept") { + return + } + } + } + + header.Add("Vary", "Accept") +} + +// labelAsMarkdown reports whether this server should set an explicit markdown +// Content-Type rather than leaving the type to http.ServeFile. +// +// Only a negotiated response gets one. ServeFile would otherwise sniff the file +// as text/plain wherever the platform has no `.md` entry — Go's built-in table +// has none and the distroless base image ships no /etc/mime.types, so that is +// the case in production. +// +// A directly requested .md is left alone on purpose: whatever it resolves to +// today is what a site's existing .md links already behave like, and browsers +// render text/plain inline but download text/markdown. Note the starting point +// differs by platform — most Linux distributions do map .md, so there the type +// is already text/markdown and this changes nothing. That is why the decision +// is tested through this function: asserting on a served response would only +// pin whatever the host's MIME table happens to say. +func labelAsMarkdown(wantsMarkdown bool, urlPath, filePath string) bool { + return wantsMarkdown && negotiable(urlPath) && strings.HasSuffix(filePath, markdownExtension) +} + +// acceptEntry is one parsed media range from an Accept header. +type acceptEntry struct { + mediaType string + quality float64 +} + +// parseAcceptEntry parses a single Accept media range. Entries that are +// malformed, or that carry an unparseable q-value, are reported as unusable +// rather than guessed at. +func parseAcceptEntry(entry string) (acceptEntry, bool) { + mediaType, params, err := mime.ParseMediaType(strings.TrimSpace(entry)) + if err != nil { + return acceptEntry{}, false + } + + quality := defaultQuality + + if raw, ok := params["q"]; ok { + parsed, err := strconv.ParseFloat(raw, 64) + if err != nil { + return acceptEntry{}, false + } + + quality = parsed + } + + return acceptEntry{mediaType: mediaType, quality: quality}, true +} + +// markdownPreferred reports whether the client asked for markdown in +// preference to HTML. +// +// "Asked for" means named the type. Browsers send +// `text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8` — that +// trailing wildcard technically matches text/markdown, so matching on +// wildcards would serve raw markdown to every browser on the internet. Only an +// explicit media type counts. +// +// Agents that want markdown do name it: Claude Code, Cursor and OpenCode all +// send `text/markdown` today. Quality values are honored, so a client that +// lists markdown below HTML (`text/html, text/markdown;q=0.1`) still gets +// HTML — it expressed a preference and we respect it. +func markdownPreferred(accept string) bool { + if accept == "" { + return false + } + + var markdownQuality, htmlQuality float64 + + named := false + + for _, raw := range strings.Split(accept, ",") { + entry, ok := parseAcceptEntry(raw) + if !ok { + continue + } + + switch entry.mediaType { + case markdownMediaType, legacyMarkdownMediaType: + named = true + markdownQuality = max(markdownQuality, entry.quality) + case htmlMediaType, xhtmlMediaType: + htmlQuality = max(htmlQuality, entry.quality) + } + } + + return named && markdownQuality > 0 && markdownQuality >= htmlQuality +} diff --git a/negotiate_test.go b/negotiate_test.go new file mode 100644 index 0000000..99932d3 --- /dev/null +++ b/negotiate_test.go @@ -0,0 +1,438 @@ +package main + +import ( + "mime" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "gofr.dev/pkg/gofr/datasource/file" + "gofr.dev/pkg/gofr/logging" +) + +func TestMarkdownPreferred(t *testing.T) { + tests := []struct { + name string + accept string + want bool + }{ + {"empty", "", false}, + {"bare markdown", "text/markdown", true}, + {"markdown with charset", "text/markdown; charset=utf-8", true}, + {"legacy spelling", "text/x-markdown", true}, + {"markdown first", "text/markdown, text/html", true}, + {"markdown listed after html, equal q", "text/html, text/markdown", true}, + + // The one that matters: a browser's Accept ends in */*;q=0.8, which + // matches text/markdown by the letter of RFC 9110. Treating that as a + // request for markdown would serve raw source to every human visitor. + {"chrome", "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8", false}, + {"safari", "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8", false}, + {"wildcard only", "*/*", false}, + {"curl default", "*/*", false}, + + // A client that ranks markdown below HTML expressed a preference. + {"markdown deprioritised", "text/html, text/markdown;q=0.1", false}, + {"markdown zero q", "text/markdown;q=0", false}, + {"html deprioritised", "text/html;q=0.2, text/markdown;q=0.9", true}, + {"xhtml outranks markdown", "application/xhtml+xml;q=0.9, text/markdown;q=0.5", false}, + + {"unrelated types", "application/json, text/plain", false}, + {"malformed entry ignored", "text/markdown, ;;;broken", true}, + {"malformed q ignored", "text/markdown;q=notanumber", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, markdownPreferred(tt.accept)) + }) + } +} + +func writeFile(t *testing.T, dir, name, content string) { + t.Helper() + + path := filepath.Join(dir, name) + + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + t.Fatalf("Failed to create dir for %s: %v", name, err) + } + + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write %s: %v", name, err) + } +} + +// setupNegotiationDir mirrors what a static site generator emits: the rendered +// page as a directory index, with the markdown source as its sibling. +func setupNegotiationDir(t *testing.T) string { + t.Helper() + + dir := setupTestDir(t) + + writeFile(t, dir, "about/index.html", "about") + writeFile(t, dir, "about.md", "# About\n\nmarkdown source\n") + // A page with no markdown sibling — negotiation must fall through to HTML. + writeFile(t, dir, "legal/index.html", "legal") + + return dir +} + +func TestResolveFilePathMarkdownNegotiation(t *testing.T) { + dir := setupNegotiationDir(t) + fs := file.NewLocalFileSystem(logging.NewMockLogger(logging.ERROR)) + + tests := []struct { + name string + urlPath string + accept string + wantFile string + }{ + {"agent gets markdown", "/about", "text/markdown", "about.md"}, + {"browser gets html", "/about", "text/html,*/*;q=0.8", "about/index.html"}, + {"no accept header gets html", "/about", "", "about/index.html"}, + {"falls through when no .md exists", "/legal", "text/markdown", "legal/index.html"}, + {"root is never negotiated", rootPath, "text/markdown", "index.html"}, + {"explicit extension is untouched", "/style.css", "text/markdown", "style.css"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := &staticFileHandler{fs: fs, staticFilePath: dir, defaultExtension: ".html"} + + path, _ := h.resolveFilePath(tt.urlPath, markdownPreferred(tt.accept)) + + assert.Equal(t, filepath.Join(dir, tt.wantFile), path) + }) + } +} + +func TestServeHTTPMarkdownNegotiation(t *testing.T) { + dir := setupNegotiationDir(t) + fs := file.NewLocalFileSystem(logging.NewMockLogger(logging.ERROR)) + + newHandler := func() *staticFileHandler { + return &staticFileHandler{ + fs: fs, + staticFilePath: dir, + defaultExtension: ".html", + next: http.NotFoundHandler(), + } + } + + t.Run("agent receives markdown with the right content type", func(t *testing.T) { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/about", http.NoBody) + req.Header.Set("Accept", "text/markdown") + + rec := httptest.NewRecorder() + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Contains(t, rec.Body.String(), "markdown source") + assert.Contains(t, rec.Header().Get("Content-Type"), "text/markdown") + }) + + t.Run("browser still receives html", func(t *testing.T) { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/about", http.NoBody) + req.Header.Set("Accept", "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8") + + rec := httptest.NewRecorder() + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Contains(t, rec.Body.String(), "about") + assert.Contains(t, rec.Header().Get("Content-Type"), "text/html") + }) + + t.Run("Vary: Accept is set on a route that can negotiate", func(t *testing.T) { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/about", http.NoBody) + rec := httptest.NewRecorder() + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, "Accept", rec.Header().Get("Vary")) + }) + + t.Run("a miss is answered in markdown, not a large HTML shell", func(t *testing.T) { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/no/such/page", http.NoBody) + req.Header.Set("Accept", "text/markdown") + + rec := httptest.NewRecorder() + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, http.StatusNotFound, rec.Code) + assert.Contains(t, rec.Header().Get("Content-Type"), "text/markdown") + assert.Contains(t, rec.Body.String(), "404 Not Found") + // Recovery pointers, so the reader can find real URLs itself. + assert.Contains(t, rec.Body.String(), "/sitemap.xml") + // An HTML 404 shell on a real site measured 144 KB. + assert.Less(t, rec.Body.Len(), 1024) + }) + + t.Run("a browser still gets the HTML 404 page", func(t *testing.T) { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/no/such/page", http.NoBody) + req.Header.Set("Accept", "text/html,*/*;q=0.8") + + rec := httptest.NewRecorder() + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, http.StatusNotFound, rec.Code) + assert.Contains(t, rec.Body.String(), "404") + assert.NotContains(t, rec.Header().Get("Content-Type"), "markdown") + }) +} + +// TestLabelAsMarkdown pins which responses this server relabels as markdown. +// +// Asserted on the decision rather than on a served Content-Type, because the +// starting point is not the same everywhere: most Linux distributions map .md +// in /etc/mime.types, so http.ServeFile already answers text/markdown there, +// while macOS and the distroless image that ships to production have no entry +// and sniff text/plain. A test that read the header back would pin the host's +// MIME table, not this server's behavior — and would pass or fail by platform. +func TestLabelAsMarkdown(t *testing.T) { + tests := []struct { + name string + wantsMarkdown bool + urlPath string + filePath string + want bool + }{ + {"negotiated route", true, "/about", "/site/about.md", true}, + {"nested negotiated route", true, "/docs/intro", "/site/docs/intro.md", true}, + + // The case this scoping exists for: the client did not negotiate, so + // the file keeps whatever type it already had. + {"direct .md, no Accept", false, "/about.md", "/site/about.md", false}, + {"direct .md, but asking for markdown", true, "/about.md", "/site/about.md", false}, + + {"negotiable route that fell through to html", true, "/legal", "/site/legal/index.html", false}, + {"root never negotiates", true, rootPath, "/site/index.html", false}, + {"asset", true, "/style.css", "/site/style.css", false}, + {"client never asked", false, "/about", "/site/about.md", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, labelAsMarkdown(tt.wantsMarkdown, tt.urlPath, tt.filePath)) + }) + } +} + +// TestMarkdownContentTypeScope checks the same scoping end to end, in the terms +// a platform can actually agree on. +func TestMarkdownContentTypeScope(t *testing.T) { + dir := setupNegotiationDir(t) + fs := file.NewLocalFileSystem(logging.NewMockLogger(logging.ERROR)) + + newHandler := func() *staticFileHandler { + return &staticFileHandler{ + fs: fs, + staticFilePath: dir, + defaultExtension: ".html", + next: http.NotFoundHandler(), + } + } + + // What http.ServeFile labels a .md as when this server keeps its hands off: + // the system MIME table where there is an entry, sniffed text/plain where + // there is not (Go's built-in table has none, and neither does distroless). + untouchedType := mime.TypeByExtension(markdownExtension) + if untouchedType == "" { + untouchedType = "text/plain; charset=utf-8" + } + + t.Run("direct .md keeps working and keeps the platform's type", func(t *testing.T) { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/about.md", http.NoBody) + rec := httptest.NewRecorder() + + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Contains(t, rec.Body.String(), "markdown source") + assert.Equal(t, untouchedType, rec.Header().Get("Content-Type"), + "a direct .md must keep the type it would have had without this server") + assert.Empty(t, rec.Header().Get("Vary"), "a direct .md never negotiates") + }) + + // A negotiated response is labeled on every platform, including the one + // that would otherwise sniff it as plain text. + t.Run("a negotiated response is always labeled markdown", func(t *testing.T) { + negotiated := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/about", http.NoBody) + negotiated.Header.Set("Accept", "text/markdown") + + negRec := httptest.NewRecorder() + + newHandler().ServeHTTP(negRec, negotiated) + + direct := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/about.md", http.NoBody) + dirRec := httptest.NewRecorder() + + newHandler().ServeHTTP(dirRec, direct) + + assert.Equal(t, negRec.Body.String(), dirRec.Body.String(), "same bytes either way") + assert.Equal(t, markdownContentType, negRec.Header().Get("Content-Type")) + }) + + // The root is served straight from index.html and never negotiates, so it + // must not advertise Vary either — it is usually the most-cached URL a site + // has, and fragmenting it on Accept buys nothing. + t.Run("the root never negotiates and never varies", func(t *testing.T) { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, rootPath, http.NoBody) + req.Header.Set("Accept", "text/markdown") + + rec := httptest.NewRecorder() + + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Contains(t, rec.Body.String(), "index", "root is always index.html") + assert.Empty(t, rec.Header().Get("Vary"), "the root cannot vary by Accept") + assert.NotContains(t, rec.Header().Get("Content-Type"), "text/markdown") + }) + + // A miss answers in markdown whenever asked, whatever the path shape — so + // even a path that never negotiates on a hit must still advertise Vary, or + // a cache can hand an agent the HTML shell it stored for a browser. + t.Run("a miss advertises Vary even on an extensioned path", func(t *testing.T) { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/gone.html", http.NoBody) + req.Header.Set("Accept", "text/markdown") + + rec := httptest.NewRecorder() + + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, http.StatusNotFound, rec.Code) + assert.Equal(t, "Accept", rec.Header().Get("Vary")) + assert.Contains(t, rec.Header().Get("Content-Type"), "text/markdown") + }) +} + +// TestSPAFallbackAdvertisesVary covers the one shape where a single URL yields +// two bodies without either being a negotiated hit: SPA mode, a `.md` on disk, +// and no HTML page beside it. Markdown clients take the hit path and get the +// markdown; browsers fall through to the shell. A cache that stored the shell +// unkeyed would hand it to the next client that asked for markdown. +func TestSPAFallbackAdvertisesVary(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "index.html", "shell") + writeFile(t, dir, "404.html", "404") + // Deliberately no foo.html and no foo/index.html. + writeFile(t, dir, "foo.md", "# Foo\n\nmarkdown only\n") + + fs := file.NewLocalFileSystem(logging.NewMockLogger(logging.ERROR)) + + newHandler := func() *staticFileHandler { + return &staticFileHandler{ + fs: fs, staticFilePath: dir, defaultExtension: ".html", + spaMode: true, next: http.NotFoundHandler(), + } + } + + t.Run("the browser reaching the shell still gets Vary", func(t *testing.T) { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/foo", http.NoBody) + req.Header.Set("Accept", "text/html,*/*;q=0.8") + + rec := httptest.NewRecorder() + + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Contains(t, rec.Body.String(), "shell", "control: this really is the SPA fallback") + assert.Equal(t, "Accept", rec.Header().Get("Vary")) + }) + + // The other half of the pair — proving the two responses really do differ, + // which is what makes the header above load-bearing. + t.Run("the agent gets markdown for the same URL", func(t *testing.T) { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/foo", http.NoBody) + req.Header.Set("Accept", "text/markdown") + + rec := httptest.NewRecorder() + + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Contains(t, rec.Body.String(), "markdown only") + assert.Equal(t, "Accept", rec.Header().Get("Vary")) + }) + + // The root is not negotiable — it is served from index.html for every client + // alike — so it must not be keyed on Accept even in SPA mode, where it is the + // most-requested URL the site has. + t.Run("the root is not keyed on Accept", func(t *testing.T) { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, rootPath, http.NoBody) + req.Header.Set("Accept", "text/markdown") + + rec := httptest.NewRecorder() + + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Contains(t, rec.Body.String(), "shell", "control: the root is served, not missed") + assert.Empty(t, rec.Header().Get("Vary")) + }) +} + +func TestAdvertiseAcceptVaries(t *testing.T) { + tests := []struct { + name string + existing []string + want []string + }{ + {"nothing declared", nil, []string{"Accept"}}, + + // A site's own Vary must survive: Set would discard it, and repeated + // field lines are combined by caches, so this reads as + // "Accept-Encoding, Accept". + {"site declared something else", []string{"Accept-Encoding"}, []string{"Accept-Encoding", "Accept"}}, + + // ...but naming Accept twice is pointless. + {"site already declared Accept", []string{"Accept"}, []string{"Accept"}}, + {"site declared Accept in a list", []string{"Accept-Encoding, Accept"}, []string{"Accept-Encoding, Accept"}}, + {"case-insensitive per RFC 9110", []string{"accept"}, []string{"accept"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + header := http.Header{} + for _, v := range tt.existing { + header.Add("Vary", v) + } + + advertiseAcceptVaries(header) + + assert.Equal(t, tt.want, header.Values("Vary")) + }) + } +} + +// TestSPAFallbackRootIsNeverKeyed pins the negotiable() guard on the SPA +// fallback. That guard is only observable in one shape — SPA mode with no +// index.html on disk, so the root itself reaches the fallback — because every +// other path that gets there is extensionless and therefore negotiable. Without +// the test the guard would be an unverified assertion rather than a checked one. +func TestSPAFallbackRootIsNeverKeyed(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "404.html", "404") + + fs := file.NewLocalFileSystem(logging.NewMockLogger(logging.ERROR)) + h := &staticFileHandler{ + fs: fs, staticFilePath: dir, defaultExtension: ".html", + spaMode: true, next: http.NotFoundHandler(), + } + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, rootPath, http.NoBody) + req.Header.Set("Accept", "text/markdown") + + rec := httptest.NewRecorder() + + h.ServeHTTP(rec, req) + + // The root is served from index.html for every client alike, so nothing + // about this response can depend on Accept — whether that file is there or, + // as here, missing. + assert.Empty(t, rec.Header().Get("Vary")) +}