fix: range-check Duration::round()'s relativeTo anchor for every spelling - #82
Merged
MidnightDesign merged 1 commit intoAug 17, 2026
Merged
Conversation
MidnightDesign
force-pushed
the
56-duration-round-bag-validation
branch
from
August 17, 2026 10:50
f317e36 to
3b636b4
Compare
MidnightDesign
force-pushed
the
56-duration-round-bag-validation
branch
from
August 17, 2026 12:23
3b636b4 to
691a977
Compare
…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
force-pushed
the
56-duration-round-bag-validation
branch
from
August 17, 2026 12:39
691a977 to
6876c37
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Duration::round()applied norelativeTorange check at all when the anchor was written as a property bag.TC39's
GetTemporalRelativeToOptionaccepts four spellings of an anchor — aZonedDateTime, aPlainDate, 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
DurationRoundingkeyed off the spelling instead:A property bag matches neither, so it reached the arithmetic unbounded. Ordering was the other half of the same defect: the
ZonedDateTimeguard 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:
masterseconds: 9007199254740991UTCDurationRangeErrorseconds: 9007199254740991America/VancouverTypeErrorRangeErrorseconds: 9007199254740991ZonedDateTime,America/VancouverTypeErrorRangeErrordays: 104249991374UTCDurationRangeErrordays: 104249991374America/VancouverDateMalformedStringExceptionRangeErrorseconds: 9007199254740991[UTC]RangeErrorRangeErrorThe
TypeErroris int64 overflow of$timeOnlyNssurfacing as a float where anintparameter 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 anInternal\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 toZonedDateTime::from(); two pre-existing divergences in that path would have come with it (see below). It reusesresolveZdt()for IANA zones andTimeZoneHelperfor constant offsets.Two spelling-keyed divergences fall out with it:
ToRelativeTemporalObjectasks (month: 99clamps rather than overflowing the Julian-day math);Stacked on #89
The
TimeZoneHelperdrive-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:TimeZoneHelperaccepted an inline offset in a date-time-string zone identifier only when written with a colon, and the oldrelativeTopath got away with it by validating the string and then ignoring the zone it names. OnceresolveAnchor()actually resolves that zone, bothrelativeto-propertybag-timezone-string-datetimefixtures fail without #89 — verified by reverting the one regex on this tree.RelativeAnchorandRelativeTo::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 fivePossiblyUnusedMethod/PossiblyUnusedPropertyerrors. 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.relativeto-string-limits.js,next-day-out-of-range.js) and this implementation is correct.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
Durationcorpus (1093 tests,intl402included) passes on the unfixed source. Not one fixture catches this. Theround/andintl402/round/trees are byte-for-byte in sync with tc39/test262main, 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 —
RelativeAnchoris at 100% line and method coverage without any new test, and the four newRelativeTomethods 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:#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 onlyvalidatePropertyBag(), with no epoch-range check;resolveAnchor()is now available to close it. Thetotalhalf of Temporal: Test Duration.prototype.{round,total} with zoned property bags -> out-of-epoch-range tc39/test262#5107 fails against this branch.ZonedFields.php:225rejects valid boundary instants from bags. It bounds the wall clock to ±MAX_EPOCH_SECONDSwhileZonedParse.php:138allows an extra day at the top, soZonedDateTime::from(['year' => 275760, 'month' => 9, 'day' => 13, 'hour' => 23, 'minute' => 59, 'timeZone' => '+23:59'])throws where the identical string is accepted — and test262'sZonedDateTime/from/argument-string-limits.jssays the string is right.ZonedFields.php:421rejectsoffset: '+00:45:00.000000000'thatRelativeTo::validatePropertyBag()accepts.RelativeTo::anchorYmd()reads$bag['year']unconditionally, so anera/eraYearbag warnsUndefined 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.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