Fix signed overflow in Datetime.add-seconds for large offsets

[carpentry-agent]

The bug

Datetime.add-seconds computed (+ s n) in its non-negative branch, where s is the datetime's seconds field (0–59) and n is the caller's offset. For n near Int.MAX that sum wraps negative, (< a 60) then holds, and the function returns a Datetime with a negative seconds field and the date untouched:

(Datetime.add-seconds &(dt 2020 1 1 23 59 59) 2147483647)
;=> 2020-01-01 23:59:-10        ; expected 2088-01-20 03:14:06

The same base with a zero seconds field does not overflow (0 + 2147483647 is exactly Int.MAX), which is why every existing test missed it — they all start from 00:00:00.

This has a live downstream consequence: http's cookie max-age-expiry (http.carp:107) is (Datetime.add-seconds &(Datetime.now) i) with no guard, so a large Max-Age yields a structurally invalid expiry instead of the far-future one RFC 6265 asks for.

The fix

Split n into whole days and a sub-day remainder before touching the seconds field:

• days (/ n DAY), and the seconds addition becomes (+ s (Int.mod n DAY)), bounded by 59 + 86399 = 86458 — it can no longer overflow for any n.
• days is folded into the Gregorian ordinal alongside the carry the innermost branch already computes. The largest possible days is Int.MAX / 86400 = 24855 (plus at most 1 from the hour carry), and to-ordinal/from-ordinal are already the mechanism that branch uses, so an ordinal shifted by ≤ 24856 days stays far inside Int.

No clamping, and no widening to Long — the day/remainder split keeps everything inside Int by construction, which also matters because Long is 32-bit on some targets.

The three early-exit conditions gain a (= days 0) conjunct. Without it a datetime whose seconds/minutes/hours don't carry would keep its old date while days was silently dropped; with it, the branch taken for any given input is exactly the one the old code took, so the Maybe-ness of the hours and minutes fields is preserved bit-for-bit (Datetime.= compares those, so this is observable).

The negative branch: no change needed

Asked to check rather than assume. (+ s n) cannot overflow there, because that branch only runs when n < 0 while s is in [0, 59] — adding a non-negative value to a negative one moves it toward zero. The worst case is n = Int.MIN, s = 59, giving -2147483589, comfortably representable. Downstream, div-/mod- route through Double (which represents every Int exactly), ma ≥ -35791395, ha ≥ -596524, and the ordinal shift is ≥ -24856. So the branch is left alone.

(Going far enough back does eventually walk from-ordinal off the bottom of the Gregorian ordinal — 1-01-01 minus a day aborts in Array.unsafe-nth. That's a pre-existing out-of-range concern in from-ordinal, not an overflow, and it's out of scope here.)

Differential: behaviour is otherwise unchanged

Ran the old and new implementations side by side (old copied verbatim into a scratch module) over 27 bases × 60 non-overflowing offsets, plus 3 ordinal-floor bases restricted to non-negative offsets:

• bases: seconds/minutes/hours at their boundaries (23:59:59, 00:59:59, 23:59:00, 23:00:00), leap days (2024-02-29, 2000-02-29), non-leap Feb ends (2023, 1900, 2100), month ends (Jan 31, Apr 30, Dec 31), year ends (1999, 2024), the epoch, the 2038 cliff, year 1, year 9000, date-only datetimes with all-Nothing time fields, and datetimes with a Nothing hours or minutes field
• offsets: 0, ±1s through ±10y, exact unit boundaries (59/60/61, 3599/3600/3601, 86399/86400/86401), and large-but-safe values up to 2147483588 (the largest offset that cannot overflow for any s) and down to Int.MIN

Result: 1803 cases compared, 0 disagreements.

The harness is not vacuous — re-running it with overflowing offsets (2147483647, 2147483600) added produces 35 mismatches, every one of them a case where the old code returns a negative seconds field.

Separately, the new results were cross-checked against Python's datetime for 11 large-offset cases (including Int.MAX and Int.MIN); all 11 agree exactly.

