Describe code in its own terms, not by an earlier build's data (closes #522)
check / check (push) Waiting to run
check / check (push) Waiting to run
Several comments justified code by what an earlier build would have left in a database or on disk. Each now says what the code does: reserveSQLiteFile sets the mode on every open whoever created the file, and expired treats a session lacking a deadline's timestamp as expired. The comments on retention, resubmitted events, archive pruning, target headers, the redactor and the delivery result view drop their mentions of older rows. Comments only. Model: opus-5-5
This commit is contained in:
@@ -45,8 +45,7 @@ type Event struct {
|
|||||||
|
|
||||||
// ResubmittedFromID names the event this one was copied from by
|
// ResubmittedFromID names the event this one was copied from by
|
||||||
// an operator resubmit. It is nil for an event that arrived on
|
// an operator resubmit. It is nil for an event that arrived on
|
||||||
// the receiver, which is every event created before the column
|
// the receiver. It is not a foreign key: the source event can be
|
||||||
// existed. It is not a foreign key: the source event can be
|
|
||||||
// reaped by retention while its copies remain, and the id is
|
// reaped by retention while its copies remain, and the id is
|
||||||
// kept as the record of where the copy came from either way.
|
// kept as the record of where the copy came from either way.
|
||||||
ResubmittedFromID *string `gorm:"type:uuid;index:idx_events_resubmitted_from_id,priority:1;index:idx_events_entrypoint_id,priority:3" json:"resubmittedFromId,omitempty"`
|
ResubmittedFromID *string `gorm:"type:uuid;index:idx_events_resubmitted_from_id,priority:1;index:idx_events_entrypoint_id,priority:3" json:"resubmittedFromId,omitempty"`
|
||||||
|
|||||||
@@ -98,8 +98,8 @@ func (w *Webhook) BeforeSave(_ *gorm.DB) error {
|
|||||||
// are exempt from reaping.
|
// are exempt from reaping.
|
||||||
//
|
//
|
||||||
// It accepts the RetentionForeverDays sentinel written by BeforeSave
|
// It accepts the RetentionForeverDays sentinel written by BeforeSave
|
||||||
// and, defensively, the non-positive values that rows written before
|
// and, defensively, any non-positive value, which BeforeSave treats
|
||||||
// the sentinel existed may still carry.
|
// as a request to retain forever.
|
||||||
func retainsForever(retentionDays int) bool {
|
func retainsForever(retentionDays int) bool {
|
||||||
return retentionDays <= 0 ||
|
return retentionDays <= 0 ||
|
||||||
retentionDays >= RetentionForeverDays
|
retentionDays >= RetentionForeverDays
|
||||||
|
|||||||
@@ -253,7 +253,7 @@ func (r *RetentionReaper) reapWebhook(
|
|||||||
// row in the database: the sweep then deletes every event, delivery,
|
// row in the database: the sweep then deletes every event, delivery,
|
||||||
// and delivery result, including ones created seconds ago. Rejecting
|
// and delivery result, including ones created seconds ago. Rejecting
|
||||||
// out-of-range input at the form is the primary guard; saturating here
|
// out-of-range input at the form is the primary guard; saturating here
|
||||||
// means an old row, a migration, or a future call site cannot turn a
|
// means a value that reaches the row by any other path cannot turn a
|
||||||
// too-large retention into total data loss.
|
// too-large retention into total data loss.
|
||||||
func retentionCutoff(
|
func retentionCutoff(
|
||||||
now time.Time,
|
now time.Time,
|
||||||
|
|||||||
@@ -118,9 +118,8 @@ const SQLiteFilePerm fs.FileMode = 0o600
|
|||||||
// SQLite still reports the absence rather than this function
|
// SQLite still reports the absence rather than this function
|
||||||
// materializing an empty database the caller asked not to create.
|
// materializing an empty database the caller asked not to create.
|
||||||
//
|
//
|
||||||
// Chmod of a file that already exists is what tightens a data
|
// The mode is set on every open, whoever created the file, so a
|
||||||
// directory an earlier build left at 0644 — including a developer's
|
// database or sidecar found at a wider mode is tightened too.
|
||||||
// own scratch directory — without any migration machinery.
|
|
||||||
func reserveSQLiteFile(path string, create bool) error {
|
func reserveSQLiteFile(path string, create bool) error {
|
||||||
if create {
|
if create {
|
||||||
// gosec G304: the path is the database file the caller asked
|
// gosec G304: the path is the database file the caller asked
|
||||||
|
|||||||
@@ -201,8 +201,7 @@ func (s *ArchiveSweeper) sweep(ctx context.Context) {
|
|||||||
|
|
||||||
// sweepTarget prunes the archive of a single database target.
|
// sweepTarget prunes the archive of a single database target.
|
||||||
// A missing, empty, or "never" expiry parses as a zero duration
|
// A missing, empty, or "never" expiry parses as a zero duration
|
||||||
// and is skipped entirely, so those archives keep exactly the
|
// and is skipped entirely, so those archives are never pruned.
|
||||||
// behaviour they had before the sweep existed.
|
|
||||||
func (s *ArchiveSweeper) sweepTarget(target *database.Target) {
|
func (s *ArchiveSweeper) sweepTarget(target *database.Target) {
|
||||||
expiry, err := parseArchiveExpiry(target.Config)
|
expiry, err := parseArchiveExpiry(target.Config)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -82,8 +82,8 @@ func isReservedTargetHeader(name string) bool {
|
|||||||
// one pair silently overwriting the other.
|
// one pair silently overwriting the other.
|
||||||
//
|
//
|
||||||
// An input with no pairs yields an empty map, which omitempty drops
|
// An input with no pairs yields an empty map, which omitempty drops
|
||||||
// from the stored config: a target configured with no headers keeps
|
// from the stored config, so a target configured with no headers
|
||||||
// the same config JSON it had before this field existed.
|
// stores no headers key at all.
|
||||||
func ParseTargetHeaders(raw string) (map[string]string, error) {
|
func ParseTargetHeaders(raw string) (map[string]string, error) {
|
||||||
headers := make(map[string]string)
|
headers := make(map[string]string)
|
||||||
|
|
||||||
|
|||||||
@@ -13,9 +13,9 @@ import (
|
|||||||
const RedactionMarker = "(redacted)"
|
const RedactionMarker = "(redacted)"
|
||||||
|
|
||||||
// Redactor removes one target's own credential material from
|
// Redactor removes one target's own credential material from
|
||||||
// text that target's remote peer chose: a delivery response
|
// a delivery response body, whose text the target's remote
|
||||||
// body, or a delivery error stored before the delivery path
|
// peer chose, and from a stored delivery error as a second
|
||||||
// learned to mask the URLs it embeds.
|
// line behind the delivery path's own URL masking.
|
||||||
//
|
//
|
||||||
// It removes byte-identical echoes of strings taken from the
|
// It removes byte-identical echoes of strings taken from the
|
||||||
// target's stored configuration, and nothing else. Anything
|
// target's stored configuration, and nothing else. Anything
|
||||||
|
|||||||
@@ -12,13 +12,12 @@ import (
|
|||||||
//
|
//
|
||||||
// The bound is enforced here and in SQL, because this page's
|
// The bound is enforced here and in SQL, because this page's
|
||||||
// memory profile must not depend on a constant in another
|
// memory profile must not depend on a constant in another
|
||||||
// package staying where it is, and because rows predating the
|
// package staying where it is, or on every stored row having
|
||||||
// delivery engine's own cap or restored from an archive are
|
// been written by the delivery engine under its own cap.
|
||||||
// not covered by it at all.
|
|
||||||
//
|
//
|
||||||
// It happens to equal that engine cap, so a row written by the
|
// It happens to equal that engine cap, so a row written by the
|
||||||
// current engine reaches this one exactly and is never cut
|
// engine reaches this one exactly and is never cut twice.
|
||||||
// twice. Nothing here may assume the two differ: see view.
|
// Nothing here may assume the two differ: see view.
|
||||||
const maxRenderedResponseBytes = 4096
|
const maxRenderedResponseBytes = 4096
|
||||||
|
|
||||||
// deliveryResultColumns is the delivery attempt projection.
|
// deliveryResultColumns is the delivery attempt projection.
|
||||||
@@ -40,9 +39,8 @@ const deliveryResultColumns = "delivery_id, attempt_num, created_at, " +
|
|||||||
//
|
//
|
||||||
// Both Error and ResponseBody have been through the target's
|
// Both Error and ResponseBody have been through the target's
|
||||||
// Redactor. The engine already masks the URL out of the
|
// Redactor. The engine already masks the URL out of the
|
||||||
// errors it stores, so for errors this is a second line
|
// errors it stores, so for errors this is a second line; for
|
||||||
// covering rows written before it did; for response bodies it
|
// response bodies it is the only line, and its reach is what
|
||||||
// is the only line, and its reach is what
|
|
||||||
// delivery.Redactor documents.
|
// delivery.Redactor documents.
|
||||||
type DeliveryResultView struct {
|
type DeliveryResultView struct {
|
||||||
AttemptNum int
|
AttemptNum int
|
||||||
|
|||||||
@@ -441,10 +441,9 @@ func (s *Session) Regenerate(
|
|||||||
//
|
//
|
||||||
// Whichever comes first ends the session.
|
// Whichever comes first ends the session.
|
||||||
//
|
//
|
||||||
// A session that claims to be authenticated but carries no
|
// A session that claims to be authenticated but lacks the timestamp
|
||||||
// timestamps predates this check; it is treated as expired so the
|
// a deadline is measured from is treated as expired, so the user
|
||||||
// user re-authenticates rather than being granted an unbounded
|
// re-authenticates rather than being granted an unbounded session.
|
||||||
// session.
|
|
||||||
func (s *Session) expired(sess *sessions.Session) bool {
|
func (s *Session) expired(sess *sessions.Session) bool {
|
||||||
now := s.now()
|
now := s.now()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user