diff --git a/web/handlers.go b/web/handlers.go index 6058177ab327bd5ec7fe5a4a611943dcf33763c6..935aee8261e6d52515c855aac571c2788da209e5 100644 --- a/web/handlers.go +++ b/web/handlers.go @@ -100,14 +100,26 @@ } } // fail renders the chrome error page for err, logging 5xx causes. +// +// Only a 400 carries the error's own text, and it is the one class that should: +// "invalid git ref" tells the viewer what to change about what they typed. +// Every other status takes the instance's standard sentence — a 500 because an +// error from below names paths and queries, a 404 because the standard sentence +// is exactly the one a repository that never existed produces, which is what +// keeps a repository the viewer may not see indistinguishable from an absent +// one. func (s *Server) fail(w http.ResponseWriter, r *http.Request, err error) { status := httpStatusFor(err) if status >= 500 { logrus.WithError(err).WithField("path", r.URL.Path).Error("web: request failed") - s.renderError(w, r, status, "an internal error occurred") + s.renderError(w, r, status, "") return } - s.renderError(w, r, status, err.Error()) + if status == http.StatusBadRequest { + s.renderError(w, r, status, err.Error()) + return + } + s.renderError(w, r, status, "") } // resolve authorizes and opens a repository, returning the git handle and the diff --git a/web/router.go b/web/router.go index 454d6b62c15f2eaca601a372e7a898ffdde9781b..094e12e0ea2cc4fdc606ae351f240be6d588ebea 100644 --- a/web/router.go +++ b/web/router.go @@ -2,47 +2,63 @@ package web import ( "net/http" - "path" "strings" "github.com/go-chi/chi/v5" + "sourcecraft.dev/bigbes/sr-ht-ecore/assets" + "sourcecraft.dev/bigbes/sr-ht-ecore/csrf" + "sourcecraft.dev/bigbes/sr-ht-ecore/middleware" ) -// Register mounts every compare.sr.ht route onto r. The caller is responsible -// for installing the middleware documented on the package (config + authz at a -// minimum); Register adds no middleware of its own. +// Register mounts every compare.sr.ht route onto r, inside a group carrying the +// three middlewares that need this Server. The chain documented on the package +// (config + authz at a minimum) is still the caller's, and stays outside this +// group. +// +// - PrivateCache, because every page here is a page to its owner and a 404 to +// everybody else, at a URL that says nothing about the viewer. The static +// route opts back out per asset, from inside assets.Handler, and only once +// the bytes are committed. +// - RecoverPanics, so a panicking handler produces this service's own error +// page instead of a dropped connection — and, when the response has already +// started, a dropped connection instead of an error page appended to half a +// rendered diff. +// - csrf.Require, which guards a future rather than a present: every route +// below is a GET, so nothing is refused by it today. It is installed anyway +// because the day somebody adds the first POST is exactly the day nobody +// remembers to add the check, and these services have no CSRF token to fall +// back on — the session cookie is meta.sr.ht's, set on the parent domain, +// with a SameSite no individual service can choose. +// +// The router's own 404 is set here too, so a mistyped URL lands on a page with a +// nav rather than on chi's plain-text dead end. func (s *Server) Register(r chi.Router) { - r.Get("/", s.handleIndex) - r.Get("/jump", s.handleJump) - r.Get("/healthz", s.handleHealthz) - r.Get("/static/*", s.handleStatic) + r.Group(func(r chi.Router) { + r.Use(middleware.PrivateCache) + r.Use(middleware.RecoverPanics(func(w http.ResponseWriter, r *http.Request, _ any) { + s.renderError(w, r, http.StatusInternalServerError, "") + })) + r.Use(csrf.Require(s.chromeSvc.SelfOrigin(), nil)) - r.Get("/~{owner}/{repo}", s.handleRepo) - // A single wildcard route serves both the form target (empty wildcard ⇒ - // redirect to the canonical URL) and the compare view itself. - r.Get("/~{owner}/{repo}/compare/*", s.handleCompare) - r.Get("/~{owner}/{repo}/commit/{rev}", s.handleCommit) + r.NotFound(s.handleNotFound) + + r.Get("/", s.handleIndex) + r.Get("/jump", s.handleJump) + r.Get("/healthz", s.handleHealthz) + r.Handle(assets.DefaultPrefix+"*", s.static) + + r.Get("/~{owner}/{repo}", s.handleRepo) + // A single wildcard route serves both the form target (empty wildcard ⇒ + // redirect to the canonical URL) and the compare view itself. + r.Get("/~{owner}/{repo}/compare/*", s.handleCompare) + r.Get("/~{owner}/{repo}/commit/{rev}", s.handleCommit) + }) } // handleHealthz is a dependency-free liveness probe. func (s *Server) handleHealthz(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "text/plain; charset=utf-8") _, _ = w.Write([]byte("ok\n")) -} - -// handleStatic serves the embedded assets, tagging the content-addressed -// stylesheet as immutable and forcing a JS content type for the bundle. -func (s *Server) handleStatic(w http.ResponseWriter, r *http.Request) { - name := path.Base(r.URL.Path) - if strings.HasSuffix(name, ".js") { - w.Header().Set("Content-Type", "application/javascript; charset=utf-8") - } - if hashedCSSRe.MatchString(name) || hashedBundleRe.MatchString(name) { - w.Header().Set("Cache-Control", "public, max-age=31536000, immutable") - } else { - w.Header().Set("Cache-Control", "public, max-age=3600") - } - s.staticFileServer.ServeHTTP(w, r) } // handleJump powers the owner/repo jump form: it redirects to the canonical repo diff --git a/web/server.go b/web/server.go index ce522c28276e7e8fcde5e25430cc100a7bf69142..df5b29efc38ca451ad44cb9730dd4d7bfab0b85a 100644 --- a/web/server.go +++ b/web/server.go @@ -19,16 +19,20 @@ // viewData so the fields promote into the templates. Nothing here rebuilds the // switcher or re-derives a login URL: the copy that used to live in web/chrome.go // is exactly what ecore exists to have deleted. // -// Two things about the chrome remain this service's own, because they are about -// what compare renders and not about the instance: the vendored bundle's href -// (BundleHref below), and the full-bleed ContainerClass the two diff views set — -// a side-by-side diff in a centered "container" is a column of code half the -// window wide. +// One thing about the chrome remains this service's own, because it is about +// what compare renders and not about the instance: the full-bleed +// ContainerClass the two diff views set — a side-by-side diff in a centered +// "container" is a column of code half the window wide. The vendored bundle's +// href is no longer among them: it is a hashed build artefact like the +// stylesheet, so it lives in chrome.Service.Assets, which is the slot ecore +// grew once three services had each added their own field for it. // // # What the cmd layer must wire // -// Register only installs routes; it assumes the following middleware is already -// applied to the router it is handed, in this order (outermost first): +// Register installs the middleware that needs this Server — the private cache +// policy, panic recovery through this package's error page, and the same-origin +// guard — and assumes the following is already applied to the router it is +// handed, in this order (outermost first): // // chi middleware.RealIP // chi middleware.Recoverer @@ -46,11 +50,12 @@ import ( "fmt" "io/fs" "net/http" - "path" - "regexp" + "github.com/sirupsen/logrus" "github.com/vaughan0/go-ini" + "sourcecraft.dev/bigbes/sr-ht-ecore/assets" "sourcecraft.dev/bigbes/sr-ht-ecore/chrome" + "sourcecraft.dev/bigbes/sr-ht-ecore/pages" "sourcecraft.dev/bigbes/sr-ht-compare/authz" ) @@ -62,15 +67,12 @@ // layer installs — a service that spelled it two ways would appear in the // instance's navigation and fail to recognise itself in it. const configSection = "compare.sr.ht" -// hashedCSSRe matches the content-addressed stylesheet name so it can be served -// with an immutable cache lifetime (the hash changes whenever the bytes do). -var hashedCSSRe = regexp.MustCompile(`^main\.min\.[0-9a-f]{6,}\.css$`) - -// hashedBundleRe matches the content-addressed frontend bundle. Like the -// stylesheet it carries a content hash in its name so a deploy busts the browser -// cache (a stale bundle.js is why the file tree can render blank after an -// upgrade); a matching name is served immutable. -var hashedBundleRe = regexp.MustCompile(`^bundle\.[0-9a-f]{6,}\.js$`) +// bundleAsset is the key compare's layout reads its front-end bundle's href +// under, in chrome.Service.Assets. It is spelled once here and once in the +// "scripts" block of the two diff pages; a third spelling would render no +// script tag at all rather than fail, which is why the two that exist are a +// constant and a template guarded on emptiness. +const bundleAsset = "bundle.js" // Server holds the immutable configuration a request handler needs. It is built // once at startup and is safe for concurrent use. @@ -84,18 +86,31 @@ // config.ini and asked for a per-request chrome.Page in view (chrome.go's // job until this service stopped carrying its own copy). chromeSvc *chrome.Service - bundleHref string + // pages is one parsed template set per page, discovered from the embedded + // tree by ecore rather than listed here. A page that defines no "content" + // never gets this far: pages.Load refuses it at startup, where the + // alternative was the chrome around a hole served with a 200. + pages pages.Set - staticFileServer http.Handler + // static serves the embedded asset tree with the cache policy the hashed + // name implies, and answers everything that is not a file — a directory + // above all — through this service's own 404 page. + static http.Handler } // New assembles a Server from the shared SourceHut config. It reads // [git.sr.ht] repos, [meta.sr.ht] origin and [compare.sr.ht] origin (all // required), hands the whole file to chrome.NewService — the switcher is a // question about every [*.sr.ht] section the instance defines, not about our own -// keys — and resolves the hashed stylesheet and bundle names by globbing the -// embedded static FS. A missing required key is a clear error, not a panic, so -// the cmd layer can fail startup loudly. +// keys — resolves the hashed stylesheet and bundle through ecore's assets, and +// parses the page templates through ecore's pages. A missing required key is a +// clear error, not a panic, so the cmd layer can fail startup loudly. +// +// A missing build artefact is not one of those errors. assets.Resolve answers +// "" for a stylesheet or a bundle this binary was built without, because a +// checkout that has not run `make css` must still be runnable; the layout +// guards both hrefs on emptiness so a bare page is what such a build serves, +// rather than a that re-requests the page it is on. func New(conf ini.File, authorizer authz.Authorizer) (*Server, error) { reposRoot, ok := conf.Get("git.sr.ht", "repos") if !ok || reposRoot == "" { @@ -113,15 +128,24 @@ if chromeSvc.SelfOrigin() == "" { return nil, fmt.Errorf("web: [%s] origin is required", configSection) } - cssHref, err := resolveCSSHref() + cssHref, err := assets.Resolve(staticFS, cssGlob, assets.DefaultPrefix) + if err != nil { + return nil, fmt.Errorf("web: resolve the stylesheet: %w", err) + } + bundleHref, err := assets.Resolve(staticFS, bundleGlob, assets.DefaultPrefix) if err != nil { - return nil, err + return nil, fmt.Errorf("web: resolve the bundle: %w", err) + } + if cssHref == "" || bundleHref == "" { + logrus.WithFields(logrus.Fields{"css": cssHref, "bundle": bundleHref}). + Warn("web: built without a front-end artefact; run `make` before `go build`") } chromeSvc.StyleHref = cssHref + chromeSvc.Assets = map[string]string{bundleAsset: bundleHref} - bundleHref, err := resolveBundleHref() + set, err := pages.Load(tmplFS, pages.Options{Funcs: funcMap}) if err != nil { - return nil, err + return nil, fmt.Errorf("web: load the page templates: %w", err) } staticSub, err := fs.Sub(staticFS, "static") @@ -129,13 +153,18 @@ if err != nil { return nil, fmt.Errorf("web: sub static FS: %w", err) } - return &Server{ - authorizer: authorizer, - reposRoot: reposRoot, - chromeSvc: chromeSvc, - bundleHref: bundleHref, - staticFileServer: http.StripPrefix("/static/", http.FileServer(http.FS(staticSub))), - }, nil + s := &Server{ + authorizer: authorizer, + reposRoot: reposRoot, + chromeSvc: chromeSvc, + pages: set, + } + // Built after the Server exists because the not-found arm is this service's + // own error page: an asset URL typed by hand lands on a page with a nav to + // get out of, and a directory — /static/, which the file server alone would + // answer with a listing of every artefact in the binary — lands there too. + s.static = assets.Handler(staticSub, assets.DefaultPrefix, http.HandlerFunc(s.handleNotFound)) + return s, nil } // viewData is the root value every template is executed against. @@ -148,11 +177,6 @@ // {{.Data.Something}}, which is what keeps a page from shadowing a chrome field. type viewData struct { chrome.Page - // BundleHref is the hashed front-end bundle's URL. It is chrome — every page - // may load it — but it is this service's alone: no other service on the - // instance ships a diff renderer, so it stays here and not in ecore's Page. - BundleHref string - // Data is the page's own payload. Data any } @@ -163,34 +187,5 @@ // The username is whatever the authz cookie middleware resolved, which is "" for // a viewer whose cookie is missing, expired or unreadable — so the nav offers // login to exactly the viewers the handlers treat as anonymous. func (s *Server) view(r *http.Request, title string) viewData { - return viewData{ - Page: s.chromeSvc.Page(r, title, authz.ForContext(r.Context())), - BundleHref: s.bundleHref, - } -} - -// resolveCSSHref globs the embedded static FS for the content-addressed -// stylesheet and returns its site-absolute URL. -func resolveCSSHref() (string, error) { - matches, err := fs.Glob(staticFS, "static/main.min.*.css") - if err != nil { - return "", fmt.Errorf("web: glob stylesheet: %w", err) - } - if len(matches) == 0 { - return "", fmt.Errorf("web: no main.min.*.css in embedded static assets") - } - return "/static/" + path.Base(matches[0]), nil -} - -// resolveBundleHref globs the embedded static FS for the content-addressed -// frontend bundle and returns its site-absolute URL. -func resolveBundleHref() (string, error) { - matches, err := fs.Glob(staticFS, "static/bundle.*.js") - if err != nil { - return "", fmt.Errorf("web: glob bundle: %w", err) - } - if len(matches) == 0 { - return "", fmt.Errorf("web: no bundle.*.js in embedded static assets") - } - return "/static/" + path.Base(matches[0]), nil + return viewData{Page: s.chromeSvc.Page(r, title, authz.ForContext(r.Context()))} } diff --git a/web/templates.go b/web/templates.go index 086567fde4a22cce77660535dca33d3455421b88..2586c5528228990b475982c0b9056468dd6419fc 100644 --- a/web/templates.go +++ b/web/templates.go @@ -1,19 +1,19 @@ package web import ( - "bytes" "embed" "html/template" "net/http" - "time" "github.com/sirupsen/logrus" "sourcecraft.dev/bigbes/sr-ht-ecore/chrome" + "sourcecraft.dev/bigbes/sr-ht-ecore/pages" ) -// tmplFS holds the page templates. Each page is parsed together with the shared -// layout into its own template set so that per-page "content"/"scripts" defines -// do not collide across pages. +// tmplFS holds the page templates. pages.Load discovers the pages in it and +// parses each one into its own set together with the shared layout, so that +// per-page "content"/"scripts" defines do not collide across pages — and so +// that adding templates/whatever.html is the whole registration of a page. // //go:embed templates/*.html var tmplFS embed.FS @@ -24,6 +24,14 @@ // //go:embed static var staticFS embed.FS +// The globs that find this build's content-addressed artefacts in staticFS. +// `make css` and the bundle build each write exactly one file, removing the +// previous build's first, so a glob here has at most one match to pick. +const ( + cssGlob = "static/main.min.*.css" + bundleGlob = "static/bundle.*.js" +) + // funcMap holds the template helpers shared by every page. // // It starts from chrome.Funcs — so the shared partials find the helpers they @@ -31,14 +39,13 @@ // were written against, and so "shortsha" is the instance's one abbreviation // rule rather than this service's copy of it — and adds compare's own on top. // Adding after is deliberate: a name may then be shadowed on purpose rather than // by accident of map ordering. +// The commit timestamps this service used to format with a "date" helper of its +// own now go through chrome's "reltime" and "abstime": a listing says "3 days +// ago" and hovers to the exact UTC stamp, which is the one spelling the whole +// instance shows. var funcMap = func() template.FuncMap { m := chrome.Funcs() - // date formats a commit timestamp for display. - m["date"] = func(t time.Time) string { - return t.UTC().Format("2006-01-02 15:04 MST") - } - // statusClass maps a git file-change status letter to a CSS modifier used by // the .diff-status badge (see the diff-status rules in layout.html). Anything // unrecognized falls back to the neutral "o". @@ -82,59 +89,36 @@ return m }() -// pageNames are the content templates; each is parsed with layout.html. -var pageNames = []string{"index", "repo", "compare", "commit", "error"} - -// pages maps a page name to its parsed template set: the shared chrome partials -// of sr-ht-ecore, the layout, and that one page. The partials are attached to -// every set rather than to a shared one, for the same reason the layout is — -// each page defines its own "content", and one set would let the last parsed win. -var pages = func() map[string]*template.Template { - m := make(map[string]*template.Template, len(pageNames)) - for _, name := range pageNames { - t := chrome.MustAttach(template.New("layout.html").Funcs(funcMap)) - t = template.Must(t.ParseFS(tmplFS, "templates/layout.html", "templates/"+name+".html")) - m[name] = t - } - return m -}() - -// render executes a page into a buffer first, so a template error yields a clean -// 500 rather than a half-written response. On success it writes the status and -// the buffered HTML. +// render writes one page, and logs whatever pages.Render gives back. +// +// The log line is the whole of what a caller may do with that error: Render has +// already answered the response — a 500 carrying a fixed string when the +// template failed — so handing it to fail would write a second response over a +// committed one, or, when the failure is in the error page itself, recurse +// through the page that just broke. That is why this returns nothing. func (s *Server) render(w http.ResponseWriter, status int, page string, vd viewData) { - t, ok := pages[page] - if !ok { - logrus.WithField("page", page).Error("web: unknown template page") - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } - var buf bytes.Buffer - if err := t.ExecuteTemplate(&buf, "layout.html", vd); err != nil { - logrus.WithError(err).WithField("page", page).Error("web: template execution failed") - http.Error(w, "internal server error", http.StatusInternalServerError) - return + if err := s.pages.Render(w, status, page, vd); err != nil { + logrus.WithError(err).WithField("page", page).Error("web: render") } - w.Header().Set("Content-Type", "text/html; charset=utf-8") - w.WriteHeader(status) - _, _ = buf.WriteTo(w) } -// errorData is the payload of the error page. -type errorData struct { - Status int - StatusText string - Message string +// renderError renders the chrome-wrapped error page for a status. +// +// An empty message takes the instance's standard sentence for that status, so +// the 404 a hidden repository produces reads exactly like the 404 of a +// repository that never existed — which is the point of answering 404 rather +// than 403 in the first place. A message is only ever this service's own words +// about what the viewer typed (a malformed compare spec), never an error from +// below. +func (s *Server) renderError(w http.ResponseWriter, r *http.Request, status int, message string) { + vd := s.view(r, http.StatusText(status)) + vd.Data = pages.Error(status, message) + s.render(w, status, pages.ErrorPage, vd) } -// renderError renders the chrome-wrapped error page. It never recurses into -// render on failure (render falls back to http.Error itself). -func (s *Server) renderError(w http.ResponseWriter, r *http.Request, status int, message string) { - vd := s.view(r, http.StatusText(status)) - vd.Data = errorData{ - Status: status, - StatusText: http.StatusText(status), - Message: message, - } - s.render(w, status, "error", vd) +// handleNotFound is the 404 for a route the router does not have and for an +// asset path that is not a file. It is a http.HandlerFunc so it can be handed +// to chi's NotFound and to assets.Handler, which both want one. +func (s *Server) handleNotFound(w http.ResponseWriter, r *http.Request) { + s.renderError(w, r, http.StatusNotFound, "") } diff --git a/web/templates/commit.html b/web/templates/commit.html index d95402977573a210b49c73f1aa185ed248a9a604..0c0b5185f7a804deb248fd7424636990f98f40ac 100644 --- a/web/templates/commit.html +++ b/web/templates/commit.html @@ -11,7 +11,7 @@