Tests

Four cases added to test/time.carp: Int.MAX and Int.MIN offsets applied to a 23:59:59 base, and two large-but-not-extreme offsets (2000000000, 1000000000) crossing year and month ends.

carp -x test/time.carp: 244 passed, 0 failed (was 240). Reverting time.carp alone drops it to 243/1 — only the Int.MAX test fails, confirming the other three pin behaviour the old code already had.

Also ran CI's -Werror variant, carp-fmt -c, and angler — all clean.

No CHANGELOG entry: this repo has no changelog file, and adding one for a single bugfix seemed like the wrong call.

---

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

[carpentry-reviewer]

Build & Tests

Checked out claude/fix-add-seconds-overflow at b3cc30c. Branch's merge-base is origin/master exactly, so nothing here is being evaluated against a stale tree.

carp -x test/time.carp → 244 passed, 0 failed on armhf, matching the PR. CI green on ubuntu + macOS.

Findings

None blocking. I tried to break this from four directions and it held up.

Independent differential against Python's datetime (the PR cross-checked 11 cases; I ran 630). 18 bases × 35 offsets — unit boundaries, leap days, non-leap Feb ends, month/year ends, the epoch, the 2038 cliff, year 9000, year 9999, and bases with Nothing hours/minutes — against datetime + timedelta:

new code:  checked=630  ok=607  mismatch=0   py-out-of-range=23
old code:  checked=630  ok=571  mismatch=36  py-out-of-range=23

All 36 old-code mismatches sit at offsets ≥ 2147483589 with a non-zero seconds field, i.e. exactly the reported bug. The harness is therefore not vacuous, and every case where old and new disagree (39 total) is one the new code gets right — the remaining 3 are the year-9999 base, where Python can't represent the answer and the old code returned 9999-12-31 23:59:-10.

The branch-equivalence claim checks out. The argument for (= days 0) is that every input takes the branch the old code took; I confirmed it holds rather than taking it on faith. For days = 0 we have n < DAY, so a_new = a_old and the guards are literally the same test; for days ≠ 0 we have a_old = s + n ≥ 86400, so the old code descended too. Empirically, a 184-case probe printing the whole struct (8 Maybe shapes — including all-Nothing and Nothing-hours-with-Just-minutes — × 23 offsets straddling every carry boundary) differs from master in 4 of 184 cases, and all 4 are the Int.MAX overflow. Every Nothing field is preserved bit-for-bit everywhere else.

Bounds re-derived independently: a ≤ 59 + 86399, ma ≤ 1499, ha ≤ 47, days ≤ 24855, so the ordinal shift is ≤ 24856 — no path back to overflow. Int.mod// truncation is safe here because the branch is guarded by (neg? n), so n ≥ 0 and truncation is floor.

No new crash path. Pushing the ordinal past the Gregorian range was my main worry, since a bad index into Array.unsafe-nth aborts. from-ordinal turns out to be pure arithmetic with the month index bounded by construction, so year 9999 + Int.MAX seconds yields 10068-01-19 03:14:06 rather than crashing. The one abort I did find — year-1 bases with large negative offsets walking off the bottom of the ordinal — reproduces identically on master (same Array_unsafe_nth assertion), so it is pre-existing and correctly called out as out of scope in the PR body.

Test expectations verified independently — all four new expected strings match Python exactly.

Two pre-existing notes, neither for this PR: Duration.sub (time.carp:1153) does (neg @(seconds- delta)), which is a no-op at Int.MIN; and Duration.days/weeks multiply before reaching add-seconds, so they can overflow ahead of it. The other add-seconds call sites (to-utc, in-timezone) only ever pass timezone deltas, so they were never exposed. Duration.add is fixed for free by this change.

No changelog in this repo and none added — right call.

Verdict: merge

The fix is minimal, the day/remainder split provably closes the overflow without widening to Long (which would be wrong on 32-bit-Long targets anyway), and 630 independent cases against Python show zero behavioural change outside the bug.