Close the retention follow-ups from the August review (closes #99)
check / check (push) Successful in 3m16s
check / check (push) Successful in 3m16s
A retention value above the keep-forever sentinel is now pinned by a test to be stored as the sentinel. parseRetentionDays returns the form's message directly, so the two error values that only existed to pick that message are gone. The sweep no longer checks keep-forever itself; retentionCutoff does, before the webhook's database is opened. The create-form refill test uses HTML-special characters and checks they come back escaped. Model: opus-5-5
This commit is contained in:
@@ -41,71 +41,51 @@ type WebhookListItem struct {
|
||||
// errMissingURL signals that a required URL was not provided.
|
||||
var errMissingURL = errors.New("missing URL")
|
||||
|
||||
// errInvalidRetention signals a retention_days form value that is not
|
||||
// a non-negative whole number.
|
||||
var errInvalidRetention = errors.New("invalid retention days")
|
||||
|
||||
// errRetentionTooLarge signals a retention_days form value that is a
|
||||
// whole number but larger than the reaper's cutoff arithmetic can
|
||||
// represent. It is distinguished from errInvalidRetention so the form
|
||||
// can tell the user the actual ceiling instead of implying their input
|
||||
// was not a number.
|
||||
var errRetentionTooLarge = errors.New("retention days out of range")
|
||||
|
||||
// retentionErrorMessage returns the message the create and edit forms
|
||||
// show the user for a rejected retention_days value. Any error other
|
||||
// than errRetentionTooLarge falls back to the generic wording, so an
|
||||
// unrecognised parse failure still produces a sensible 400 rather than
|
||||
// an empty alert.
|
||||
func retentionErrorMessage(err error) string {
|
||||
if errors.Is(err, errRetentionTooLarge) {
|
||||
return "Retention must be at most " +
|
||||
strconv.Itoa(database.MaxFiniteRetentionDays) +
|
||||
" days, or 0 to retain events forever."
|
||||
}
|
||||
|
||||
return "Retention must be a whole number of days, or 0 to " +
|
||||
"retain events forever."
|
||||
}
|
||||
|
||||
// parseRetentionDays interprets a retention_days form value.
|
||||
// parseRetentionDays interprets a retention_days form value. It
|
||||
// returns the number of days, or, for a value it refuses, the message
|
||||
// the create and edit forms show; the message is empty when the value
|
||||
// is accepted.
|
||||
//
|
||||
// An empty value yields fallback, which lets the create path apply the
|
||||
// default and the edit path leave the stored value unchanged. A value
|
||||
// of 0 is returned as 0 and is rewritten to the retain-forever
|
||||
// sentinel by database.Webhook's BeforeSave hook. Anything unparseable
|
||||
// or negative is an error rather than a silently substituted default.
|
||||
// or negative is refused rather than silently given a default.
|
||||
//
|
||||
// The upper bound is not cosmetic. The reaper computes its cutoff as a
|
||||
// time.Duration, an int64 nanosecond count, so a day count above
|
||||
// database.MaxFiniteRetentionDays overflows, puts the cutoff in the
|
||||
// future, and deletes every event the webhook has. A finite value
|
||||
// above that ceiling is therefore a 400.
|
||||
// above that ceiling is therefore refused, and the message names the
|
||||
// ceiling rather than implying the input was not a number.
|
||||
//
|
||||
// A value at or above the retain-forever sentinel is not out of range:
|
||||
// it is what the edit form pre-fills for a retain-forever webhook, so
|
||||
// submitting the form back unchanged has to keep meaning "forever"
|
||||
// rather than being rejected.
|
||||
func parseRetentionDays(raw string, fallback int) (int, error) {
|
||||
func parseRetentionDays(raw string, fallback int) (int, string) {
|
||||
raw = strings.TrimSpace(raw)
|
||||
if raw == "" {
|
||||
return fallback, nil
|
||||
return fallback, ""
|
||||
}
|
||||
|
||||
v, err := strconv.Atoi(raw)
|
||||
if err != nil || v < 0 {
|
||||
return 0, errInvalidRetention
|
||||
return 0, "Retention must be a whole number of days, or 0 to " +
|
||||
"retain events forever."
|
||||
}
|
||||
|
||||
if v >= database.RetentionForeverDays {
|
||||
return database.RetentionForeverDays, nil
|
||||
return database.RetentionForeverDays, ""
|
||||
}
|
||||
|
||||
if v > database.MaxFiniteRetentionDays {
|
||||
return 0, errRetentionTooLarge
|
||||
return 0, "Retention must be at most " +
|
||||
strconv.Itoa(database.MaxFiniteRetentionDays) +
|
||||
" days, or 0 to retain events forever."
|
||||
}
|
||||
|
||||
return v, nil
|
||||
return v, ""
|
||||
}
|
||||
|
||||
// DeliveryView is the display-safe projection of a delivery
|
||||
@@ -361,16 +341,13 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
retentionDays, retErr := parseRetentionDays(
|
||||
retentionDays, errMsg := parseRetentionDays(
|
||||
retentionStr, database.DefaultRetentionDays,
|
||||
)
|
||||
if retErr != nil {
|
||||
if errMsg != "" {
|
||||
h.renderTemplateStatus(
|
||||
w, r, "sources_new.html",
|
||||
newSourceFormData(
|
||||
retentionErrorMessage(retErr),
|
||||
name, description,
|
||||
),
|
||||
newSourceFormData(errMsg, name, description),
|
||||
http.StatusBadRequest,
|
||||
)
|
||||
|
||||
@@ -655,13 +632,13 @@ func (h *Handlers) applyWebhookEdit(
|
||||
|
||||
// An empty field falls back to the stored value, so submitting the
|
||||
// form without touching retention leaves the policy alone.
|
||||
retentionDays, retErr := parseRetentionDays(
|
||||
retentionDays, errMsg := parseRetentionDays(
|
||||
r.PostFormValue("retention_days"), webhook.RetentionDays,
|
||||
)
|
||||
if retErr != nil {
|
||||
if errMsg != "" {
|
||||
data := map[string]any{
|
||||
tmplKeyWebhook: webhook,
|
||||
tmplKeyError: retentionErrorMessage(retErr),
|
||||
tmplKeyError: errMsg,
|
||||
}
|
||||
|
||||
h.renderTemplateStatus(w, r, "source_edit.html", data, http.StatusBadRequest)
|
||||
|
||||
Reference in New Issue
Block a user