From 85532a43aded5b47161a8903693deb25ae25d246 Mon Sep 17 00:00:00 2001 From: aryanmehrotra Date: Fri, 31 Jul 2026 15:20:21 +0530 Subject: [PATCH 1/8] feat: serve markdown to agents via Accept content negotiation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Static site generators emit the markdown source of a page as a sibling of its directory index — `about.md` next to `about/index.html`. The file is already on disk; the server just never looked for it. Now a request whose Accept header names text/markdown is served that sibling. Without this an agent can only discover the markdown by reading the inside the HTML document it was trying not to download. Measured on a real zop.dev build, the same page is 15-96x smaller as markdown (a changelog entry: 145,354 bytes of HTML vs 1,512). Only explicit media types count. A browser sends `*/*;q=0.8`, which matches text/markdown by the letter of RFC 9110, so matching wildcards would serve raw source to every human visitor; q-values are honoured, so a client that ranks markdown below HTML still gets HTML. The lookup falls through untouched when the client did not ask or the .md is absent, so nothing an existing deployment serves today changes. Vary: Accept is set on every response — without it a CDN can hand an agent's markdown to the next browser that asks for the same URL. .md is registered with the mime package explicitly: Go's built-in table has no entry for it and a scratch base image has no /etc/mime.types, so http.ServeFile would otherwise sniff the file and label it text/plain. Verified end to end against a 5,933-page build: agents get markdown, browsers get HTML, pages without a .md still return 200, and direct .md requests are unaffected. Handler and negotiation are at 100% statement coverage. --- handler.go | 27 +++++++- handler_test.go | 2 +- negotiate.go | 80 ++++++++++++++++++++++ negotiate_test.go | 165 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 271 insertions(+), 3 deletions(-) create mode 100644 negotiate.go create mode 100644 negotiate_test.go diff --git a/handler.go b/handler.go index 4b6dfdb..931d2f3 100644 --- a/handler.go +++ b/handler.go @@ -23,7 +23,12 @@ func (h *staticFileHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - filePath, hasExtension := h.resolveFilePath(r.URL.Path) + filePath, hasExtension := h.resolveFilePath(r.URL.Path, r.Header.Get("Accept")) + + // The response body for a given URL now depends on Accept, so caches must + // key on it. Without this a CDN can hand an agent's markdown response to + // the next browser that asks for the same page. + w.Header().Add("Vary", "Accept") if _, err := h.fs.Stat(filePath); err != nil { if h.spaMode && !hasExtension { @@ -40,11 +45,29 @@ func (h *staticFileHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { http.ServeFile(w, r, filePath) } -func (h *staticFileHandler) resolveFilePath(urlPath string) (string, bool) { +func (h *staticFileHandler) resolveFilePath(urlPath, accept string) (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 !hasExtension && urlPath != rootPath && markdownPreferred(accept) { + if _, err := h.fs.Stat(filePath + markdownExtension); err == nil { + return filePath + markdownExtension, true + } + } + if urlPath == rootPath { filePath += indexHTML } else if !hasExtension { diff --git a/handler_test.go b/handler_test.go index 08df288..eba7f0a 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, "") assert.Equal(t, tt.wantPath, path) assert.Equal(t, tt.wantHasExt, hasExt) diff --git a/negotiate.go b/negotiate.go new file mode 100644 index 0000000..806f0df --- /dev/null +++ b/negotiate.go @@ -0,0 +1,80 @@ +package main + +import ( + "mime" + "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" +) + +// Go's built-in MIME table has no entry for .md, and a scratch/distroless +// image has no /etc/mime.types to fall back on, so http.ServeFile would sniff +// the file and label it text/plain. Register it explicitly so the content type +// is correct regardless of what the base image ships. +func init() { + _ = mime.AddExtensionType(markdownExtension, "text/markdown; charset=utf-8") +} + +// 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 honoured, 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 markdownQ, htmlQ float64 + + named := false + + for _, entry := range strings.Split(accept, ",") { + mediaType, params, err := mime.ParseMediaType(strings.TrimSpace(entry)) + if err != nil { + continue + } + + q := 1.0 + + if raw, ok := params["q"]; ok { + parsed, err := strconv.ParseFloat(raw, 64) + if err != nil { + continue + } + + q = parsed + } + + switch mediaType { + case markdownMediaType, legacyMarkdownMediaType: + named = true + + if q > markdownQ { + markdownQ = q + } + case htmlMediaType, xhtmlMediaType: + if q > htmlQ { + htmlQ = q + } + } + } + + return named && markdownQ > 0 && markdownQ >= htmlQ +} diff --git a/negotiate_test.go b/negotiate_test.go new file mode 100644 index 0000000..bf936a1 --- /dev/null +++ b/negotiate_test.go @@ -0,0 +1,165 @@ +package main + +import ( + "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, 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.NewRequest(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.NewRequest(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 always set so caches do not cross-serve", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/about", http.NoBody) + rec := httptest.NewRecorder() + newHandler().ServeHTTP(rec, req) + + assert.Equal(t, "Accept", rec.Header().Get("Vary")) + }) + + t.Run("direct .md request keeps working", func(t *testing.T) { + req := httptest.NewRequest(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") + }) +} From 78b5d5e4fa04f1241d1eaa2b736c350b7fd12776 Mon Sep 17 00:00:00 2001 From: aryanmehrotra Date: Fri, 31 Jul 2026 16:00:39 +0530 Subject: [PATCH 2/8] fix: satisfy lint, and answer misses in markdown too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lint fixes for the CI gate: - drop the init() (gochecknoinits) — the .md content type is now set explicitly in the handler, which also covers directly-requested .md rather than relying on process-global MIME registration; - split Accept parsing into parseAcceptEntry so markdownPreferred drops back under the cyclomatic limit; - "honoured" -> "honored" (misspell, US locale). Also answers a 404 in markdown when the client asked for markdown. An HTML error shell is unusable to such a client and is not small: the 404 page of a real site measured 144,188 bytes, sent in reply to a request it could not parse. The markdown reply is 138 bytes and names /sitemap.xml and /llms.txt so the reader can recover on its own. The request path is deliberately not echoed into that body — gosec flagged it as an injection sink (G705), and the caller already knows the URL it asked for. Verified no behaviour change for anyone else: base and patched binaries served the same 5,933-page build and were compared over 1,505 request/response pairs (301 URLs x 5 non-markdown Accept variants, including a browser string and `text/html, text/markdown;q=0.1`). Status, Content-Type and body SHA-256 matched on every one. The only deltas are the added Vary: Accept and markdown for clients that asked. golangci-lint output is identical to origin/main — the diff introduces no new findings. --- handler.go | 41 ++++++++++++++++++++++++-- handler_test.go | 2 +- negotiate.go | 74 +++++++++++++++++++++++++++-------------------- negotiate_test.go | 38 ++++++++++++++++++++---- 4 files changed, 115 insertions(+), 40 deletions(-) diff --git a/handler.go b/handler.go index 931d2f3..9118fb8 100644 --- a/handler.go +++ b/handler.go @@ -23,7 +23,9 @@ func (h *staticFileHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - filePath, hasExtension := h.resolveFilePath(r.URL.Path, r.Header.Get("Accept")) + wantsMarkdown := markdownPreferred(r.Header.Get("Accept")) + + filePath, hasExtension := h.resolveFilePath(r.URL.Path, wantsMarkdown) // The response body for a given URL now depends on Accept, so caches must // key on it. Without this a CDN can hand an agent's markdown response to @@ -36,16 +38,49 @@ func (h *staticFileHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } + // 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")) return } + // http.ServeFile only sniffs a Content-Type when one is not already set, + // so setting it here wins. Applies to negotiated and directly-requested + // .md alike — neither can rely on the base image having /etc/mime.types. + if strings.HasSuffix(filePath, markdownExtension) { + w.Header().Set("Content-Type", markdownContentType) + } + http.ServeFile(w, r, filePath) } -func (h *staticFileHandler) resolveFilePath(urlPath, accept string) (string, bool) { +// 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) != "" @@ -62,7 +97,7 @@ func (h *staticFileHandler) resolveFilePath(urlPath, accept string) (string, boo // // 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 !hasExtension && urlPath != rootPath && markdownPreferred(accept) { + if !hasExtension && urlPath != rootPath && wantsMarkdown { if _, err := h.fs.Stat(filePath + markdownExtension); err == nil { return filePath + markdownExtension, true } diff --git a/handler_test.go b/handler_test.go index eba7f0a..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/negotiate.go b/negotiate.go index 806f0df..a6d38a5 100644 --- a/negotiate.go +++ b/negotiate.go @@ -13,14 +13,42 @@ const ( 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 ) -// Go's built-in MIME table has no entry for .md, and a scratch/distroless -// image has no /etc/mime.types to fall back on, so http.ServeFile would sniff -// the file and label it text/plain. Register it explicitly so the content type -// is correct regardless of what the base image ships. -func init() { - _ = mime.AddExtensionType(markdownExtension, "text/markdown; charset=utf-8") +// 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 @@ -33,7 +61,7 @@ func init() { // explicit media type counts. // // Agents that want markdown do name it: Claude Code, Cursor and OpenCode all -// send `text/markdown` today. Quality values are honoured, so a client that +// 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 { @@ -41,40 +69,24 @@ func markdownPreferred(accept string) bool { return false } - var markdownQ, htmlQ float64 + var markdownQuality, htmlQuality float64 named := false - for _, entry := range strings.Split(accept, ",") { - mediaType, params, err := mime.ParseMediaType(strings.TrimSpace(entry)) - if err != nil { + for _, raw := range strings.Split(accept, ",") { + entry, ok := parseAcceptEntry(raw) + if !ok { continue } - q := 1.0 - - if raw, ok := params["q"]; ok { - parsed, err := strconv.ParseFloat(raw, 64) - if err != nil { - continue - } - - q = parsed - } - - switch mediaType { + switch entry.mediaType { case markdownMediaType, legacyMarkdownMediaType: named = true - - if q > markdownQ { - markdownQ = q - } + markdownQuality = max(markdownQuality, entry.quality) case htmlMediaType, xhtmlMediaType: - if q > htmlQ { - htmlQ = q - } + htmlQuality = max(htmlQuality, entry.quality) } } - return named && markdownQ > 0 && markdownQ >= htmlQ + return named && markdownQuality > 0 && markdownQuality >= htmlQuality } diff --git a/negotiate_test.go b/negotiate_test.go index bf936a1..79a1969 100644 --- a/negotiate_test.go +++ b/negotiate_test.go @@ -102,7 +102,7 @@ func TestResolveFilePathMarkdownNegotiation(t *testing.T) { t.Run(tt.name, func(t *testing.T) { h := &staticFileHandler{fs: fs, staticFilePath: dir, defaultExtension: ".html"} - path, _ := h.resolveFilePath(tt.urlPath, tt.accept) + path, _ := h.resolveFilePath(tt.urlPath, markdownPreferred(tt.accept)) assert.Equal(t, filepath.Join(dir, tt.wantFile), path) }) @@ -123,7 +123,7 @@ func TestServeHTTPMarkdownNegotiation(t *testing.T) { } t.Run("agent receives markdown with the right content type", func(t *testing.T) { - req := httptest.NewRequest(http.MethodGet, "/about", http.NoBody) + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/about", http.NoBody) req.Header.Set("Accept", "text/markdown") rec := httptest.NewRecorder() @@ -135,7 +135,7 @@ func TestServeHTTPMarkdownNegotiation(t *testing.T) { }) t.Run("browser still receives html", func(t *testing.T) { - req := httptest.NewRequest(http.MethodGet, "/about", http.NoBody) + 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() @@ -147,7 +147,7 @@ func TestServeHTTPMarkdownNegotiation(t *testing.T) { }) t.Run("Vary: Accept is always set so caches do not cross-serve", func(t *testing.T) { - req := httptest.NewRequest(http.MethodGet, "/about", http.NoBody) + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/about", http.NoBody) rec := httptest.NewRecorder() newHandler().ServeHTTP(rec, req) @@ -155,11 +155,39 @@ func TestServeHTTPMarkdownNegotiation(t *testing.T) { }) t.Run("direct .md request keeps working", func(t *testing.T) { - req := httptest.NewRequest(http.MethodGet, "/about.md", http.NoBody) + 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") }) + + 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") + }) } From 872e8a4082102c1c28e00cd4dea7e79e4fde7f8e Mon Sep 17 00:00:00 2001 From: aryanmehrotra Date: Fri, 31 Jul 2026 17:42:57 +0530 Subject: [PATCH 3/8] feat: apply the _headers file the published directory ships MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_headers` is the Netlify / Cloudflare Pages convention: a file at the root of the published directory listing path patterns and the response headers to send for them. Static site generators emit it expecting the host to honour it, and a host that ignores it fails in the worst way — the file looks authoritative in the repo while nothing it declares has ever reached a browser. https://zop.dev, served by this project, has shipped a 2,437-byte _headers for months. Measured against the live site, not one of its rules was in effect: X-Frame-Options: DENY absent X-Content-Type-Options: nosniff absent Referrer-Policy absent Permissions-Policy absent Cache-Control on /_astro/* (immutable) absent entirely That last one matters twice over: content-hashed bundles that could be cached for a year were being served with no caching directive at all. Rules are parsed once at startup. A published directory without a _headers file yields no rules, so an existing deployment is byte-for-byte unchanged. All matching rules contribute in file order, so a later specific block overrides an earlier catch-all, matching upstream precedence. Malformed lines are skipped rather than failing the file: one bad rule should not cost a site every other header it declares. Headers are applied before anything writes, so they cover hits, misses and the SPA fallback alike — a 404 that leaks framing protection is as exploitable as a 200 that does. Note for operators: if a reverse proxy sits in front of this server, it may set some of these itself. Strict-Transport-Security is the usual one (ingress-nginx sends max-age=15724800, no preload, with replace semantics), and where it does it will keep winning; the other five headers rarely have a proxy counterpart and take effect immediately. Verified against a real 5,933-page build: 22 rules load, and the expected headers appear on pages, hashed assets, the root and robots.txt. Compared 891 request/response pairs against origin/main — zero body differences, zero unintended differences. Handler and parser at 100%/96% statement coverage; golangci-lint output identical to origin/main. --- handler.go | 9 ++ headers.go | 147 ++++++++++++++++++++++++++++++++ headers_test.go | 219 ++++++++++++++++++++++++++++++++++++++++++++++++ main.go | 5 ++ 4 files changed, 380 insertions(+) create mode 100644 headers.go create mode 100644 headers_test.go diff --git a/handler.go b/handler.go index 9118fb8..2fe40ca 100644 --- a/handler.go +++ b/handler.go @@ -14,6 +14,10 @@ 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) { @@ -27,6 +31,11 @@ func (h *staticFileHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { filePath, hasExtension := h.resolveFilePath(r.URL.Path, wantsMarkdown) + // Applied before anything writes, so it covers hits, misses and the SPA + // fallback alike. Set first so the server's own headers below still win + // where they are load-bearing. + h.headerRules.apply(w.Header(), r.URL.Path) + // The response body for a given URL now depends on Accept, so caches must // key on it. Without this a CDN can hand an agent's markdown response to // the next browser that asks for the same page. 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..01f8ea1 --- /dev/null +++ b/headers_test.go @@ -0,0 +1,219 @@ +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")) + }) + + // The rules must not be able to strip Vary and let a CDN cross-serve + // markdown to a browser. + t.Run("Vary: Accept survives", 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, "Accept", rec.Header().Get("Vary")) + }) +} diff --git a/main.go b/main.go index 4e434b8..c9bcc2a 100644 --- a/main.go +++ b/main.go @@ -35,6 +35,11 @@ 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) + ctx.Logger.Infof("loaded %d %s rule(s)", len(handler.headerRules), headersFileName) + return nil }) From 7444624f0ef93ae0d0593a57f58f21aa6fc7df70 Mon Sep 17 00:00:00 2001 From: aryanmehrotra Date: Fri, 31 Jul 2026 18:38:45 +0530 Subject: [PATCH 4/8] fix: fall back to defaults when config values are present but empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by running the actual distroless image rather than a native binary. `GetOrDefault` only falls back when a key is ABSENT. The shipped configs/.env sets STATIC_DIR_PATH= and DEFAULT_EXTENSION= with empty values, so a deployment that supplies STATIC_DIR_PATH through the environment got "" instead — every path lookup silently rooted at the process working directory. In that shape the server still served pages but loaded zero _headers rules, which is exactly the kind of half-working state that never gets noticed. The default shape (./static, where a Dockerfile typically copies the published directory) was unaffected and loaded all 22 rules. This makes the other shapes behave the same. The startup line now names the resolved directory alongside the count. "0 rules" is normal for a site with no _headers file and indistinguishable from a misrooted path unless the path is on the line too. Verified in the real gcr.io/distroless/static-debian12 image against a 5,978-page build, 16/16: correct Content-Type for html/css/svg/txt/md with no /etc/mime.types present, all _headers rules applied including on 404s, markdown negotiation, Vary, and a 138-byte markdown 404. --- main.go | 16 +++++++++++++++- main_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/main.go b/main.go index c9bcc2a..4bf0e78 100644 --- a/main.go +++ b/main.go @@ -17,10 +17,21 @@ const rootPath = "/" func main() { app := gofr.New() + // GetOrDefault only falls back when the key is absent, so a key present but + // empty — which the shipped configs/.env has for all four of these — yields + // an empty path rather than the default. That silently roots every lookup + // at the process working directory. Treat empty as unset. staticFilePath := app.Config.GetOrDefault("STATIC_DIR_PATH", defaultStaticFilePath) + if staticFilePath == "" { + staticFilePath = defaultStaticFilePath + } + spaMode, _ := strconv.ParseBool(app.Config.GetOrDefault("SPA_MODE", "false")) defaultExtension := app.Config.GetOrDefault("DEFAULT_EXTENSION", htmlExtension) + if defaultExtension == "" { + defaultExtension = htmlExtension + } handler := &staticFileHandler{ staticFilePath: staticFilePath, @@ -38,7 +49,10 @@ func main() { // 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) - ctx.Logger.Infof("loaded %d %s rule(s)", len(handler.headerRules), headersFileName) + // 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..07613f0 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,33 @@ 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. +func TestEmptyConfigValuesFallBackToDefaults(t *testing.T) { + tests := []struct { + name string + value string + fallback string + want string + }{ + {"empty static path falls back", "", defaultStaticFilePath, defaultStaticFilePath}, + {"empty extension falls back", "", htmlExtension, htmlExtension}, + {"a real value is kept", "/static", defaultStaticFilePath, "/static"}, + {"a real extension is kept", ".htm", htmlExtension, ".htm"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.value + if got == "" { + got = tt.fallback + } + + assert.Equal(t, tt.want, got) + }) + } +} From fb19ca88e8a76dd71bf5ec57c474d2ee177330df Mon Sep 17 00:00:00 2001 From: aryanmehrotra Date: Mon, 3 Aug 2026 13:02:12 +0530 Subject: [PATCH 5/8] fix: narrow Vary, markdown labelling and cache rules to where they belong MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three header changes reached responses that cannot benefit from them. Each was found by diffing the real binaries against origin/main rather than by reading the diff — the earlier comparison covered status, Content-Type and body only, so nothing had ever checked Vary or Cache-Control. Vary: Accept was sent on every response, including the hashed bundles under /_astro/ that the same _headers file marks immutable. Only an extensionless route can resolve to a .md sibling, so everything else was keying caches on a header that cannot change what they return — the cost landing precisely on the responses this server most wants cached. It is now gated on the same condition resolveFilePath uses to negotiate, and negotiable() is the one definition of that condition so the two cannot drift apart. A miss still varies whatever the path looks like, because a miss is answered in markdown whenever the client asked for it — including for extensions that never negotiate on a hit. Without that a cache can hand an agent the HTML shell it stored for a browser. A site's Cache-Control reached its 404s. `/*.html` with max-age=300 meant a file merely not propagated yet during a deploy was pinned into every cache downstream for the rule's lifetime. Cache directives are now dropped on a miss. The security headers still apply — a 404 that leaks framing protection is as exploitable as a 200 that does — and the SPA fallback keeps its caching, since that is a real route being served, not a miss. Directly requested .md files were relabelled text/plain -> text/markdown. Bodies were identical, which is how it read as a no-op, but browsers render text/plain inline and download text/markdown: every existing .md link on a site would have turned into a download prompt. The explicit type is now set only for a negotiated response, which is the case that needs it — Go's MIME table has no .md entry and distroless has no /etc/mime.types. Verified against base and patched binaries on the same build across 13 path x Accept combinations, comparing status, Content-Type, Vary, Cache-Control, X-Frame-Options, Content-Length and body SHA-256. The only changed or removed field in the whole matrix is the one intended negotiation; every other delta is a header the site's own _headers file declares. Six mutations go red: Vary unconditional on hits, no Vary on a miss, the Cache-Control strip removed, the markdown Content-Type unscoped, and negotiable() ignoring either the extension or the root. golangci-lint output is identical to origin/main (4 findings, all pre-existing). --- handler.go | 64 +++++++++++++++++++++---------- headers_test.go | 44 +++++++++++++++++++++- main.go | 8 ++-- negotiate.go | 16 ++++++++ negotiate_test.go | 96 ++++++++++++++++++++++++++++++++++++++++++----- 5 files changed, 193 insertions(+), 35 deletions(-) diff --git a/handler.go b/handler.go index 2fe40ca..ef17d0f 100644 --- a/handler.go +++ b/handler.go @@ -36,42 +36,66 @@ func (h *staticFileHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // where they are load-bearing. h.headerRules.apply(w.Header(), r.URL.Path) - // The response body for a given URL now depends on Accept, so caches must - // key on it. Without this a CDN can hand an agent's markdown response to - // the next browser that asks for the same page. - w.Header().Add("Vary", "Accept") - if _, err := h.fs.Stat(filePath); err != nil { if h.spaMode && !hasExtension { http.ServeFile(w, r, filepath.Join(h.staticFilePath, indexHTML)) return } - // 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")) + 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) { + w.Header().Add("Vary", "Accept") + } + // http.ServeFile only sniffs a Content-Type when one is not already set, - // so setting it here wins. Applies to negotiated and directly-requested - // .md alike — neither can rely on the base image having /etc/mime.types. - if strings.HasSuffix(filePath, markdownExtension) { + // so setting it here wins. Scoped to a negotiated response on purpose: a + // directly requested .md keeps the type it resolves to today, because + // browsers render text/plain inline but download text/markdown, and + // relabelling would turn every existing .md link into a download prompt. + if wantsMarkdown && negotiable(r.URL.Path) && strings.HasSuffix(filePath, markdownExtension) { w.Header().Set("Content-Type", markdownContentType) } http.ServeFile(w, r, filePath) } +// 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. + w.Header().Add("Vary", "Accept") + + // A site's Cache-Control is written for the pages it publishes, not for the + // ones it does not have. Letting a `/*` rule reach here would pin a + // transient miss — a file not yet propagated mid-deploy — into every cache + // between this server and the reader for the rule's full lifetime. The + // security headers still apply: a 404 that leaks framing protection is as + // exploitable as a 200 that does. + w.Header().Del("Cache-Control") + + // 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 @@ -106,7 +130,7 @@ func (h *staticFileHandler) resolveFilePath(urlPath string, wantsMarkdown bool) // // 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 !hasExtension && urlPath != rootPath && wantsMarkdown { + if wantsMarkdown && negotiable(urlPath) { if _, err := h.fs.Stat(filePath + markdownExtension); err == nil { return filePath + markdownExtension, true } diff --git a/headers_test.go b/headers_test.go index 01f8ea1..f1c04f4 100644 --- a/headers_test.go +++ b/headers_test.go @@ -207,13 +207,53 @@ func TestServeHTTPAppliesHeaderRules(t *testing.T) { 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", func(t *testing.T) { - req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/style.css", http.NoBody) + 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") + }) } diff --git a/main.go b/main.go index 4bf0e78..97676fc 100644 --- a/main.go +++ b/main.go @@ -18,9 +18,11 @@ func main() { app := gofr.New() // GetOrDefault only falls back when the key is absent, so a key present but - // empty — which the shipped configs/.env has for all four of these — yields - // an empty path rather than the default. That silently roots every lookup - // at the process working directory. Treat empty as unset. + // empty — which the shipped configs/.env has for all three settings below — + // yields an empty value rather than the default. For the path, that silently + // roots every lookup at the process working directory. Treat empty as unset. + // (SPA_MODE needs no guard: ParseBool rejects "" and leaves the same false + // the default would have produced.) staticFilePath := app.Config.GetOrDefault("STATIC_DIR_PATH", defaultStaticFilePath) if staticFilePath == "" { staticFilePath = defaultStaticFilePath diff --git a/negotiate.go b/negotiate.go index a6d38a5..7ba568b 100644 --- a/negotiate.go +++ b/negotiate.go @@ -2,6 +2,7 @@ package main import ( "mime" + "path/filepath" "strconv" "strings" ) @@ -22,6 +23,21 @@ const ( 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) == "" +} + // acceptEntry is one parsed media range from an Accept header. type acceptEntry struct { mediaType string diff --git a/negotiate_test.go b/negotiate_test.go index 79a1969..4bbc807 100644 --- a/negotiate_test.go +++ b/negotiate_test.go @@ -146,7 +146,7 @@ func TestServeHTTPMarkdownNegotiation(t *testing.T) { assert.Contains(t, rec.Header().Get("Content-Type"), "text/html") }) - t.Run("Vary: Accept is always set so caches do not cross-serve", func(t *testing.T) { + 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) @@ -154,15 +154,6 @@ func TestServeHTTPMarkdownNegotiation(t *testing.T) { assert.Equal(t, "Accept", rec.Header().Get("Vary")) }) - t.Run("direct .md request keeps working", 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") - }) - 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") @@ -191,3 +182,88 @@ func TestServeHTTPMarkdownNegotiation(t *testing.T) { assert.NotContains(t, rec.Header().Get("Content-Type"), "markdown") }) } + +// TestMarkdownContentTypeScope pins which responses get relabelled as markdown. +// +// The scope is deliberately narrow. A directly requested .md is not a +// negotiated response and must keep the type it resolves to today: browsers +// render text/plain inline but download text/markdown, so relabelling it would +// turn every existing .md link on a site into a download prompt. +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(), + } + } + + t.Run("direct .md keeps working and keeps its 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.NotContains(t, rec.Header().Get("Content-Type"), "text/markdown", + "a direct .md must keep the type it has today") + assert.Empty(t, rec.Header().Get("Vary"), "a direct .md never negotiates") + }) + + // The same bytes reached two ways: only the negotiated route relabels them. + t.Run("the same file is text/markdown only when negotiated", 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.Contains(t, negRec.Header().Get("Content-Type"), "text/markdown") + assert.NotContains(t, dirRec.Header().Get("Content-Type"), "text/markdown") + }) + + // 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") + }) +} From bd8031e68bfa3167dcd8d48272bb16443ad34278 Mon Sep 17 00:00:00 2001 From: aryanmehrotra Date: Mon, 3 Aug 2026 13:15:29 +0530 Subject: [PATCH 6/8] fix: test the markdown labelling decision, not the host's MIME table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught what a macOS run could not. The previous commit asserted that a directly requested .md keeps a non-markdown Content-Type — true on macOS and in the distroless image that ships, false on the Ubuntu runner, because most Linux distributions map .md in /etc/mime.types and http.ServeFile answers text/markdown there before this server does anything. The assertion pinned the host's MIME table rather than any behaviour of ours, so it passed locally and failed in CI. The scoping itself was right and is unchanged. What changes is how it is checked: the condition moves into labelAsMarkdown, covered by a table that runs identically everywhere, and the end-to-end test now compares a direct .md against mime.TypeByExtension — the same lookup ServeFile makes — instead of against a hardcoded type. Where the platform has no entry that is text/plain, which is the production case and the one the scoping exists for; where it has one, the test agrees with it. A negotiated response is asserted to carry the explicit type on every platform, since that is the case that must not depend on the base image having a MIME table at all. Verified by running the suite under golang:1.26 with media-types installed, so /etc/mime.types really did contain `text/markdown md markdown` — the exact shape that failed. All six mutations still go red; golangci-lint remains identical to origin/main. --- handler.go | 8 +++--- negotiate.go | 19 ++++++++++++++ negotiate_test.go | 66 ++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 76 insertions(+), 17 deletions(-) diff --git a/handler.go b/handler.go index ef17d0f..e3d34df 100644 --- a/handler.go +++ b/handler.go @@ -57,11 +57,9 @@ func (h *staticFileHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } // http.ServeFile only sniffs a Content-Type when one is not already set, - // so setting it here wins. Scoped to a negotiated response on purpose: a - // directly requested .md keeps the type it resolves to today, because - // browsers render text/plain inline but download text/markdown, and - // relabelling would turn every existing .md link into a download prompt. - if wantsMarkdown && negotiable(r.URL.Path) && strings.HasSuffix(filePath, markdownExtension) { + // 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) } diff --git a/negotiate.go b/negotiate.go index 7ba568b..53b6f3d 100644 --- a/negotiate.go +++ b/negotiate.go @@ -38,6 +38,25 @@ func negotiable(urlPath string) bool { return urlPath != rootPath && filepath.Ext(urlPath) == "" } +// 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 diff --git a/negotiate_test.go b/negotiate_test.go index 4bbc807..8ae9ba6 100644 --- a/negotiate_test.go +++ b/negotiate_test.go @@ -1,6 +1,7 @@ package main import ( + "mime" "net/http" "net/http/httptest" "os" @@ -183,12 +184,45 @@ func TestServeHTTPMarkdownNegotiation(t *testing.T) { }) } -// TestMarkdownContentTypeScope pins which responses get relabelled as markdown. +// TestLabelAsMarkdown pins which responses this server relabels as markdown. // -// The scope is deliberately narrow. A directly requested .md is not a -// negotiated response and must keep the type it resolves to today: browsers -// render text/plain inline but download text/markdown, so relabelling it would -// turn every existing .md link on a site into a download prompt. +// 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)) @@ -202,7 +236,15 @@ func TestMarkdownContentTypeScope(t *testing.T) { } } - t.Run("direct .md keeps working and keeps its type", func(t *testing.T) { + // 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() @@ -210,13 +252,14 @@ func TestMarkdownContentTypeScope(t *testing.T) { assert.Equal(t, http.StatusOK, rec.Code) assert.Contains(t, rec.Body.String(), "markdown source") - assert.NotContains(t, rec.Header().Get("Content-Type"), "text/markdown", - "a direct .md must keep the type it has today") + 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") }) - // The same bytes reached two ways: only the negotiated route relabels them. - t.Run("the same file is text/markdown only when negotiated", func(t *testing.T) { + // 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") @@ -230,8 +273,7 @@ func TestMarkdownContentTypeScope(t *testing.T) { newHandler().ServeHTTP(dirRec, direct) assert.Equal(t, negRec.Body.String(), dirRec.Body.String(), "same bytes either way") - assert.Contains(t, negRec.Header().Get("Content-Type"), "text/markdown") - assert.NotContains(t, dirRec.Header().Get("Content-Type"), "text/markdown") + assert.Equal(t, markdownContentType, negRec.Header().Get("Content-Type")) }) // The root is served straight from index.html and never negotiates, so it From f70acf8d191742e45f72b8cb93c0c580e2a60531 Mon Sep 17 00:00:00 2001 From: aryanmehrotra Date: Mon, 3 Aug 2026 13:29:03 +0530 Subject: [PATCH 7/8] fix: apply the site's headers to delegated .well-known paths too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit .well-known is handed to the next handler untouched, so that an ACME challenge is not given an extension or swallowed by the SPA fallback. That early return also skipped the _headers rules, so a site declaring X-Frame-Options for `/*` got it everywhere except there. A `/*` block means the whole site, and a path this server delegates is still a path it answered for. The rules are now applied before the branch, which is where they belonged: they depend only on the request path, not on anything the resolution step produces. Delegation itself is unchanged — the path reaches the next handler exactly as before, with no rewriting. That raises the question the miss path already answered: the site's Cache-Control must not attach to a response that is not a page the site publishes. Here the status is chosen by the delegate, so it cannot be decided up front, and the directives are withdrawn on the way out instead — a delegated 200 is a real file and keeps its caching, a delegated 404 does not. Both paths now call withdrawCacheDirectives, so the rule has one definition and one rationale rather than two that can drift. The writer is wrapped only for the delegated paths. Wrapping the main serving path would hide net/http's io.ReaderFrom from http.ServeFile and cost every static file its sendfile fast path; ACME challenges are small and rare enough not to be worth a special case to keep fast. Verified against the real binary with GoFr's own chain as the delegate: on main an ACME challenge file comes back with no security headers at all, and with this change it carries X-Frame-Options while still returning the token body intact, while an absent .well-known path 404s with the security headers and without Cache-Control. Four mutations go red: skipping the rules for .well-known, dropping the wrapper, scrubbing on every status rather than errors only, and making withdrawCacheDirectives a no-op. golangci-lint reports fewer findings than origin/main rather than more (1 vs 4). The three that went are gosec G703 taint-analysis hits on http.ServeFile, whose call sites this commit does not touch — gosec's taint walk is sensitive to unrelated edits in the same function. --- handler.go | 62 ++++++++++++++++++++++++++++-------- headers_test.go | 85 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 13 deletions(-) diff --git a/handler.go b/handler.go index e3d34df..bafb27f 100644 --- a/handler.go +++ b/handler.go @@ -21,8 +21,20 @@ type staticFileHandler struct { } 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 } @@ -31,11 +43,6 @@ func (h *staticFileHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { filePath, hasExtension := h.resolveFilePath(r.URL.Path, wantsMarkdown) - // Applied before anything writes, so it covers hits, misses and the SPA - // fallback alike. Set first so the server's own headers below still win - // where they are load-bearing. - h.headerRules.apply(w.Header(), r.URL.Path) - if _, err := h.fs.Stat(filePath); err != nil { if h.spaMode && !hasExtension { http.ServeFile(w, r, filepath.Join(h.staticFilePath, indexHTML)) @@ -73,13 +80,7 @@ func (h *staticFileHandler) serveNotFound(w http.ResponseWriter, r *http.Request // extensions that never negotiate on a hit. w.Header().Add("Vary", "Accept") - // A site's Cache-Control is written for the pages it publishes, not for the - // ones it does not have. Letting a `/*` rule reach here would pin a - // transient miss — a file not yet propagated mid-deploy — into every cache - // between this server and the reader for the rule's full lifetime. The - // security headers still apply: a 404 that leaks framing protection is as - // exploitable as a 200 that does. - w.Header().Del("Cache-Control") + 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, @@ -147,6 +148,41 @@ func (h *staticFileHandler) resolveFilePath(urlPath string, wantsMarkdown 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/headers_test.go b/headers_test.go index f1c04f4..5af29b6 100644 --- a/headers_test.go +++ b/headers_test.go @@ -257,3 +257,88 @@ func TestServeHTTPAppliesHeaderRules(t *testing.T) { 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") + }) +} From 75f4c8a7c4ce1c33c09da611209ef38f0993767f Mon Sep 17 00:00:00 2001 From: aryanmehrotra Date: Mon, 3 Aug 2026 18:24:54 +0530 Subject: [PATCH 8/8] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20docum?= =?UTF-8?q?ent=20the=20features,=20and=20test=20the=20config=20guard=20for?= =?UTF-8?q?=20real?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four items from review, all confirmed against the code before acting. The empty-config test was vacuous, and the mutation narrative made that worse rather than better. It re-implemented the fallback in its own body: got := tt.value if got == "" { got = tt.fallback } assert.Equal(t, tt.want, got) which passes by construction and never touches main.go. Deleting both guards from main() left the suite green — the one bug that commit fixes had no test at all. The resolution now lives in resolveOrDefault, taking the narrow config interface it needs, and is table-tested through the real function against a fake whose keys are present-but-empty, which is the distinction that matters and the one configs/.env actually ships. Removing the guard now fails. The SPA fallback could serve a negotiable route without Vary: Accept. With foo.md on disk and no HTML page beside it, markdown clients take the hit path and browsers land on the shell, so one URL yields two bodies while only one of them said it varies — a shared cache could hand the shell to an agent. Reproduced against a running server before fixing. Vary is now advertised through one helper, which keeps Add over Set: a site's _headers may declare its own Vary and Set would discard it, while repeated field lines are combined by caches, so a declared `Vary: Accept-Encoding` plus ours reads as `Accept-Encoding, Accept`. What the helper adds is idempotence, so a site that already named Accept does not end up with `Accept, Accept`. The README documented neither content negotiation nor _headers, though it is the repo's only doc surface and already carries a behaviour contract. Shipping _headers support undocumented is the same failure this PR argues against for _headers itself. Both are now described, including the Vary scoping, the Cache-Control-on-miss rule, `.well-known`, and the empty-value config fallback. Every claim in those sections was checked against a running server rather than written from memory: pattern anchoring both ways, no Vary on assets, a site-declared Vary surviving, Cache-Control withdrawn from a miss while X-Frame-Options is not, the legacy text/x-markdown spelling, and the startup log. Mutation coverage extended to all of it: resolveOrDefault ignoring empty, the SPA fallback dropping Vary or advertising it unconditionally, and the idempotence guard removed. The unconditional case is only observable for a root with no index.html, so that shape is pinned explicitly rather than left as an unverified assertion. Suite green under golang:1.26 with media-types installed; golangci-lint unchanged at 1 finding against origin/main's 4. --- README.md | 42 +++++++++++++++ handler.go | 14 ++++- main.go | 41 ++++++++++----- main_test.go | 42 ++++++++++----- negotiate.go | 22 ++++++++ negotiate_test.go | 127 ++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 260 insertions(+), 28 deletions(-) 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 bafb27f..5ef476a 100644 --- a/handler.go +++ b/handler.go @@ -45,7 +45,17 @@ func (h *staticFileHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { 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 } @@ -60,7 +70,7 @@ func (h *staticFileHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // 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) { - w.Header().Add("Vary", "Accept") + advertiseAcceptVaries(w.Header()) } // http.ServeFile only sniffs a Content-Type when one is not already set, @@ -78,7 +88,7 @@ func (h *staticFileHandler) serveNotFound(w http.ResponseWriter, r *http.Request // 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. - w.Header().Add("Vary", "Accept") + advertiseAcceptVaries(w.Header()) withdrawCacheDirectives(w.Header()) diff --git a/main.go b/main.go index 97676fc..b1aba58 100644 --- a/main.go +++ b/main.go @@ -14,26 +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() - // GetOrDefault only falls back when the key is absent, so a key present but - // empty — which the shipped configs/.env has for all three settings below — - // yields an empty value rather than the default. For the path, that silently - // roots every lookup at the process working directory. Treat empty as unset. - // (SPA_MODE needs no guard: ParseBool rejects "" and leaves the same false - // the default would have produced.) - staticFilePath := app.Config.GetOrDefault("STATIC_DIR_PATH", defaultStaticFilePath) - if staticFilePath == "" { - staticFilePath = 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) - if defaultExtension == "" { - defaultExtension = htmlExtension - } + defaultExtension := resolveOrDefault(app.Config, "DEFAULT_EXTENSION", htmlExtension) handler := &staticFileHandler{ staticFilePath: staticFilePath, diff --git a/main_test.go b/main_test.go index 07613f0..beecf7c 100644 --- a/main_test.go +++ b/main_test.go @@ -63,27 +63,45 @@ func TestServer(t *testing.T) { // 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. -func TestEmptyConfigValuesFallBackToDefaults(t *testing.T) { +// 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 - value string + cfg fakeConfig + key string fallback string want string }{ - {"empty static path falls back", "", defaultStaticFilePath, defaultStaticFilePath}, - {"empty extension falls back", "", htmlExtension, htmlExtension}, - {"a real value is kept", "/static", defaultStaticFilePath, "/static"}, - {"a real extension is kept", ".htm", htmlExtension, ".htm"}, + // 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) { - got := tt.value - if got == "" { - got = tt.fallback - } - - assert.Equal(t, tt.want, got) + assert.Equal(t, tt.want, resolveOrDefault(tt.cfg, tt.key, tt.fallback)) }) } } diff --git a/negotiate.go b/negotiate.go index 53b6f3d..82bb5d8 100644 --- a/negotiate.go +++ b/negotiate.go @@ -2,6 +2,7 @@ package main import ( "mime" + "net/http" "path/filepath" "strconv" "strings" @@ -38,6 +39,27 @@ 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. // diff --git a/negotiate_test.go b/negotiate_test.go index 8ae9ba6..99932d3 100644 --- a/negotiate_test.go +++ b/negotiate_test.go @@ -309,3 +309,130 @@ func TestMarkdownContentTypeScope(t *testing.T) { 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")) +}