From a34c251b0b0c6b04c42a31b617c616a5e7b9c1ad Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Tue, 20 Oct 2026 03:15:26 +0200 Subject: [PATCH] test(surf): cover error page negotiation, unchanged API bodies and overrides Negotiation table (Sec-Fetch-Mode, X-Requested-With, Accept q values, the 32-range cap, /api, /_ and admin paths), byte equality with the http.error_pages.enabled false router for curl, fetch, XHR and JSON clients, page content and headers for every covered status, details gating and escaping, renderer override and fallbacks, the admin prefix and the uploads route. --- modules/surf/errorpage_test.go | 469 +++++++++++++++++++++++++++++---- 1 file changed, 423 insertions(+), 46 deletions(-) diff --git a/modules/surf/errorpage_test.go b/modules/surf/errorpage_test.go index 699cf898..e044d023 100644 --- a/modules/surf/errorpage_test.go +++ b/modules/surf/errorpage_test.go @@ -1,11 +1,14 @@ package surf import ( + "maps" "net/http" "net/http/httptest" + "strconv" "strings" "testing" + "git.golem15.com/golem15/summercms/modules/backpack" "git.golem15.com/golem15/summercms/modules/pact" "git.golem15.com/golem15/summercms/modules/party" ) @@ -63,63 +66,437 @@ func TestErrorPageTracer(t *testing.T) { } } -// TestErrorPageCallSites: surf's own 404 and panic recovery negotiate too, -// and http.error_pages.enabled false restores the stdlib text. -func TestErrorPageCallSites(t *testing.T) { - routes := routesPluginFunc{id: "acme.blog", routes: func(r pact.Router) { - r.Get("/posts/{id}", func(w http.ResponseWriter, _ *http.Request) {}) - r.Where("id", "[0-9]+") - r.Get("/boom", func(http.ResponseWriter, *http.Request) { panic("blog: secret panic detail") }) - r.GroupRaw("/raw", nil, func(g pact.Router) { - g.Get("/boom", func(http.ResponseWriter, *http.Request) { panic("blog: secret panic detail") }) +// browserReq is a top-level Chrome navigation. +func browserReq(method, path string) *http.Request { + req := httptest.NewRequest(method, path, nil) + req.Header.Set("Accept", chromeAccept) + req.Header.Set("Sec-Fetch-Mode", "navigate") + return req +} + +// curlReq is a curl-style request. +func curlReq(method, path string) *http.Request { + req := httptest.NewRequest(method, path, nil) + req.Header.Set("Accept", "*/*") + return req +} + +func serveReq(h http.Handler, req *http.Request) *httptest.ResponseRecorder { + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + return rec +} + +const errorPagePanic = "blog: secret panic detail" + +// errorPageRoutes covers every site the error page negotiates: a POST-only +// route (405), a constrained route, panics in a normal and a raw group and +// an overlap family. +var errorPageRoutes = routesPluginFunc{id: "acme.blog", routes: func(r pact.Router) { + ok := func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte("ok")) } + boom := func(http.ResponseWriter, *http.Request) { panic(errorPagePanic) } + r.Get("/api/ping", ok) + r.Post("/blog/submit", ok) + r.Get("/posts/{id}", ok) + r.Where("id", "[0-9]+") + r.Get("/boom", boom) + r.GroupRaw("/raw", nil, func(g pact.Router) { + g.Get("/boom", boom) + }) + r.Get("/shelves/items/{id}", ok) + r.Where("id", "[0-9]+") + r.Get("/shelves/{shelfId}/follow", ok) + r.Where("shelfId", "[0-9]+") +}} + +// buildErrorPageRouter assembles plugins on a transportApp; extra files are +// merged over http.yaml with the body limits. +func buildErrorPageRouter(t *testing.T, env string, files map[string]string, publish func(*backpack.App), plugins ...party.Plugin) http.Handler { + t.Helper() + all := map[string]string{"http.yaml": transportBodyLimits} + maps.Copy(all, files) + app := transportApp(t, env, all) + if publish != nil { + publish(app) + } + h, err := Assemble(app, plugins) + if err != nil { + t.Fatal(err) + } + return h +} + +// twoRouters returns the default router and the same one with +// http.error_pages.enabled false. +func twoRouters(t *testing.T, env string, files map[string]string, plugins ...party.Plugin) (on, off http.Handler) { + t.Helper() + offFiles := map[string]string{} + maps.Copy(offFiles, files) + offFiles["http.yaml"] = transportBodyLimits + offFiles["http.yaml"] + "error_pages:\n enabled: false\n" + return buildErrorPageRouter(t, env, files, nil, plugins...), buildErrorPageRouter(t, env, offFiles, nil, plugins...) +} + +func TestErrorPageNegotiation(t *testing.T) { + pages := &errorPages{} + pages.setAdminPrefix("/backend") + type hdr = map[string]string + nav := hdr{"Accept": chromeAccept, "Sec-Fetch-Mode": "navigate"} + ranges40 := make([]string, 40) + for i := range ranges40 { + ranges40[i] = "application/x-acme-" + string(rune('a'+i%26)) + } + ranges40[34] = "text/html" + for _, tc := range []struct { + name string + method string + path string + headers hdr + want bool + }{ + {"chrome navigation", "GET", "/nowhere", nav, true}, + {"POST navigation", "POST", "/nowhere", nav, true}, + {"GET text/html without Sec-Fetch", "GET", "/nowhere", hdr{"Accept": "text/html"}, true}, + {"HEAD text/html without Sec-Fetch", "HEAD", "/nowhere", hdr{"Accept": "text/html"}, true}, + {"html above json", "GET", "/nowhere", hdr{"Accept": "text/html, application/json;q=0.5"}, true}, + {"xhtml only", "GET", "/nowhere", hdr{"Accept": "application/xhtml+xml"}, true}, + {"path next to the admin prefix", "GET", "/backendx", nav, true}, + {"curl */*", "GET", "/nowhere", hdr{"Accept": "*/*"}, false}, + {"no Accept", "GET", "/nowhere", hdr{}, false}, + {"text/* only", "GET", "/nowhere", hdr{"Accept": "text/*"}, false}, + {"fetch with text/html", "GET", "/nowhere", hdr{"Accept": "text/html", "Sec-Fetch-Mode": "cors"}, false}, + {"XHR", "GET", "/nowhere", hdr{"Accept": "text/html", "X-Requested-With": "XMLHttpRequest"}, false}, + {"empty X-Requested-With", "GET", "/nowhere", hdr{"Accept": "text/html", "X-Requested-With": ""}, false}, + {"json", "GET", "/nowhere", hdr{"Accept": "application/json"}, false}, + {"json and html tied", "GET", "/nowhere", hdr{"Accept": "application/json, text/html"}, false}, + {"json above html", "GET", "/nowhere", hdr{"Accept": "text/html;q=0.5, application/json;q=0.9"}, false}, + {"html q=0", "GET", "/nowhere", hdr{"Accept": "text/html;q=0"}, false}, + {"text/json above html", "GET", "/nowhere", hdr{"Accept": "text/json, text/html;q=0.9"}, false}, + {"+json above html", "GET", "/nowhere", hdr{"Accept": "application/vnd.api+json, text/html;q=0.1"}, false}, + {"malformed q skips html", "GET", "/nowhere", hdr{"Accept": "text/html;q=high"}, false}, + {"POST without Sec-Fetch", "POST", "/nowhere", hdr{"Accept": "text/html"}, false}, + {"/api", "GET", "/api", nav, false}, + {"/api/...", "GET", "/api/blog/posts", nav, false}, + {"/_plugin", "GET", "/_acme/feed", nav, false}, + {"admin prefix", "GET", "/backend", nav, false}, + {"under the admin prefix", "GET", "/backend/users", nav, false}, + {"html past the first 32 ranges", "GET", "/nowhere", hdr{"Accept": strings.Join(ranges40, ", ")}, false}, + } { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest(tc.method, tc.path, nil) + for k, v := range tc.headers { + req.Header.Set(k, v) + } + if got := pages.wants(req); got != tc.want { + t.Fatalf("wants = %v, want %v", got, tc.want) + } }) - }} - build := func(t *testing.T, httpYAML string) http.Handler { + } + var nilPages *errorPages + if nilPages.wants(browserReq("GET", "/nowhere")) { + t.Fatal("a nil *errorPages must never want the page") + } +} + +func TestErrorPagesKeepAPIBodies(t *testing.T) { + on, off := twoRouters(t, "production", map[string]string{}, errorPageRoutes) + targets := []struct { + method, path string + status int + }{ + {"GET", "/nowhere", 404}, + {"GET", "/blog/submit", 405}, + {"GET", "/posts/abc", 404}, + {"GET", "/boom", 500}, + {"GET", "/raw/boom", 500}, + {"GET", "/shelves/items/abc", 404}, + {"POST", "/shelves/items/7", 405}, + {"GET", "/api/nowhere", 404}, + } + clients := []struct { + name string + headers map[string]string + }{ + {"curl", map[string]string{"Accept": "*/*"}}, + {"no Accept", map[string]string{}}, + {"fetch", map[string]string{"Accept": "*/*", "Sec-Fetch-Mode": "cors"}}, + {"XHR", map[string]string{"Accept": "text/html, */*", "X-Requested-With": "XMLHttpRequest"}}, + {"JSON", map[string]string{"Accept": "application/json"}}, + } + for _, tgt := range targets { + for _, c := range clients { + t.Run(tgt.method+" "+tgt.path+" "+c.name, func(t *testing.T) { + req := func() *http.Request { + req := httptest.NewRequest(tgt.method, tgt.path, nil) + for k, v := range c.headers { + req.Header.Set(k, v) + } + return req + } + a, b := serveReq(on, req()), serveReq(off, req()) + if a.Code != tgt.status || b.Code != tgt.status { + t.Fatalf("status on=%d off=%d, want %d", a.Code, b.Code, tgt.status) + } + if a.Body.String() != b.Body.String() { + t.Fatalf("body on=%q off=%q", a.Body.String(), b.Body.String()) + } + for _, k := range []string{"Content-Type", "Allow", "Vary", "Cache-Control", "Content-Security-Policy", "X-Content-Type-Options"} { + if a.Header().Get(k) != b.Header().Get(k) { + t.Fatalf("%s on=%q off=%q", k, a.Header().Get(k), b.Header().Get(k)) + } + } + }) + } + } + for _, tc := range []struct { + method, path, body string + }{ + {"GET", "/nowhere", "404 page not found\n"}, + {"GET", "/blog/submit", "Method Not Allowed\n"}, + {"POST", "/shelves/items/7", "Method Not Allowed\n"}, + {"GET", "/boom", `{"error":true,"message":"Internal server error"}`}, + {"GET", "/raw/boom", ""}, + } { + if got := serveReq(on, curlReq(tc.method, tc.path)).Body.String(); got != tc.body { + t.Fatalf("curl %s %s body = %q, want %q", tc.method, tc.path, got, tc.body) + } + } + // The kill switch: browsers get today's bytes too. + rec := serveReq(off, browserReq("GET", "/nowhere")) + if rec.Code != 404 || rec.Body.String() != "404 page not found\n" || rec.Header().Get("Content-Type") != "text/plain; charset=utf-8" { + t.Fatalf("disabled browser: %d %q %q", rec.Code, rec.Header().Get("Content-Type"), rec.Body.String()) + } + rec = serveReq(off, browserReq("GET", "/boom")) + if rec.Code != 500 || rec.Body.String() != `{"error":true,"message":"Internal server error"}` { + t.Fatalf("disabled browser panic: %d %q", rec.Code, rec.Body.String()) + } +} + +// checkErrorPage asserts the built-in page's status, headers and body. +func checkErrorPage(t *testing.T, rec *httptest.ResponseRecorder, status int) { + t.Helper() + if rec.Code != status { + t.Fatalf("status = %d, want %d", rec.Code, status) + } + h := rec.Header() + if got := h.Get("Content-Type"); got != "text/html; charset=utf-8" { + t.Fatalf("Content-Type = %q", got) + } + if h.Get("Cache-Control") != "no-store" || h.Get("X-Content-Type-Options") != "nosniff" { + t.Fatalf("Cache-Control = %q, X-Content-Type-Options = %q", h.Get("Cache-Control"), h.Get("X-Content-Type-Options")) + } + for _, v := range []string{"Accept", "Sec-Fetch-Mode", "X-Requested-With"} { + if !varyHas(h, v) { + t.Fatalf("Vary %v lacks %s", h.Values("Vary"), v) + } + } + if csp := h.Get("Content-Security-Policy"); csp != errorPageCSP || !strings.Contains(csp, "frame-ancestors 'none'") || strings.Contains(csp, "script-src") { + t.Fatalf("Content-Security-Policy = %q", csp) + } +} + +func TestErrorPageHTML(t *testing.T) { + on, off := twoRouters(t, "production", map[string]string{}, errorPageRoutes) + for _, tc := range []struct { + method, path string + status int + title string + }{ + {"GET", "/nowhere", 404, "Page not found"}, + {"GET", "/blog/submit", 405, "Method not allowed"}, + {"GET", "/posts/abc", 404, "Page not found"}, + {"GET", "/boom", 500, "Something went wrong"}, + {"GET", "/raw/boom", 500, "Something went wrong"}, + {"GET", "/shelves/items/abc", 404, "Page not found"}, + {"POST", "/shelves/items/7", 405, "Method not allowed"}, + } { + t.Run(tc.method+" "+tc.path, func(t *testing.T) { + rec := serveReq(on, browserReq(tc.method, tc.path)) + checkErrorPage(t, rec, tc.status) + body := rec.Body.String() + for _, want := range []string{"", tc.title, `href="/"`, "data:image/webp;base64,", "Powered by SummerCMS"} { + if !strings.Contains(body, want) { + t.Fatalf("body lacks %q", want) + } + } + if !strings.Contains(body, `

