From 17fa0f16c9e02de640d6be5fce3a1a5237f0e462 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Sat, 8 Aug 2026 22:44:12 +0300 Subject: [PATCH] web: render through ecore's pages and its error page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The page list, the per-page parse loop and the view-template loop are gone: pages discovers every file in templates/, so a page is registered by existing, and one that defines no content block is refused at startup rather than served as chrome around a hole. 404.html and 403.html are gone with them — the shared error page carries the same body, and its prose is deliberately the same for a database that is not there and one the viewer may not see. The renderer that replaced them closes a leak: the old one wrote "template render error: "+err.Error() into the response body, handing the viewer template names and field paths. pages answers a fixed sentence and returns the error for the log. reltime and abstime come from chrome.Funcs now; ours called every future instant "just now", where the shared one says "in 3 weeks". --- web/handlers_browse.go | 8 +- web/handlers_keys.go | 2 +- web/handlers_repo.go | 8 +- web/handlers_settings.go | 2 +- web/handlers_view.go | 2 +- web/router.go | 48 +++-- web/templates.go | 177 +++++------------- web/templates/403.html | 6 - web/templates/404.html | 6 - .../{partials.html => _partials.html} | 0 web/templates/layout.html | 12 +- web/views.go | 19 +- web/views_test.go | 22 ++- web/web_test.go | 58 ++++++ 14 files changed, 176 insertions(+), 194 deletions(-) delete mode 100644 web/templates/403.html delete mode 100644 web/templates/404.html rename web/templates/{partials.html => _partials.html} (100%) diff --git a/web/handlers_browse.go b/web/handlers_browse.go index c9baf7496edb1ccaedc8b7e0d91f374a351d1e24..262dba33fa5da5732b4d10a9ca80bbcddc68d5fe 100644 --- a/web/handlers_browse.go +++ b/web/handlers_browse.go @@ -82,7 +82,7 @@ func (a *app) handleLog(w http.ResponseWriter, r *http.Request) { Commits: commits, NextHash: nextHash, } - a.render(w, http.StatusOK, "log.html", view) + a.render(w, http.StatusOK, "log", view) } // handleCommit renders a single commit's per-table diff summary. @@ -117,7 +117,7 @@ func (a *app) handleCommit(w http.ResponseWriter, r *http.Request) { Repo: repo, Summary: summary, } - a.render(w, http.StatusOK, "commit.html", view) + a.render(w, http.StatusOK, "commit", view) } // handleTree renders the tables (with schemas) present at a ref. @@ -156,7 +156,7 @@ func (a *app) handleTree(w http.ResponseWriter, r *http.Request) { Tables: tables, Views: applicableViews(a.views, tables), } - a.render(w, http.StatusOK, "tree.html", view) + a.render(w, http.StatusOK, "tree", view) } // handleTable renders a table's schema and a paginated page of its rows. Pages @@ -232,5 +232,5 @@ func (a *app) handleTable(w http.ResponseWriter, r *http.Request) { // is the table's, not ours, so the centered container turns a wide table into // a narrow strip with a scrollbar under it. view.ContainerClass = "container-fluid" - a.render(w, http.StatusOK, "table.html", view) + a.render(w, http.StatusOK, "table", view) } diff --git a/web/handlers_keys.go b/web/handlers_keys.go index 9095cc985603668944c73ba1f07c6f2cfb1f15df..6deb311b1b353e08ec1b7d237876fe02cab9bfea 100644 --- a/web/handlers_keys.go +++ b/web/handlers_keys.go @@ -31,7 +31,7 @@ func (a *app) renderKeys(w http.ResponseWriter, r *http.Request, ac *authContext Error: errMsg, Notice: notice, } - a.render(w, status, "keys.html", view) + a.render(w, status, "keys", view) } // handleKeys renders the dolt-key page: the user's registered keys and the diff --git a/web/handlers_repo.go b/web/handlers_repo.go index b2f991b43814918e40e77c70b70d2d63d0bac5f2..09355a98a07b68c6b017579111b9d08770df30b9 100644 --- a/web/handlers_repo.go +++ b/web/handlers_repo.go @@ -36,7 +36,7 @@ func (a *app) handleIndex(w http.ResponseWriter, r *http.Request) { } view.Repos = repoList(repos) } - a.render(w, http.StatusOK, "index.html", view) + a.render(w, http.StatusOK, "index", view) } // handleCreateForm renders the new-database form. Login is required. @@ -64,7 +64,7 @@ func (a *app) renderCreate(w http.ResponseWriter, r *http.Request, status int, f Form: form, Error: errMsg, } - a.render(w, status, "create.html", view) + a.render(w, status, "create", view) } // handleCreate processes the new-database form. It validates the name, creates @@ -164,7 +164,7 @@ func (a *app) handleUser(w http.ResponseWriter, r *http.Request) { Owner: owner, Repos: repoList(repos), } - a.render(w, http.StatusOK, "user.html", view) + a.render(w, http.StatusOK, "user", view) } // repoList adapts our databases to the shared listing partial @@ -245,7 +245,7 @@ func (a *app) handleOverview(w http.ResponseWriter, r *http.Request) { CloneURL: a.cloneURL(repo), BrowseError: browseErr, } - a.render(w, http.StatusOK, "overview.html", view) + a.render(w, http.StatusOK, "overview", view) } // cloneURL builds the HTTPS clone URL for repo: {self origin}/~owner/name. The diff --git a/web/handlers_settings.go b/web/handlers_settings.go index c3b3e08f435cd046aaec5ca89e768f2c2bba8321..21bd49021e9500b0ed0496d1d7d3c4decd520e00 100644 --- a/web/handlers_settings.go +++ b/web/handlers_settings.go @@ -68,7 +68,7 @@ func (a *app) renderSettings(w http.ResponseWriter, r *http.Request, status int, Error: errMsg, Notice: notice, } - a.render(w, status, "settings.html", view) + a.render(w, status, "settings", view) } // handleSettings renders the settings page (description/visibility, ACLs, danger diff --git a/web/handlers_view.go b/web/handlers_view.go index 600b17364fe680a2c12ef5ec7b74fdc6bc7cf575..cc033c74cb5e80961612d27afda7ce29c3bd07bd 100644 --- a/web/handlers_view.go +++ b/web/handlers_view.go @@ -93,5 +93,5 @@ func (a *app) handleView(w http.ResponseWriter, r *http.Request) { Views: applicableViews(a.views, tables), Data: data, } - a.render(w, http.StatusOK, view.Template(), envelope) + a.render(w, http.StatusOK, pageName(view.Template()), envelope) } diff --git a/web/router.go b/web/router.go index c548e4e047e27a64efeec2f02a79fc3e6fb28db4..9bb6ac13d447dfcc4c56c6f7c07655b72fdfcc4b 100644 --- a/web/router.go +++ b/web/router.go @@ -10,6 +10,7 @@ import ( "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-dolt/authn" "sourcecraft.dev/bigbes/sr-ht-dolt/core" @@ -33,8 +34,10 @@ const ( // app bundles the parsed templates, the shared chrome and the injected config. // Handlers are methods on *app so they share this state without a global. type app struct { - cfg Config - templates templateSet + cfg Config + // pages is sr-ht-ecore's page-template machinery: one parsed set per page in + // templates/, plus the shared error page. + pages pages.Set // chrome is sr-ht-ecore's shared page frame: the brand, the service // switcher, the login block and the environment banner, built once from the // instance config and asked for a per-request chrome.Page (see page below). @@ -91,7 +94,7 @@ func newApp(cfg Config) (*app, error) { return nil, fmt.Errorf("web: Register requires Conf (the chrome and the origins are built from it)") } - templates, err := loadTemplates() + set, err := loadPages() if err != nil { return nil, err } @@ -121,10 +124,10 @@ func newApp(cfg Config) (*app, error) { chromeSvc.StyleHref = styleHref return &app{ - cfg: cfg, - templates: templates, - chrome: chromeSvc, - static: static, + cfg: cfg, + pages: set, + chrome: chromeSvc, + static: static, // Snapshot the registry so all handlers see a stable set and tests can // override it per-app without mutating the global. views: append([]View{}, registeredViews...), @@ -185,22 +188,35 @@ func (emptyFS) Open(name string) (fs.File, error) { // --- shared response helpers ------------------------------------------------- +// errorView is the dot of the shared error page: the chrome, and the payload in +// the .Data field the page reads. +type errorView struct { + chrome.Page + Data pages.ErrorData +} + +// fail renders the shared error page. An empty message takes the standard +// sentence for the status, which is one of the reasons the page is shared: the +// visibility rules here require "somebody else's private database" and "no such +// database" to be indistinguishable, and two 404s whose prose differed would +// rebuild the distinction the status code was chosen to erase. +func (a *app) fail(w http.ResponseWriter, r *http.Request, status int, msg string) { + view := errorView{ + Page: a.page(r, http.StatusText(status)+" — "+serviceName), + Data: pages.Error(status, msg).BackTo("/", "Return to the dashboard"), + } + a.render(w, status, pages.ErrorPage, view) +} + // notFound renders the 404 page. Used both for genuinely missing repos and to // hide the existence of PRIVATE repos the caller may not browse. func (a *app) notFound(w http.ResponseWriter, r *http.Request) { - view := struct { - chrome.Page - }{Page: a.page(r, "Not found — "+serviceName)} - a.render(w, http.StatusNotFound, "404.html", view) + a.fail(w, r, http.StatusNotFound, "") } // forbidden renders the 403 page for a denied but non-hidden request. func (a *app) forbidden(w http.ResponseWriter, r *http.Request, msg string) { - view := struct { - chrome.Page - Message string - }{Page: a.page(r, "Forbidden — "+serviceName), Message: msg} - a.render(w, http.StatusForbidden, "403.html", view) + a.fail(w, r, http.StatusForbidden, msg) } // redirectLogin sends an unauthenticated caller to meta's login, returning them diff --git a/web/templates.go b/web/templates.go index 423cfecfd021536d93c8c989a582a06eeebe8929..a9079e2852e69946378125990fd039cba56d975a 100644 --- a/web/templates.go +++ b/web/templates.go @@ -5,96 +5,43 @@ import ( "fmt" "html/template" "io/fs" + "log" "net/http" "net/url" "strings" - "time" - "sourcecraft.dev/bigbes/sr-ht-ecore/chrome" + "sourcecraft.dev/bigbes/sr-ht-ecore/pages" ) //go:embed templates/*.html templates/icons/*.svg var templateFS embed.FS -// pageTemplates lists every content page. Each is parsed together with the -// shared layout, nav and partials into its own *template.Template, so the -// per-page {{define "content"}} blocks never collide. -var pageTemplates = []string{ - "index.html", - "create.html", - "user.html", - "overview.html", - "log.html", - "commit.html", - "tree.html", - "table.html", - "settings.html", - "keys.html", - "404.html", - "403.html", -} - -// sharedTemplates are parsed into every page: the outer layout and this -// service's own reusable partials (badges, tab bars). The chrome partials — -// the brand, the switcher, the login block, the environment banner, the -// listing — are NOT here: they come from sr-ht-ecore and are attached to every -// set by chrome.Attach (loadTemplates), which is the copy every custom service -// on the instance draws from. -var sharedTemplates = []string{ - "templates/layout.html", - "templates/partials.html", -} - -// templateSet maps a page name to its fully-parsed template (execute "layout"). -type templateSet map[string]*template.Template - -// loadTemplates parses every page template with the shared chrome and the -// funcmap. It fails loudly (returns an error) on any parse problem so a broken -// template surfaces at startup, never as a blank page at request time. -func loadTemplates() (templateSet, error) { +// loadPages parses one template set per page in templates/, through +// sr-ht-ecore's pages: the layout, the shared chrome partials, our own +// _partials.html and that page's content. +// +// There is no list of pages here any more. Pages are discovered from the +// directory, so a new page — or a new View's beads.html — is registered by +// existing as a file, and the loader a registration used to have to remember is +// gone with it. A page that defines no "content" block, and a missing layout, +// are startup errors: the first would otherwise serve the chrome around a hole +// under a 200. +// +// The error page is not ours: pages ships one (registered as pages.ErrorPage) +// and parses its "srht-error" body into every set, so the 404 and 403 templates +// this service used to carry are gone rather than reworded. +func loadPages() (pages.Set, error) { icons, err := loadIcons() if err != nil { return nil, err } - funcs := templateFuncs(icons) - - set := make(templateSet, len(pageTemplates)) - for _, page := range pageTemplates { - t, err := chrome.Attach(template.New("layout").Funcs(funcs)) - if err != nil { - return nil, fmt.Errorf("web: attach the shared chrome partials for %s: %w", page, err) - } - files := append(append([]string{}, sharedTemplates...), "templates/"+page) - if _, err := t.ParseFS(templateFS, files...); err != nil { - return nil, fmt.Errorf("web: parse template %s: %w", page, err) - } - set[page] = t - } + return pages.Load(templateFS, pages.Options{Funcs: templateFuncs(icons)}) +} - // Parse the page template of every registered view the same way, so a - // concrete view (e.g. the future "beads" view) becomes renderable by adding - // only its own web/beads.go + templates/beads.html — the embed.FS glob picks - // the new file up at compile time and this loop parses it, with no edit to - // this loader. The registry is fully populated by the init()s that ran before - // Register called us. A view whose Template() is already a known page (which - // a test may deliberately reuse, e.g. "tree.html") is skipped rather than - // re-parsed, so registration never clobbers a page template. - for _, v := range registeredViews { - name := v.Template() - if _, ok := set[name]; ok { - continue - } - t, err := chrome.Attach(template.New("layout").Funcs(funcs)) - if err != nil { - return nil, fmt.Errorf("web: attach the shared chrome partials for %s: %w", name, err) - } - files := append(append([]string{}, sharedTemplates...), "templates/"+name) - if _, err := t.ParseFS(templateFS, files...); err != nil { - return nil, fmt.Errorf("web: parse view template %s: %w", name, err) - } - set[name] = t - } - return set, nil +// pageName maps a template file name ("beads.html", what a View declares) to +// the name pages registers it under ("beads", what Render takes). +func pageName(file string) string { + return strings.TrimSuffix(file, ".html") } // loadIcons reads every embedded icon SVG into a name→markup map for the icon @@ -120,24 +67,19 @@ func loadIcons() (map[string]template.HTML, error) { return icons, nil } -// templateFuncs is the funcmap available in every template. -// -// It starts from chrome.Funcs — "dict" and "shortsha", which the shared -// partials and half this family's pages were written against — and adds this -// service's own on top. Adding after is deliberate: a name may then be shadowed -// on purpose rather than by accident of map ordering. Nothing here re-defines a -// shared helper; the local copies of dict and the hash abbreviator are gone. +// templateFuncs is this service's own funcmap. pages merges it over +// chrome.Funcs — "dict", "shortsha", "reltime" and "abstime", which the shared +// partials and half this family's pages were written against — so only the +// helpers nobody else has are listed here. The local copies of the relative and +// absolute time formatters are gone with the rest; chrome's reltime also faces +// forward ("in 3 weeks"), where ours called every future instant "just now". func templateFuncs(icons map[string]template.HTML) template.FuncMap { - m := chrome.Funcs() + m := template.FuncMap{} // icon renders a named inline SVG (from templates/icons). An unknown name // yields empty output rather than a hard error, so a missing icon never // crashes a page. m["icon"] = func(name string) template.HTML { return icons[name] } - // reltime renders a humanized relative time ("3 hours ago"), no deps. - m["reltime"] = humanizeTime - // abstime renders an absolute UTC timestamp for tooltips/detail. - m["abstime"] = func(t time.Time) string { return t.UTC().Format("2006-01-02 15:04:05 UTC") } // humansize renders a byte count as a human-readable size. m["humansize"] = humanizeSize // "upper" and "lower" used to be here for the environment banner and the @@ -171,36 +113,6 @@ func doltHost(origin string) string { return u.Host + ":443" } -// humanizeTime renders t as a coarse relative time in the past. It is a small -// self-contained helper (no new dependency) covering seconds→years. -func humanizeTime(t time.Time) string { - d := time.Since(t) - if d < 0 { - return "just now" - } - switch { - case d < time.Minute: - return "just now" - case d < time.Hour: - return plural(int(d/time.Minute), "minute") - case d < 24*time.Hour: - return plural(int(d/time.Hour), "hour") - case d < 30*24*time.Hour: - return plural(int(d/(24*time.Hour)), "day") - case d < 365*24*time.Hour: - return plural(int(d/(30*24*time.Hour)), "month") - default: - return plural(int(d/(365*24*time.Hour)), "year") - } -} - -func plural(n int, unit string) string { - if n == 1 { - return "1 " + unit + " ago" - } - return fmt.Sprintf("%d %ss ago", n, unit) -} - // humanizeSize renders a byte count with binary (1024) units. func humanizeSize(n uint64) string { const unit = 1024 @@ -215,22 +127,19 @@ func humanizeSize(n uint64) string { return fmt.Sprintf("%.1f %ciB", float64(n)/float64(div), "KMGTPE"[exp]) } -// render executes the named page template with the layout, writing an HTML -// response with the given status. A template execution error is a programming -// error (bad template or view struct); it is logged and a 500 is written, but -// never a partially-flushed page — we render into a buffer first. +// render is pages.Set.Render with this service's log line on the end. +// +// It is the whole of what is left of the old renderer, and the deleted half is +// the point: the previous one wrote "template render error: "+err.Error() into +// the response body, publishing template names, field paths and whatever the +// payload's String method produced to whoever asked for the page. pages answers +// a fixed sentence and hands the error back for the log, which is where a +// broken template belongs. +// +// A returned error means the response is already answered; there is nothing to +// do with it here but say so. func (a *app) render(w http.ResponseWriter, status int, page string, data any) { - t, ok := a.templates[page] - if !ok { - http.Error(w, "template not found", http.StatusInternalServerError) - return - } - var buf strings.Builder - if err := t.ExecuteTemplate(&buf, "layout", data); err != nil { - http.Error(w, "template render error: "+err.Error(), http.StatusInternalServerError) - return + if err := a.pages.Render(w, status, page, data); err != nil { + log.Printf("web: %v", err) } - w.Header().Set("Content-Type", "text/html; charset=utf-8") - w.WriteHeader(status) - _, _ = w.Write([]byte(buf.String())) } diff --git a/web/templates/403.html b/web/templates/403.html deleted file mode 100644 index 1c794a5c76a120d9fd2115d9074744de2f67733f..0000000000000000000000000000000000000000 --- a/web/templates/403.html +++ /dev/null @@ -1,6 +0,0 @@ -{{define "content" -}} -
-

