M web/router.go => web/router.go +57 -15
@@ 6,21 6,40 @@ import (
"strings"
"github.com/go-chi/chi/v5"
+ chimiddleware "github.com/go-chi/chi/v5/middleware"
"sourcecraft.dev/bigbes/sr-ht-ecore/assets"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/chimw"
"sourcecraft.dev/bigbes/sr-ht-ecore/csrf"
"sourcecraft.dev/bigbes/sr-ht-ecore/middleware"
)
// Handler returns a router with everything this package needs already
-// installed: panic recovery, the private cache policy, the authn principal
-// middleware and the same-origin guard, then the routes. The daemon mounts it
-// at "/".
+// installed: the request line, panic recovery, the private cache policy, the
+// authn principal middleware and the same-origin guard, then the routes, then
+// the two refusals chi answers when routing fails. The daemon mounts it at "/".
//
-// The order is the one the shared packages ask for. RecoverPanics is outermost
-// so it covers the later middleware as well as the handlers, and it is
-// sr-ht-ecore's rather than chi's or our own for one behaviour: a panic that
-// arrives *after* the response has started aborts the connection instead of
+// The order is the one the shared packages ask for. RequestID and RealIP first,
+// because chimw.RequestLogger reads both and can only carry an id and the
+// viewer's address if they have already run. The logger is next and — the one
+// piece of ordering that is not cosmetic — *outside* RecoverPanics rather than
+// under it: it has to observe the status that actually went out, and for a
+// panicking handler that status is the 500 RecoverPanics renders. Installed the
+// other way round it would see an unwinding stack and nothing written, and would
+// log the request that produced an error page as if it had produced nothing.
+//
+// Nothing is left uncovered by putting it outside: this middleware builds a
+// record and calls slog, and a panic in slog is not a failure any error page was
+// going to survive either.
+//
+// Until now this surface logged no request lines at all. core-go installs chi's
+// own Logger, but only under -d and only on the groups it registers itself,
+// which the router mounted at "/" is not one of — so a 500 here left a stack in
+// the journal with no line saying which URL produced it.
+//
+// RecoverPanics then covers every later middleware as well as the handlers, and
+// it is sr-ht-ecore's rather than chi's or our own for one behaviour: a panic
+// that arrives *after* the response has started aborts the connection instead of
// appending an error page to a truncated one. PrivateCache sits inside it so
// that every answer — including the two refusals below — carries the same
// private, no-store a page rendered behind a login cookie needs.
@@ 40,6 59,9 @@ import (
// this guard: Register installs no middleware of its own.
func (s *Server) Handler() http.Handler {
r := chi.NewRouter()
+ r.Use(chimiddleware.RequestID)
+ r.Use(chimiddleware.RealIP)
+ r.Use(chimw.RequestLogger(chimw.SlogFormatter{Skip: chimw.SkipPaths("/healthz")}))
r.Use(middleware.RecoverPanics(func(w http.ResponseWriter, r *http.Request, _ any) {
s.renderError(w, r, http.StatusInternalServerError, "")
}))
@@ 49,6 71,15 @@ func (s *Server) Handler() http.Handler {
s.renderError(w, r, http.StatusForbidden, csrf.Message)
}))
s.Register(r)
+
+ // The two refusals chi answers when routing fails, pointed at this service's
+ // error page. Without them a mistyped URL — the one refusal a viewer is most
+ // likely to meet — came back as net/http's plain text, with no nav to get out
+ // of and nothing to say which service it came from, while every refusal a
+ // handler produced was a rendered page. It is registered here rather than in
+ // Register because it is not a route: a caller that owns its own router gets
+ // its own answer for an address it does not serve.
+ chimw.RenderRefusals(r, s.renderError)
return r
}
@@ 64,32 95,43 @@ func (s *Server) Handler() http.Handler {
// document called "notes/2026.json.md" is still reachable and a request for
// "notes/2026.json" still means "the JSON of notes/2026".
func (s *Server) Register(r chi.Router) {
- r.Get("/", s.handleIndex)
- r.Get("/healthz", s.handleHealthz)
+ // Every read route is registered for GET and HEAD both, through
+ // chimw.GetHead. chi registers GET alone and net/http synthesizes nothing, so
+ // each of these answered `curl -I` — and every uptime probe, and every cache
+ // revalidating what it holds — with a 405 naming GET as the only method it
+ // takes, plus a page of rendered chrome from a path whose whole job is to be
+ // cheap. The pair shares one handler, so a HEAD produces exactly the status
+ // its GET would: there is no second path that could answer 200 where the GET
+ // answers 404, which on these pages would be a visibility leak.
+ //
+ // The mutating routes are deliberately not in it. A HEAD that writes is not a
+ // HEAD, so each POST stays a POST and a HEAD on that path resolves to the page.
+ chimw.GetHead(r, "/", s.handleIndex)
+ chimw.GetHead(r, "/healthz", s.handleHealthz)
r.Mount(assets.DefaultPrefix, s.static)
- r.Get("/search", s.handleSearch)
- r.Get("/inbox", s.handleInbox)
+ chimw.GetHead(r, "/search", s.handleSearch)
+ chimw.GetHead(r, "/inbox", s.handleInbox)
r.Post("/inbox/seen", s.handleInboxSeen)
// /tokens redirects to tokens.sr.ht, which issues every agent credential on
// the instance. The POST routes that minted and revoked here went with the
// table behind them; the GET stays so that a bookmark, the dashboard button
// and every doc that ever said "see /tokens" still land somewhere useful.
- r.Get("/tokens", s.handleTokens)
+ chimw.GetHead(r, "/tokens", s.handleTokens)
// The proposal routes are registered before the document wildcard. chi gives
// the static "p" segment priority over the "*" catch-all regardless, but
// keeping them adjacent makes the "/p/ is the proposal namespace" decision
// visible in one place.
- r.Get("/~{owner}/{space}/p/{id}", s.handleProposal)
+ chimw.GetHead(r, "/~{owner}/{space}/p/{id}", s.handleProposal)
r.Post("/~{owner}/{space}/p/{id}/approve", s.handleProposalApprove)
r.Post("/~{owner}/{space}/p/{id}/reject", s.handleProposalReject)
r.Post("/~{owner}/{space}/p/{id}/comment", s.handleProposalComment)
r.Post("/~{owner}/{space}/p/{id}/reply", s.handleProposalReply)
r.Post("/~{owner}/{space}/p/{id}/resolve", s.handleProposalResolve)
- r.Get("/~{owner}/{space}", s.handleSpace)
- r.Get("/~{owner}/{space}/*", s.handleDocument)
+ chimw.GetHead(r, "/~{owner}/{space}", s.handleSpace)
+ chimw.GetHead(r, "/~{owner}/{space}/*", s.handleDocument)
}
// handleHealthz is a dependency-free liveness probe.
M web/server.go => web/server.go +11 -6
@@ 14,20 14,25 @@
// that code — inherited from compare.sr.ht, which had inherited it from
// somewhere else — is what ecore exists to have deleted.
//
-// Four more of ecore's packages carry what used to be local copies of the same
-// idea, and the pattern is the same every time — the rule lives in one place
-// and this package supplies only what is genuinely spec.sr.ht's:
+// Five more of ecore's packages carry what used to be local copies of the same
+// idea, or what no copy here ever had, and the pattern is the same every time —
+// the rule lives in one place and this package supplies only what is genuinely
+// spec.sr.ht's:
//
// - pages discovers the page templates, refuses at startup a page that
// defines no "content", renders into a buffer before touching the response
// and ships the shared error body. What stays here is renderError, which
// wraps that body in this service's view struct, and fail, which maps this
-// service's own sentinels onto statuses.
-// - assets finds the hashed stylesheet and serves the static tree with the
-// cache policy each name implies.
+// service's own sentinels onto statuses. FormValues is its other half: a
+// bounded read that answers r.PostForm and never r.Form.
+// - assets finds the hashed stylesheet and the favicon and serves the static
+// tree with the cache policy each name implies.
// - csrf is the same-origin guard, installed on the router rather than called
// by three handlers — see [Server.Handler].
// - middleware is the private-cache policy and the panic guard.
+// - chimw is the chi-shaped half: the request line as a slog record, the read
+// routes registered for GET and HEAD both, and the two refusals chi answers
+// when routing fails, which this surface previously did not answer at all.
//
// pages.Render answers the response itself and returns an error only for the
// log. It must never be handed to fail: that would either write a second
M web/web_test.go => web/web_test.go +43 -0
@@ 707,6 707,49 @@ func TestMissingSpaceIs404(t *testing.T) {
}
}
+// The router's own two refusals are this service's error page and not net/http's
+// plain text. They are the ones a viewer is most likely to meet — a mistyped URL
+// and a stale bookmark — and until chimw.RenderRefusals was installed they were
+// the only refusals on this surface with no nav to get out of.
+func TestRoutingFailuresRenderTheChromePage(t *testing.T) {
+ h, _ := testServer(t)
+
+ rec := get(t, h, "/no/such/route", "bigbes")
+ assert.Equal(t, http.StatusNotFound, rec.Code)
+ assert.Contains(t, rec.Body.String(), "navbar-brand", "the 404 carries the chrome")
+
+ // A method the router does not allow on a path it does serve: /inbox/seen is
+ // a POST, so a GET of it is chi's 405 rather than a miss.
+ rec = get(t, h, "/inbox/seen", "bigbes")
+ assert.Equal(t, http.StatusMethodNotAllowed, rec.Code)
+ assert.Contains(t, rec.Body.String(), "navbar-brand", "the 405 carries the chrome")
+}
+
+// Every read route answers HEAD, because RFC 9110 §9.3.2 makes it mandatory for
+// anything serving GET and because a monitor and a revalidating cache both reach
+// for it. The pair shares a handler, so the status is the GET's — including the
+// 404 of a document that is not there, which must not become a 200 on a method
+// that returns no body to contradict it.
+func TestReadRoutesAnswerHead(t *testing.T) {
+ h, _ := testServer(t)
+ for target, want := range map[string]int{
+ "/": http.StatusOK,
+ "/healthz": http.StatusOK,
+ "/inbox": http.StatusOK,
+ "/~bigbes/rfcs": http.StatusOK,
+ "/~bigbes/rfcs/specs/0007-storage": http.StatusOK,
+ "/~bigbes/rfcs/specs/nope": http.StatusNotFound,
+ } {
+ t.Run(target, func(t *testing.T) {
+ req := httptest.NewRequest(http.MethodHead, target, nil)
+ login(req, "bigbes")
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ assert.Equal(t, want, rec.Code, "HEAD %s", target)
+ })
+ }
+}
+
func TestTrailingSlashRedirectsToTheSpace(t *testing.T) {
h, _ := testServer(t)
rec := get(t, h, "/~bigbes/rfcs/", "bigbes")