{{$c.Subject}}

{{if $c.Body}}
{{$c.Body}}
{{end}}

- {{$c.AuthorName}} <{{$c.AuthorEmail}}> — {{date $c.Date}} + {{$c.AuthorName}} <{{$c.AuthorEmail}}> — {{abstime $c.Date}}

Commit {{$c.SHA}} — @@ -77,6 +77,10 @@ {{end}} +{{/* The bundle's href comes from the chrome's Assets, and is guarded on + emptiness for the reason StyleHref is: a build without the artefact must + render a page that says so by being inert, not a +{{with index .Assets "bundle.js"}}{{end}} {{end}} diff --git a/web/templates/compare.html b/web/templates/compare.html index 4a27aab56a3619915a5e579840730c83a72f5b6b..1a1d302463e7890ed0da063e8a337a86a7574664 100644 --- a/web/templates/compare.html +++ b/web/templates/compare.html @@ -35,7 +35,7 @@ {{range .Data.Commits}}

  • {{.ShortSHA}} {{.Subject}} - — {{.AuthorName}}, {{date .Date}} + — {{.AuthorName}}, {{reltime .Date}}
    • {{end}} @@ -79,6 +79,7 @@ {{end}} +{{/* See commit.html: the bundle is a chrome asset, guarded on emptiness. */}} {{define "scripts"}} - +{{with index .Assets "bundle.js"}}{{end}} {{end}} diff --git a/web/templates/layout.html b/web/templates/layout.html index 28ee73a13b7c3e093ed55046e6f064ee5dda796e..815cac6596cfa1ca845bdb451564336d20af4497 100644 --- a/web/templates/layout.html +++ b/web/templates/layout.html @@ -9,7 +9,9 @@ What is left here is the document, the diff-status colours below (compare's own, and nobody else's), and three seams: "head" and "scripts" are blocks, so a page that needs neither still renders; "content" is a {{template}} and must stay one — a block would give every page an empty default, which is the chrome - around a hole served 200 for the page that forgot to define it. + around a hole served 200 for the page that forgot to define it. ecore's + pages.Load refuses such a page at startup, and it can go on doing so only + while this line stays a {{template}}. */}} @@ -19,8 +21,10 @@ {{.Title}} {{/* Guarded rather than emitted empty: re-requests the page - it is on, which is a page load per page load. New refuses to start - without a stylesheet, so in a deployed binary this is always taken. */}} + it is on, which is a page load per page load. assets.Resolve answers "" + for a binary built without `make css`, which is a checkout run from + source; a deployed build always has the artefact and always takes + this. */}} {{if .StyleHref}}{{end}}