403 — Forbidden

-

{{if .Message}}{{.Message}}{{else}}You do not have access to this resource.{{end}}

-

Return to the dashboard

-{{- end}} diff --git a/web/templates/404.html b/web/templates/404.html deleted file mode 100644 index 8a6fb5495574de2088834712144c6f3ce9831716..0000000000000000000000000000000000000000 --- a/web/templates/404.html +++ /dev/null @@ -1,6 +0,0 @@ -{{define "content" -}} -
-

404 — Not found

-

The page or database you requested does not exist.

-

Return to the dashboard

-{{- end}} diff --git a/web/templates/partials.html b/web/templates/_partials.html similarity index 100% rename from web/templates/partials.html rename to web/templates/_partials.html diff --git a/web/templates/layout.html b/web/templates/layout.html index cfba3cc769007d581a771a8b15280c229e9330be..59a2a52a209923798252a1924d3f210fb9babbf0 100644 --- a/web/templates/layout.html +++ b/web/templates/layout.html @@ -9,8 +9,15 @@ width is .ContainerClass rather than a literal "container": the row browser hands out "container-fluid" so a wide table gets the whole viewport, and every other page gets the centered default chrome.Page already carries. -*/}} -{{define "layout" -}} + + The file is the layout's body rather than a {{define "layout"}} block: pages + names each set after this file and executes the set itself, so a body wrapped + in a define would leave the template that is actually executed empty. + + The content hole is {{template}} and never {{block}}: a block would define an + empty default for "content" on every page at once, which is exactly what + pages.Load refuses a page for at startup. +*/ -}} @@ -32,4 +39,3 @@ -{{- end}} diff --git a/web/views.go b/web/views.go index bbce981210da037905e46bc296be570598e8610f..b7e5a908b0628d05cbb299544e2f45473dee3404 100644 --- a/web/views.go +++ b/web/views.go @@ -28,17 +28,20 @@ type View interface { } // registeredViews is the package-global registry populated at init time by -// RegisterView. It is read once per process: loadTemplates parses each view's -// Template() at startup, and Register snapshots it into app.views so handlers -// (and tests, which set app.views directly) never touch the global at request -// time. +// RegisterView. It is read once per process: Register snapshots it into +// app.views so handlers (and tests, which set app.views directly) never touch +// the global at request time. +// +// Nothing here parses a view's Template() any more: pages discovers every page +// in templates/, so a view's page is registered by its file existing. A view +// whose Template() names no such file renders as a 500 with a log line, which +// is what it is. var registeredViews []View // RegisterView adds v to the global registry. It is meant to be called from an -// init() in the file that defines a concrete view, so the registry is fully -// populated before loadTemplates runs and the first request arrives. If a view -// with the same Name() is already registered it is replaced, so double -// registration (e.g. from a duplicated init) is safe. +// init() in the file that defines a concrete view. If a view with the same +// Name() is already registered it is replaced, so double registration (e.g. +// from a duplicated init) is safe. func RegisterView(v View) { for i, existing := range registeredViews { if existing.Name() == v.Name() { diff --git a/web/views_test.go b/web/views_test.go index 0e6d6635c358cf61c2397d45a97b154475e73643..752736c6d7f67041aa15fac0507616b90a0c3803 100644 --- a/web/views_test.go +++ b/web/views_test.go @@ -2,29 +2,31 @@ package web import ( "context" + "fmt" "net/http" "net/url" "regexp" "strings" "testing" + "sourcecraft.dev/bigbes/sr-ht-ecore/pages" + "sourcecraft.dev/bigbes/sr-ht-dolt/browse" "sourcecraft.dev/bigbes/sr-ht-dolt/core" ) // dummyView is a test-only View that fingerprints repos containing a table -// named "issues". It renders through an EXISTING page template ("404.html") so -// the test needs no newly embedded file. That template references only the -// shared chrome (basePage), so it renders cleanly with the fixed view envelope -// (which does not carry a .Tables field the way tree.html would demand). It -// records whether Build ran. +// named "issues". It renders through the shared error page, which is the one +// page every service's set is guaranteed to carry and the only one that asks +// nothing of the envelope beyond the .Data field every view already has. Its +// Build therefore returns a pages.ErrorData. It records whether Build ran. type dummyView struct { built bool } func (d *dummyView) Name() string { return "issues" } func (d *dummyView) Label() string { return "Issues" } -func (d *dummyView) Template() string { return "404.html" } +func (d *dummyView) Template() string { return pages.ErrorPage + ".html" } func (d *dummyView) Applies(tables []browse.TableInfo) bool { for _, t := range tables { @@ -38,12 +40,12 @@ func (d *dummyView) Applies(tables []browse.TableInfo) bool { func (d *dummyView) Build(_ context.Context, sess BrowseSession, _ *core.Repo, ref string, _ url.Values) (any, error) { d.built = true // Pull data through the BrowseSession surface only, mirroring how a real - // view works, and return the tables so tree.html renders something. + // view works, then hand the page the payload it reads. tables, err := sess.Tables(context.Background(), ref) if err != nil { return nil, err } - return tables, nil + return pages.Error(http.StatusOK, fmt.Sprintf("%d tables", len(tables))), nil } func issueTables() []browse.TableInfo { @@ -84,10 +86,10 @@ type alwaysView struct{} func (alwaysView) Name() string { return "always" } func (alwaysView) Label() string { return "Always" } -func (alwaysView) Template() string { return "404.html" } +func (alwaysView) Template() string { return pages.ErrorPage + ".html" } func (alwaysView) Applies([]browse.TableInfo) bool { return true } func (alwaysView) Build(context.Context, BrowseSession, *core.Repo, string, url.Values) (any, error) { - return nil, nil + return pages.Error(http.StatusOK, ""), nil } func TestHandleViewSelectedAndBuilt(t *testing.T) { diff --git a/web/web_test.go b/web/web_test.go index 4ce2f3caf0b1147f02071b0d57af07c984797220..400e27362904762e1637c88db89622f656805c83 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -21,6 +21,8 @@ import ( "github.com/vaughan0/go-ini" "sourcecraft.dev/bigbes/sr-ht-core/auth" + "sourcecraft.dev/bigbes/sr-ht-ecore/pages" + "sourcecraft.dev/bigbes/sr-ht-dolt/authn" "sourcecraft.dev/bigbes/sr-ht-dolt/browse" "sourcecraft.dev/bigbes/sr-ht-dolt/core" @@ -638,6 +640,62 @@ func TestPagesAreDrawnThroughTheSharedChrome(t *testing.T) { "the row browser must be full-bleed") } +// The 404 and 403 templates this service carried are sr-ht-ecore's error page +// now: one body, drawn through our own chrome, with the shared sentence. The +// wording matters here — a 404 that described the missing thing would tell an +// anonymous viewer which private databases exist. +func TestRefusalsRenderTheSharedErrorPage(t *testing.T) { + h := newHarness(t) + h.store.add(&core.Repo{Name: "db", OwnerID: 10, OwnerName: "owner", Path: "/d", Visibility: core.VisibilityPublic}) + + missing := h.do("GET", "/~owner/nosuch", nil, nil) + require.Equal(t, http.StatusNotFound, missing.Code) + assert.Contains(t, missing.Body.String(), pages.NotFoundMessage) + assert.Contains(t, missing.Body.String(), "404 — Not Found") + assert.Contains(t, missing.Body.String(), `dolt`, + "the error page is drawn through our chrome") + + denied := h.do("GET", "/~owner/db/settings", testCaller(99, "intruder"), nil) + require.Equal(t, http.StatusForbidden, denied.Code) + assert.Contains(t, denied.Body.String(), "Only the owner may change database settings.") +} + +// leakyView renders a page whose content block reads a field its envelope does +// not carry, so executing it fails halfway. It is the shape of the bug the old +// renderer turned into a disclosure. +type leakyView struct{} + +func (leakyView) Name() string { return "leaky" } +func (leakyView) Label() string { return "Leaky" } +func (leakyView) Template() string { return "keys.html" } +func (leakyView) Applies([]browse.TableInfo) bool { return true } +func (leakyView) Build(context.Context, BrowseSession, *core.Repo, string, url.Values) (any, error) { + return nil, nil +} + +// A template that fails halfway answers a fixed sentence. The previous renderer +// wrote "template render error: "+err.Error() into the body, which hands the +// viewer the template's name and the field path that was not there; the error +// belongs in the log and nowhere else. +func TestATemplateFailureTellsTheViewerNothing(t *testing.T) { + h := newHarness(t) + h.store.add(&core.Repo{Name: "db", OwnerID: 1, OwnerName: "alice", Path: "/d", Visibility: core.VisibilityPublic}) + h.browse.sess = &fakeSession{ + branches: []browse.Branch{{Name: "main", Head: "abcdef1234567890"}}, + tables: issueTables(), + } + setViews(t, h, leakyView{}) + + rec := h.do("GET", "/~alice/db/view/leaky", nil, nil) + require.Equal(t, http.StatusInternalServerError, rec.Code) + + body := rec.Body.String() + assert.Equal(t, "internal server error\n", body) + assert.NotContains(t, body, "keys.html", "the template name must not reach the viewer") + assert.NotContains(t, body, "Keys", "the field path must not reach the viewer") + assert.NotContains(t, body, "can't evaluate") +} + // The static tree is served by sr-ht-ecore's assets handler, which is tested // there. What is ours is that we mounted it: that the hashed stylesheet this // build produced is the one the layout links, that a name whose bytes cannot