`+strconv.Itoa(tc.status)+"

") { + t.Fatalf("body lacks the status %d", tc.status) + } + if strings.Contains(body, "GET", "
Request ID
"} { + if !strings.Contains(body, want) { + t.Fatalf("body lacks %q", want) + } + } + body = serveReq(h, browserReq("GET", marker)).Body.String() + if strings.Contains(body, "Request ID") { + t.Fatal("Request ID row shown without the header") + } + body = serveReq(h, browserReq("GET", "/%3Cb%3Ex%3C/b%3E")).Body.String() + if strings.Contains(body, "") || !strings.Contains(body, "<b>x</b>") { + t.Fatal("the path is not HTML-escaped") + } + body = serveReq(h, withID(browserReq("GET", marker), strings.Repeat("r", 300))).Body.String() + if !strings.Contains(body, strings.Repeat("r", 128)) || strings.Contains(body, strings.Repeat("r", 129)) { + t.Fatal("X-Request-Id is not cut to 128 bytes") + } + body = serveReq(h, browserReq("GET", "/boom")).Body.String() + if strings.Contains(body, errorPagePanic) { + t.Fatal("the panic value leaked onto the debug page") + } + }) + } +} + +// renderFunc adapts a func to ErrorPageRenderer. +type renderFunc func(*http.Request, ErrorPage) ([]byte, bool) + +func (f renderFunc) RenderErrorPage(r *http.Request, page ErrorPage) ([]byte, bool) { + return f(r, page) +} + +func TestErrorPageRendererOverride(t *testing.T) { + build := func(t *testing.T, renderer ErrorPageRenderer) http.Handler { + return buildErrorPageRouter(t, "production", nil, func(app *backpack.App) { + if err := app.Publish[ErrorPageRenderer](renderer); err != nil { + t.Fatal(err) + } + }, errorPageRoutes) + } + var seen ErrorPage + custom := build(t, renderFunc(func(_ *http.Request, page ErrorPage) ([]byte, bool) { + seen = page + if page.Status != http.StatusNotFound { + return nil, false + } + return []byte("

custom 404

"), true + })) + rec := serveReq(custom, browserReq("GET", "/nowhere")) + checkErrorPage(t, rec, 404) + if rec.Body.String() != "

custom 404

" { + t.Fatalf("custom body = %q", rec.Body.String()) + } + if seen.Status != 404 || seen.Title != "Page not found" || seen.Home != "/" || seen.ShowDetails || seen.Path != "" { + t.Fatalf("renderer got %+v", seen) + } + rec = serveReq(custom, browserReq("GET", "/boom")) + checkErrorPage(t, rec, 500) + if !strings.Contains(rec.Body.String(), "Something went wrong") { + t.Fatal("ok=false did not keep the built-in 500 page") + } + if rec := serveReq(custom, curlReq("GET", "/nowhere")); rec.Body.String() != "404 page not found\n" { + t.Fatalf("curl with a renderer: %q", rec.Body.String()) + } + + for name, renderer := range map[string]ErrorPageRenderer{ + "panicking renderer": renderFunc(func(*http.Request, ErrorPage) ([]byte, bool) { panic("theme: broken") }), + "empty body": renderFunc(func(*http.Request, ErrorPage) ([]byte, bool) { return nil, true }), + } { + t.Run(name, func(t *testing.T) { + h := build(t, renderer) + for path, status := range map[string]int{"/nowhere": 404, "/boom": 500} { + rec := serveReq(h, browserReq("GET", path)) + checkErrorPage(t, rec, status) + if !strings.Contains(rec.Body.String(), "Powered by SummerCMS") || strings.Contains(rec.Body.String(), "theme: broken") { + t.Fatalf("%s: not the built-in page: %q", path, rec.Body.String()) + } + } + }) + } +} + +func TestErrorPageAdminUntouched(t *testing.T) { + build := func(t *testing.T, disable bool) http.Handler { t.Helper() - app := transportApp(t, "production", map[string]string{"http.yaml": transportBodyLimits + httpYAML}) - h, err := Assemble(app, []party.Plugin{routes}) + app := adminPrefixApp(t) + if disable { + if err := app.Config.Set("http.error_pages.enabled", false); err != nil { + t.Fatal(err) + } + } + h, err := Assemble(app, []party.Plugin{adminPrefixPlugin{}}) if err != nil { t.Fatal(err) } return h } - serve := func(h http.Handler, path string, browser bool) *httptest.ResponseRecorder { - req := httptest.NewRequest(http.MethodGet, path, nil) - if browser { - req.Header.Set("Accept", chromeAccept) - req.Header.Set("Sec-Fetch-Mode", "navigate") - } else { - req.Header.Set("Accept", "*/*") - } - rec := httptest.NewRecorder() - h.ServeHTTP(rec, req) - return rec - } - on := build(t, "") - for _, tc := range []struct { - path string - status int - curl string - }{ - {"/posts/abc", 404, "404 page not found\n"}, - {"/boom", 500, `{"error":true,"message":"Internal server error"}`}, - {"/raw/boom", 500, ""}, + on, off := build(t, false), build(t, true) + for _, tc := range []struct{ method, path string }{ + {"GET", "/acme-admin/no-such-screen"}, + {"POST", "/acme-admin/no-such-screen"}, + {"GET", "/acme-admin/api/no-such-endpoint"}, } { - rec := serve(on, tc.path, true) - if rec.Code != tc.status || rec.Header().Get("Content-Type") != "text/html; charset=utf-8" { - t.Fatalf("browser %s: %d %q", tc.path, rec.Code, rec.Header().Get("Content-Type")) + a, b := serveReq(on, browserReq(tc.method, tc.path)), serveReq(off, browserReq(tc.method, tc.path)) + if a.Code != b.Code || a.Body.String() != b.Body.String() || a.Header().Get("Content-Type") != b.Header().Get("Content-Type") { + t.Fatalf("%s %s: on=%d %q %q off=%d %q %q", tc.method, tc.path, + a.Code, a.Header().Get("Content-Type"), a.Body.String(), b.Code, b.Header().Get("Content-Type"), b.Body.String()) } - if strings.Contains(rec.Body.String(), "secret panic detail") { - t.Fatalf("browser %s: panic value leaked", tc.path) - } - rec = serve(on, tc.path, false) - if rec.Code != tc.status || strings.TrimSpace(rec.Body.String()) != strings.TrimSpace(tc.curl) { - t.Fatalf("curl %s: %d %q", tc.path, rec.Code, rec.Body.String()) + if a.Header().Get("Content-Security-Policy") == errorPageCSP { + t.Fatalf("%s %s: the admin got the error page", tc.method, tc.path) } } - off := build(t, "error_pages:\n enabled: false\n") - rec := serve(off, "/nowhere", true) + // Outside the admin prefix the same router still serves the page. + checkErrorPage(t, serveReq(on, browserReq("GET", "/acme-adminx")), 404) +} + +func TestErrorPageUploads(t *testing.T) { + h := buildErrorPageRouter(t, "production", map[string]string{"storage.yaml": "uploads:\n serve: true\n"}, nil) + rec := serveReq(h, browserReq("GET", "/storage/uploads/a/b.png")) + checkErrorPage(t, rec, 404) + if rec.Header().Get("Content-Disposition") != "" { + t.Fatalf("Content-Disposition = %q", rec.Header().Get("Content-Disposition")) + } + + req := httptest.NewRequest("GET", "/storage/uploads/a/b.png", nil) + req.Header.Set("Accept", "image/avif,image/webp,*/*") + req.Header.Set("Sec-Fetch-Mode", "no-cors") + rec = serveReq(h, req) if rec.Code != 404 || rec.Body.String() != "404 page not found\n" { - t.Fatalf("disabled browser: %d %q", rec.Code, rec.Body.String()) + t.Fatalf("image request: %d %q", rec.Code, rec.Body.String()) + } + if got := rec.Header().Get("Content-Security-Policy"); got != uploadsContentSecurityPolicy { + t.Fatalf("image request CSP = %q", got) } }