mirror of
https://github.com/hanzoai/static.git
synced 2026-10-30 18:22:04 +00:00
zooqueenandHanzo Dev
070bf46353
Configuring ErrorPage404 set notFoundResponseCode to StatusOK, so a site that
ships a 404 page served it with 200. That is a soft 404: the body says the page is
gone while the status says it is fine. A crawler indexes every wrong URL as a real
page duplicating the error content, and a client cannot tell a typo from a hit.
d8ee68e found this and deliberately did NOT change it, for two good reasons: the
200 was pinned by TestCustomErrorPage, so it was encoded behaviour rather than an
oversight; and this type is ALSO the Yaegi plugin inside hanzoai/ingress, so
flipping it moves error semantics for every static-served site at once. It asked
for a deliberate decision rather than a drive-by. This is that decision, and the
missing half was the radius.
MEASURED: nothing in the estate sets it. Searching hanzoai/universe and
hanzoai/ingress for errorPage404 / --404 returns only the CRD schema declaring the
field and a docs example (`errorPage404: foobar`) — no site configures one. A site
that does not set it already answers 404 through http.NotFound, so it is untouched.
cloud.hanzo.ai does not set it either: its image runs --spa, which is a different
branch and is deliberately left alone. So this moves nothing in production today —
it fixes the answer for whoever sets it next, which cloud.hanzo.ai should, since it
already ships a real 404.html (33KB, distinct from the 198KB homepage) that nothing
can currently reach.
SPAMode remains the one case that legitimately answers 200 to an unknown path,
because there a client-side router really will resolve it. Conflating the two is
what made a deliberate error page indistinguishable from a shell, so the field that
existed only to express that conflation is gone.
Also fixes the header ordering it hid. Both branches called w.WriteHeader BEFORE
serveFile, but WriteHeader sends the header map — so the Content-Type serveFile set
afterwards was discarded (the browser had to sniff) and net/http logged a
superfluous WriteHeader call on every miss. serveErrorPage sets headers, then
status, then body, and deliberately does not reuse serveFile: that path ends in
http.ServeContent, which negotiates Range and If-Modified-Since and writes its own
200 — none of which an error body wants. It sends Cache-Control: no-store, because
a cached 404 outlives the mistake that caused it.
TestCustomErrorPage is updated with the reasoning written down, and TestSPAMode is
left asserting 200 right beside it so the two cannot be conflated again. Five new
cases cover the status, the surviving Content-Type, no-store, a miss with no page
configured, a configured-but-missing page (404, not 500), and that real pages are
unaffected. Verified they FAIL against the old StatusOK, not merely that they pass.
Co-authored-by: Hanzo Dev <[email protected]>
134 lines
4.6 KiB
Go
134 lines
4.6 KiB
Go
package static
|
|
|
|
import (
|
|
"context"
|
|
"io"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
// A custom 404 page must be served WITH 404. Serving it with 200 is a soft 404:
|
|
// the body says the page is gone while the status says it is fine, so a crawler
|
|
// indexes every wrong URL as a real page duplicating the error content, and a
|
|
// client cannot tell a typo from a hit. Configuring ErrorPage404 used to force
|
|
// StatusOK, which is what made this worth pinning rather than assuming.
|
|
func TestErrorPageKeepsItsStatus(t *testing.T) {
|
|
root := t.TempDir()
|
|
write(t, root, "index.html", "<h1>home</h1>")
|
|
write(t, root, "404.html", "<h1>not here</h1>")
|
|
|
|
h := handler(t, &Config{Root: root, IndexFiles: []string{"index.html"}, ErrorPage404: "404.html"})
|
|
|
|
rec := httptest.NewRecorder()
|
|
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/nope-xyz", nil))
|
|
|
|
if rec.Code != http.StatusNotFound {
|
|
t.Errorf("status = %d, want 404 — a 404 body served with %d is a soft 404", rec.Code, rec.Code)
|
|
}
|
|
if body := rec.Body.String(); !strings.Contains(body, "not here") {
|
|
t.Errorf("body = %q, want the configured 404 page", body)
|
|
}
|
|
// The header must survive: the previous code called WriteHeader BEFORE the file
|
|
// helper set Content-Type, so the type was dropped and the browser had to sniff.
|
|
if ct := rec.Header().Get("Content-Type"); !strings.HasPrefix(ct, "text/html") {
|
|
t.Errorf("Content-Type = %q, want text/html — headers must be set before the status", ct)
|
|
}
|
|
if cc := rec.Header().Get("Cache-Control"); cc != "no-store" {
|
|
t.Errorf("Cache-Control = %q, want no-store — a cached 404 outlives the mistake", cc)
|
|
}
|
|
}
|
|
|
|
// The one case that legitimately answers 200 to an unknown path: a client-side
|
|
// router really will resolve it. This is what ErrorPage404 must NOT be conflated
|
|
// with, so the two are pinned against each other.
|
|
func TestSPAModeStillAnswers200(t *testing.T) {
|
|
root := t.TempDir()
|
|
write(t, root, "index.html", "<h1>app shell</h1>")
|
|
|
|
h := handler(t, &Config{Root: root, IndexFiles: []string{"index.html"}, SPAMode: true, SPAIndex: "index.html"})
|
|
|
|
rec := httptest.NewRecorder()
|
|
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/deep/client/route", nil))
|
|
|
|
if rec.Code != http.StatusOK {
|
|
t.Errorf("SPA status = %d, want 200 — the client router resolves this path", rec.Code)
|
|
}
|
|
if !strings.Contains(rec.Body.String(), "app shell") {
|
|
t.Error("SPA mode must serve the shell for an unknown path")
|
|
}
|
|
}
|
|
|
|
// With no error page configured at all, a miss is still a plain 404.
|
|
func TestMissWithoutErrorPageIs404(t *testing.T) {
|
|
root := t.TempDir()
|
|
write(t, root, "index.html", "<h1>home</h1>")
|
|
|
|
h := handler(t, &Config{Root: root, IndexFiles: []string{"index.html"}})
|
|
|
|
rec := httptest.NewRecorder()
|
|
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/nope-xyz", nil))
|
|
if rec.Code != http.StatusNotFound {
|
|
t.Errorf("status = %d, want 404", rec.Code)
|
|
}
|
|
}
|
|
|
|
// A misconfigured page (named but absent) must still answer 404, not 500 — the
|
|
// request really was for something that does not exist, and our own config
|
|
// mistake is not the caller's problem.
|
|
func TestMissingErrorPageStillAnswers404(t *testing.T) {
|
|
root := t.TempDir()
|
|
write(t, root, "index.html", "<h1>home</h1>")
|
|
|
|
h := handler(t, &Config{Root: root, IndexFiles: []string{"index.html"}, ErrorPage404: "does-not-exist.html"})
|
|
|
|
rec := httptest.NewRecorder()
|
|
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/nope-xyz", nil))
|
|
if rec.Code != http.StatusNotFound {
|
|
t.Errorf("status = %d, want 404 even when the configured page is missing", rec.Code)
|
|
}
|
|
}
|
|
|
|
// A real page must be unaffected — the fix must not turn hits into misses.
|
|
func TestExistingPageUnaffected(t *testing.T) {
|
|
root := t.TempDir()
|
|
write(t, root, "index.html", "<h1>home</h1>")
|
|
write(t, root, "404.html", "<h1>not here</h1>")
|
|
write(t, root, "pricing.html", "<h1>pricing</h1>")
|
|
|
|
h := handler(t, &Config{Root: root, IndexFiles: []string{"index.html"}, ErrorPage404: "404.html"})
|
|
|
|
for _, p := range []string{"/", "/pricing", "/pricing.html"} {
|
|
rec := httptest.NewRecorder()
|
|
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, p, nil))
|
|
if rec.Code != http.StatusOK {
|
|
t.Errorf("%s = %d, want 200", p, rec.Code)
|
|
}
|
|
}
|
|
}
|
|
|
|
func handler(t *testing.T, c *Config) http.Handler {
|
|
t.Helper()
|
|
h, err := New(context.Background(), http.NotFoundHandler(), c, "test")
|
|
if err != nil {
|
|
t.Fatalf("New: %v", err)
|
|
}
|
|
return h
|
|
}
|
|
|
|
func write(t *testing.T, root, name, body string) {
|
|
t.Helper()
|
|
p := filepath.Join(root, name)
|
|
if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.WriteFile(p, []byte(body), 0o644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
}
|
|
|
|
var _ = io.Discard
|