Skip to content

fix: range-check Duration::round()'s relativeTo anchor for every spelling - #82

Merged
MidnightDesign merged 1 commit into
timezone-basic-offsetfrom
56-duration-round-bag-validation
Aug 17, 2026
Merged

fix: range-check Duration::round()'s relativeTo anchor for every spelling#82
MidnightDesign merged 1 commit into
timezone-basic-offsetfrom
56-duration-round-bag-validation

Conversation

@MidnightDesign

@MidnightDesign MidnightDesign commented Aug 17, 2026

Copy link
Copy Markdown
Owner

Problem

Duration::round() applied no relativeTo range check at all when the anchor was written as a property bag.

TC39's GetTemporalRelativeToOption accepts four spellings of an anchor — a ZonedDateTime, a PlainDate, an ISO string, a property bag — and reduces every one of them to a record holding either a [[ZonedRelativeTo]] or a [[PlainRelativeTo]]. That reduction happens before any range rule runs, so every rule downstream keys off which of the two objects it got, never off how the option was written.

Both guards in DurationRounding keyed off the spelling instead:

if (is_string($rtRaw)) { /* … the epoch bound for a string anchor … */ }
// … 150 lines later, after the balancing …
if ($rt instanceof ZonedDateTime) { /* … the epoch bound for an instance … */ }

A property bag matches neither, so it reached the arithmetic unbounded. Ordering was the other half of the same defect: the ZonedDateTime guard ran after the DST-aware day balancing it was meant to protect, so an instance in an IANA zone failed exactly like a bag did.

The result, across the anchor spellings of one and the same instant:

duration anchor master expected
seconds: 9007199254740991 bag, UTC returns a Duration RangeError
seconds: 9007199254740991 bag, America/Vancouver TypeError RangeError
seconds: 9007199254740991 ZonedDateTime, America/Vancouver TypeError RangeError
days: 104249991374 bag, UTC returns a Duration RangeError
days: 104249991374 bag, America/Vancouver DateMalformedStringException RangeError
seconds: 9007199254740991 string, [UTC] RangeError RangeError

The TypeError is int64 overflow of $timeOnlyNs surfacing as a float where an int parameter was declared — the crash reported in #56.

A bag also went unchecked on its own account. A string anchor is bounded while parsing, so '+275760-09-14' is rejected; the equivalent ['year' => 275760, 'month' => 9, 'day' => 14] anchored a round at a date that cannot exist.

The fix

RelativeTo::resolveAnchor() performs TC39's reduction once and returns an Internal\RelativeAnchor — a value object that knows whether it sits on the instant timeline or on the calendar, and has forgotten how it was spelled. The guards ask it three questions (targetOutOfRange(), dayBoundaryOutOfRange(), midnightOutOfRange()) and now run ahead of the balancing. 110 lines of duplicated, spelling-keyed guard collapse to 23.

resolveAnchor() deliberately does not delegate a zoned bag to ZonedDateTime::from(); two pre-existing divergences in that path would have come with it (see below). It reuses resolveZdt() for IANA zones and TimeZoneHelper for constant offsets.

Two spelling-keyed divergences fall out with it:

  • a bag's fields are constrained the way ToRelativeTemporalObject asks (month: 99 clamps rather than overflowing the Julian-day math);
  • the earliest representable date is rejected as an anchor for a non-blank duration in all three plain spellings, not only the string one.

Stacked on #89

The TimeZoneHelper drive-by that used to live here is now #89, the layer below this one in the stack. It is a hard prerequisite, not a nicety: TimeZoneHelper accepted an inline offset in a date-time-string zone identifier only when written with a colon, and the old relativeTo path got away with it by validating the string and then ignoring the zone it names. Once resolveAnchor() actually resolves that zone, both relativeto-propertybag-timezone-string-datetime fixtures fail without #89 — verified by reverting the one regex on this tree.

RelativeAnchor and RelativeTo::resolveAnchor() do not get a layer of their own, and cannot: Psalm runs at error level 1 with dead-code detection on, so landing them ahead of their caller fails the gate with five PossiblyUnusedMethod/PossiblyUnusedProperty errors. Introducing them in place without also moving the guards ahead of the balancing would land a knowingly mis-ordered guard, and the ordering is half the defect. So the reduction and its activation stay together.

Verification

The reference is V8 (node --harmony-temporal), cross-checked against test262 wherever the two disagree.

  • 90-case matrix (3 durations × 10 anchor spellings × 3 option sets) now matches V8 exactly. 24 cases diverged before.
  • Two cases where PHP and V8 still differ are ones where V8 fails test262 (relativeto-string-limits.js, next-day-out-of-range.js) and this implementation is correct.
  • Every assertion was run against both source states with a throwaway harness rather than a committed test — see below.
  • Full gate green: 12396 tests / 1032460 assertions / 299 incomplete (all three unchanged from master), PHPStan level 9 clean, Psalm clean, mago lint at the 271-issue baseline, format clean, Infection 100% MSI (574 mutations) on the gated files.

Where the regression test lives

