audit 4: log paging at its edges, NaN refused, a red rate limit, recalculation asks, reload writes all
test / test (push) Canceled after 0s

Step 4 of stage 4 (L10, L11, L13, L18, L19 and a row of
review-2026-08-followups):

- The Outbound log counts first and clamps the page to the last; a number
  too large for an int (strconv's ErrRange with the largest int) is the
  last page on both logs. Tested at the edges: first, last, past the last
  however far, nonsense, an empty log asked for page 5.
- checkAutoMultiplier is "not within", so NaN and the infinities are out.
- rate-limited is red (components.go; the mockup first, 643892a).
- data-unsaved on both recalculate buttons: panel.js asks on submit
  (ev.submitter) when a field of the page differs from what it was drawn
  with; testdata/unsaved_test.js under TestPanelJS.
- HandleReload writes what a restore's first start writes - the filter
  lists and the DMARC maps too - and a failed step is an error flash on
  Health that names it, not plain text; the success flash says what was
  written.

Tested on the test server: go vet + go test of 25 packages, make e2e (all
pass, 695 s) on this tree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
mix
2026-10-15 11:20:18 +03:00
co-authored by Claude Opus 5.5
parent 643892ab91
commit ca876c1631
23 changed files with 355 additions and 48 deletions
@@ -1,6 +1,7 @@
package handlers
import (
"errors"
"net/http"
"net/http/httptest"
"net/url"
@@ -10,6 +11,7 @@ import (
"github.com/mixeme/selfpost/internal/dmarc"
"github.com/mixeme/selfpost/internal/domain"
"github.com/mixeme/selfpost/internal/health"
"github.com/mixeme/selfpost/internal/postfix"
"github.com/mixeme/selfpost/internal/store"
"github.com/mixeme/selfpost/internal/web/auth"
@@ -293,3 +295,37 @@ func TestDeletingAUserLeavesDomainAddressesAndNamesNobody(t *testing.T) {
confirm := send(s.h.HandleUserDeleteConfirm, &globalPrincipal, "GET", "/server/users/"+idStr(admin.ID)+"/delete", map[string]string{"uid": idStr(admin.ID)}, nil).Body.String()
lacks(t, "delete confirmation", confirm, `report address`, `will have no`)
}
// failingDMARCMaps stands in for Postfix refusing the DMARC maps.
type failingDMARCMaps struct{}
func (failingDMARCMaps) RebuildDMARCMaps(postfix.DMARCMapsConfig) error {
return errors.New("disk full")
}
// Reload configuration writes what a restore's first start writes, the DMARC
// maps among it, and a failure is a message on Health that names the step,
// not a plain-text page (audit 2026-10-12, L13).
func TestReloadWritesTheDMARCMapsAndSaysWhereItFailed(t *testing.T) {
s, maps := hostedStand(t)
// An OpenDKIM that reloads nothing - the stand has no supervisor - and
// a sampler for the Health page a failure is shown on.
odk := domain.NewOpenDKIM(t.TempDir())
odk.SetReloadHook(func() error { return nil })
s.h.domains = domain.NewService(s.h.store, odk, s.h.apps, "mail")
s.h.machine = &health.MachineSampler{}
rec := postFormAs(s.h.HandleReload, globalPrincipal, "/server/health/reload", nil, url.Values{})
if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/server/health?reloaded=1" {
t.Fatalf("reload = %d, Location %q", rec.Code, rec.Header().Get("Location"))
}
if len(maps.got) != 1 {
t.Errorf("the reload rebuilt the DMARC maps %d times, want once", len(maps.got))
}
s.h.dmarc = dmarc.NewService(s.h.store, failingDMARCMaps{}, "mail.example.org", true)
rec = postFormAs(s.h.HandleReload, globalPrincipal, "/server/health/reload", nil, url.Values{})
if rec.Code != http.StatusInternalServerError {
t.Fatalf("a failed reload = %d, want 500", rec.Code)
}
has(t, "failed reload", rec.Body.String(), "Reload failed at the DMARC report maps", "Reload configuration")
}
+28 -13
View File
@@ -162,26 +162,41 @@ func (h *Handlers) HandleDeleteDomain(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, "/outbound/domains?deleted=1", http.StatusSeeOther)
}
// HandleReload re-applies both the OpenDKIM configuration and the Postfix
// sender map on demand (architecture.md § Panel HTTP surface).
// HandleReload writes again, from the database, every file the panel keeps
// for the mail path and has OpenDKIM and Postfix re-read them - what the
// first start after a restore does (cmd/panel mailStack.Resync): OpenDKIM's
// tables, the sender map, with the inbound relay on its maps and the filter
// lists, with DMARC reports on their maps (architecture.md § Panel HTTP
// surface). Before 2.5.4 it left the last two out, and said "regenerated"
// over them (audit 2026-10-12, L13). A failure is a message on Health that
// names the step; the log has the error.
func (h *Handlers) HandleReload(w http.ResponseWriter, r *http.Request) {
if _, ok := h.requireGlobal(w, r); !ok {
return
}
if err := h.domains.Resync(); err != nil {
logf("panel: manual reload (opendkim): %v", err)
http.Error(w, "reload failed", http.StatusInternalServerError)
return
type step struct {
name string
run func() error
}
if err := h.apps.Resync(); err != nil {
logf("panel: manual reload (postfix): %v", err)
http.Error(w, "reload failed", http.StatusInternalServerError)
return
steps := []step{
{"OpenDKIM's tables", h.domains.Resync},
{"the sender map", h.apps.Resync},
}
if h.cfg.InboundEnabled && h.inbound != nil {
if err := h.inbound.Resync(); err != nil {
logf("panel: manual reload (inbound): %v", err)
http.Error(w, "reload failed", http.StatusInternalServerError)
steps = append(steps, step{"the inbound relay maps", h.inbound.Resync})
}
if h.cfg.InboundEnabled && h.lists != nil {
steps = append(steps, step{"the filter lists", h.lists.Sync})
}
if h.dmarc != nil && h.dmarc.Enabled() {
steps = append(steps, step{"the DMARC report maps", h.dmarc.Resync})
}
for _, s := range steps {
if err := s.run(); err != nil {
logf("panel: manual reload (%s): %v", s.name, err)
page := h.healthView(r, "")
page.Flash = &view.Flash{Error: true, Text: view.Plain("Reload failed at " + s.name + "; what came before it was written and re-read. The panel's log says why.")}
h.view.Render(w, http.StatusInternalServerError, "health", page)
return
}
}
@@ -229,6 +229,8 @@ func TestInboundLogPages(t *testing.T) {
// A page past the end, or nonsense, is the last page, or the first.
has(t, "page 9", s.get(s.h.HandleInboundLog, "/inbound/log?domain=a.example.com&p=9", globalPrincipal).Body.String(), `Page 2 of 2`, `Hello A`)
has(t, "page x", s.get(s.h.HandleInboundLog, "/inbound/log?domain=a.example.com&p=x", globalPrincipal).Body.String(), `Page 1 of 2`)
has(t, "page huge", s.get(s.h.HandleInboundLog, "/inbound/log?domain=a.example.com&p=99999999999999999999", globalPrincipal).Body.String(), `Page 2 of 2`, `Hello A`)
has(t, "page 0", s.get(s.h.HandleInboundLog, "/inbound/log?domain=a.example.com&p=0", globalPrincipal).Body.String(), `Page 1 of 2`)
}
func TestInboundLogEmptyAndOff(t *testing.T) {
@@ -237,6 +239,9 @@ func TestInboundLogEmptyAndOff(t *testing.T) {
body := s.get(h.HandleInboundLog, "/inbound/log", globalPrincipal).Body.String()
has(t, "empty log", body, `No messages logged yet.`)
lacks(t, "empty log", body, `<table`, `Page 1`)
body = s.get(h.HandleInboundLog, "/inbound/log?p=5", globalPrincipal).Body.String()
has(t, "empty log, page 5", body, `No messages logged yet.`)
lacks(t, "empty log, page 5", body, `<table`, `Page `)
h.cfg.InboundEnabled = false
if rec := s.get(h.HandleInboundLog, "/inbound/log", globalPrincipal); rec.Code != http.StatusNotFound {
+16 -7
View File
@@ -380,21 +380,22 @@ func (h *Handlers) outLogPage(r *http.Request) (*view.OutLog, error) {
}
}
page := parsePage(q.Get("p"))
total, err := h.store.CountSendLog(filter)
if err != nil {
return nil, err
}
rows, err := h.store.QuerySendLog(filter, sendLogPageSize, (page-1)*sendLogPageSize)
if err != nil {
return nil, err
}
lastPage := 1
if total > 0 {
lastPage = int((total + sendLogPageSize - 1) / sendLogPageSize)
}
// A page past the end is the last one, as on the Inbound log: a link
// kept while the log was pruned, or a number typed, shows rows, not an
// empty table under "Page 9 of 2" (audit 2026-10-12, L10).
page := min(parsePage(q.Get("p")), lastPage)
rows, err := h.store.QuerySendLog(filter, sendLogPageSize, (page-1)*sendLogPageSize)
if err != nil {
return nil, err
}
out := view.NewOutLog(h.shellMeta(r), h.sendLogRetentionDays(), p.IsGlobal(), domainNames, logins, filter.Domain, filter.AppLogin)
table := make([]view.OutLogRow, len(rows))
for i, row := range rows {
@@ -409,8 +410,16 @@ func (h *Handlers) outLogPage(r *http.Request) (*view.OutLog, error) {
// parsePage clamps the "p" query parameter to a valid page number, defaulting
// to 1 for anything missing or malformed rather than rejecting the request.
//
// A number too large for an int is past every last page, and is taken as
// that: strconv returns it as the largest int with ErrRange (Go 1.26,
// strconv.ParseInt: "the maximum magnitude integer of the appropriate
// bitSize and sign"), and the caller clamps it to the last page.
func parsePage(v string) int {
n, err := strconv.Atoi(v)
if errors.Is(err, strconv.ErrRange) && n > 0 {
return n
}
if err != nil || n < 1 {
return 1
}
@@ -74,6 +74,19 @@ func TestOutLogPagesKeepTheFiltersAndThePage(t *testing.T) {
lacks(t, "page 2", second, `Older →`)
// A row's page is opened from the page it is on and leads back to it.
has(t, "page 2", second, `?domain=bs.example.ru&amp;p=2">Details</a>`)
// The edges (audit 2026-10-12, L10): a page past the end is the last,
// however far past; nonsense, zero or a negative number is the first.
for _, p := range []string{"3", "9", "99999999999999999999", "9223372036854775807"} {
body := getBody(t, h.HandleDeliveries, "/outbound/log?domain=bs.example.ru&p="+p)
has(t, "page "+p, body, `Page 2 of 2`, `<a href="/outbound/log?domain=bs.example.ru">← Newer</a>`)
if got := strings.Count(body, `">Details</a>`); got == 0 {
t.Errorf("page %s shows no rows", p)
}
}
for _, p := range []string{"0", "-1", "x", ""} {
has(t, "page "+p, getBody(t, h.HandleDeliveries, "/outbound/log?domain=bs.example.ru&p="+p), `Page 1 of 2`)
}
}
// Nothing logged yet is a sentence in the polled region, which goes on polling,
@@ -84,6 +97,10 @@ func TestOutLogEmpty(t *testing.T) {
body := getBodyAs(t, h.HandleDeliveries, "/outbound/log", p)
has(t, "empty log", body, `No messages logged yet.`, `id="out-log-rows"`, `data-poll`, `New rows appear on their own`)
lacks(t, "empty log", body, `<table`, `Page 1`)
// A page asked of an empty log is the same empty log.
body = getBodyAs(t, h.HandleDeliveries, "/outbound/log?p=5", p)
has(t, "empty log, page 5", body, `No messages logged yet.`)
lacks(t, "empty log, page 5", body, `<table`, `Page `)
// Only the global role is pointed at the setting that decides how long rows live.
lacks(t, "empty log", body, `href="/server/settings"`)
has(t, "log", getBody(t, h.HandleDeliveries, "/outbound/log"), `href="/server/settings">Settings</a>`)
+4 -1
View File
@@ -50,8 +50,11 @@ func parseAutoMultiplier(raw string) (float64, error) {
// checkAutoMultiplier is the rule for an auto limit's multiplier, shared by
// the form and a domain import.
// Written as "not within" so NaN, which compares false with everything, is
// outside too: ParseFloat reads "NaN" (audit 2026-10-12, L11 - stored as NULL,
// read back as 0, recalculated as the default without a word).
func checkAutoMultiplier(v float64) error {
if v < store.MinAutoMultiplier || v > store.MaxAutoMultiplier {
if !(v >= store.MinAutoMultiplier && v <= store.MaxAutoMultiplier) {
return fmt.Errorf("multiplier must be between %.1f and %.1f", store.MinAutoMultiplier, store.MaxAutoMultiplier)
}
return nil
+1 -1
View File
@@ -501,7 +501,7 @@ func queueSummary(out string) string {
func statusFlash(r *http.Request) string {
switch {
case r.URL.Query().Get("reloaded") != "":
return "Configuration regenerated from the database; OpenDKIM and Postfix have re-read it."
return "Configuration regenerated from the database - OpenDKIM's tables, every map the panel writes for Postfix, the filter lists - and OpenDKIM and Postfix have re-read it."
case r.URL.Query().Get("rechecked") != "":
return "DNS re-checked."
default:
@@ -0,0 +1,23 @@
package handlers
import (
"testing"
"github.com/mixeme/selfpost/internal/store"
)
// An auto limit's multiplier is a number within its bounds: NaN and the
// infinities, which strconv reads, are refused like any other number out of
// range (audit 2026-10-12, L11).
func TestAutoMultiplierIsANumberInRange(t *testing.T) {
for _, raw := range []string{"NaN", "nan", "+Inf", "-Inf", "inf", "1.4", "5.1", "x"} {
if v, err := parseAutoMultiplier(raw); err == nil {
t.Errorf("%q accepted as %v", raw, v)
}
}
for raw, want := range map[string]float64{"": store.DefaultAutoMultiplier, "1.5": 1.5, " 2.5 ": 2.5, "5": 5} {
if v, err := parseAutoMultiplier(raw); err != nil || v != want {
t.Errorf("%q = %v, %v; want %v", raw, v, err, want)
}
}
}