The spec layer is covered by test262 and nothing else, so this PR adds no test of its own. Per #54 I checked the corpus first, and the gap is upstream's: the entire upstream Duration corpus (1093 tests, intl402 included) passes on the unfixed source. Not one fixture catches this. The round/ and intl402/round/ trees are byte-for-byte in sync with tc39/test262 main, and 0 of 247 transpiled round scripts are incomplete — so none of the three usual escape hatches applies.

Upstream tests these bounds hard, but only for the other spellings (relativeto-{plaindate,zoneddatetime}-large-time-component-out-of-range.js, relativeto-string-limits.js). It uses bag anchors in ~10 fixtures, but only for grammar/type rejection or as positive boundary cases with a blank duration. Bag anchor × out-of-range magnitude is the one empty cell.

That cell is now filed upstream as tc39/test262#5107, which fails on this branch's parent and passes on it. It lands here on the next composer test262:sync.

Until then the existing corpus already reaches almost all of the new code — RelativeAnchor is at 100% line and method coverage without any new test, and the four new RelativeTo methods are at 17/18, 8/9, 19/20 and 3/3. Three lines stay uncovered, and they are exactly the three out-of-range-bag rejections that only a bag-specific limits fixture can reach:

RelativeTo.php:179  resolveAnchor()   — bag denotes an unrepresentable date
RelativeTo.php:199  bagEpochDays()    — bag year outside the ISO limits
RelativeTo.php:244  zonedBagAnchor()  — bag instant outside the epoch range

#5107 covers the third. The first two need a plain-bag counterpart to relativeto-string-limits.js, which upstream also lacks — a second filing, not part of this PR.

Out of scope

Found while working on this, deliberately left alone:

  • Duration::total() has the same gap. Its bag branch calls only validatePropertyBag(), with no epoch-range check; resolveAnchor() is now available to close it. The total half of Temporal: Test Duration.prototype.{round,total} with zoned property bags -> out-of-epoch-range tc39/test262#5107 fails against this branch.
  • ZonedFields.php:225 rejects valid boundary instants from bags. It bounds the wall clock to ±MAX_EPOCH_SECONDS while ZonedParse.php:138 allows an extra day at the top, so ZonedDateTime::from(['year' => 275760, 'month' => 9, 'day' => 13, 'hour' => 23, 'minute' => 59, 'timeZone' => '+23:59']) throws where the identical string is accepted — and test262's ZonedDateTime/from/argument-string-limits.js says the string is right.
  • ZonedFields.php:421 rejects offset: '+00:45:00.000000000' that RelativeTo::validatePropertyBag() accepts.
  • RelativeTo::anchorYmd() reads $bag['year'] unconditionally, so an era/eraYear bag warns Undefined array key "year" and anchors at year 0. Pre-existing on the calendar path; this change makes it fire on the pure-time path too, with the same outcome. That is Duration::total() rejects era/eraYear relativeTo bags that pass validation #58's defect, so the era resolution belongs there.
  • A pre-existing sign flip, unchanged and identical on master: (new Duration(hours: 25, minutes: 30))->round(['smallestUnit' => 'hours', 'relativeTo' => '…[America/New_York]']) returns -PT26H. Consistent across all spellings; looks like the Duration::round() with IANA relativeTo returns negative result for large positive durations #55 family.

Closes #56

@MidnightDesign
MidnightDesign force-pushed the 56-duration-round-bag-validation branch from f317e36 to 3b636b4 Compare August 17, 2026 10:50
@MidnightDesign MidnightDesign changed the title Range-check Duration::round()'s relativeTo anchor for every spelling fix: range-check Duration::round()'s relativeTo anchor for every spelling Aug 17, 2026
@MidnightDesign
MidnightDesign force-pushed the 56-duration-round-bag-validation branch from 3b636b4 to 691a977 Compare August 17, 2026 12:23
…ling

Duration::round() applied no relativeTo range check at all when the anchor
was written as a property bag, and applied the ZonedDateTime one after the
DST-aware day balancing it was meant to protect.

TC39's GetTemporalRelativeToOption reduces all four spellings of an anchor
to a record holding either a [[ZonedRelativeTo]] or a [[PlainRelativeTo]]
before any range rule runs; every rule keys off which of the two it got,
never off how the option was written. Both guards keyed off the spelling
instead, so a bag matched neither and reached the arithmetic unbounded -
overflowing int64 and surfacing as the TypeError reported in #56.

RelativeTo::resolveAnchor() performs that reduction once and returns a
RelativeAnchor, which knows whether it sits on the instant timeline or on
the calendar and has forgotten the spelling. The guards ask it three
questions and now run ahead of the balancing. 110 lines of duplicated,
spelling-keyed guard collapse to 23.

Two spelling-keyed divergences fall out with it: a bag's fields are
constrained the way ToRelativeTemporalObject asks, and the earliest
representable date is rejected as an anchor for a non-blank duration in all
three plain spellings rather than only the string one.

Closes #56
@MidnightDesign
MidnightDesign force-pushed the 56-duration-round-bag-validation branch from 691a977 to 6876c37 Compare August 17, 2026 12:39
@MidnightDesign
MidnightDesign changed the base branch from master to timezone-basic-offset August 17, 2026 12:39
@MidnightDesign
MidnightDesign merged commit 9099d86 into master Aug 17, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Duration::round() property-bag relativeTo bypasses epoch-range validation

1 participant