From 7a2ea4c991170e3e808ddd2ed70a17235d90e411 Mon Sep 17 00:00:00 2001 From: Philip Chimento Date: Tue, 20 Jun 2023 12:37:05 +0200 Subject: [PATCH] Normative: Prevent indefinite loops in NormalizedTimeDurationToDays It's possible to make at least the second loop continue indefinitely with a contrived calendar and time zone. DRAFT: Still to be determined if this precludes any non-contrived use cases. If so, we will keep the loops, but still put an upper limit on the number of iterations. Includes a few more tests in the NYSE time zone cookbook example to make sure that a time zone transition of >24h continues to work. --- docs/cookbook/stockExchangeTimeZone.mjs | 25 +++++++++++++- polyfill/lib/ecmascript.mjs | 45 +++++++++++++++---------- spec/zoneddatetime.html | 31 +++++++++-------- 3 files changed, 66 insertions(+), 35 deletions(-) diff --git a/docs/cookbook/stockExchangeTimeZone.mjs b/docs/cookbook/stockExchangeTimeZone.mjs index b8cbb77a45..024962a6cf 100644 --- a/docs/cookbook/stockExchangeTimeZone.mjs +++ b/docs/cookbook/stockExchangeTimeZone.mjs @@ -222,7 +222,30 @@ assert.equal(monday.hoursInDay, 24); const friday = monday.add({ days: 4 }); assert.equal(friday.hoursInDay, 72); -// Adding 1 day to Friday gets you the next Monday +// Adding 1 day to Friday gets you the next Monday (disambiguates forward) assert.equal(friday.add({ days: 1 }).toString(), '2022-08-29T09:30:00-04:00[NYSE]'); // Adding 3 days to Friday also gets you the next Monday assert.equal(friday.add({ days: 3 }).toString(), '2022-08-29T09:30:00-04:00[NYSE]'); + +const nextMonday = monday.add({ weeks: 1 }); + +// Subtracting 1 day from Monday gets you the same day (disambiguates forward) +assert.equal(nextMonday.subtract({ days: 1 }).toString(), '2022-08-29T09:30:00-04:00[NYSE]'); +// Subtracting 3 days from Monday gets you the previous Friday +assert.equal(nextMonday.subtract({ days: 3 }).toString(), '2022-08-26T09:30:00-04:00[NYSE]'); + +// Difference between Friday and Monday is 72 hours or 3 days +const fridayUntilMonday = friday.until(nextMonday); +assert.equal(fridayUntilMonday.toString(), 'PT72H'); +assert.equal(fridayUntilMonday.total('hours'), 72); +assert.equal(fridayUntilMonday.total('days'), 3); + +const mondaySinceFriday = nextMonday.since(friday); +assert.equal(mondaySinceFriday.toString(), 'PT72H'); +assert.equal(mondaySinceFriday.total('hours'), 72); +assert.equal(mondaySinceFriday.total('days'), 3); + +// One week is still 7 days +const oneWeek = Temporal.Duration.from({ weeks: 1 }); +assert.equal(oneWeek.total({ unit: 'days', relativeTo: monday }), 7); +assert.equal(oneWeek.total({ unit: 'days', relativeTo: friday }), 7); diff --git a/polyfill/lib/ecmascript.mjs b/polyfill/lib/ecmascript.mjs index 6f05443dea..5ce7d9e9e6 100644 --- a/polyfill/lib/ecmascript.mjs +++ b/polyfill/lib/ecmascript.mjs @@ -3255,20 +3255,34 @@ export function NormalizedTimeDurationToDays(norm, zonedRelativeTo, timeZoneRec, // back inside the period where it belongs. Note that this case only can // happen for positive durations because the only direction that // `disambiguation: 'compatible'` can change clock time is forwards. - if (sign === 1) { - while (days > 0 && relativeResult.epochNs.greater(endNs)) { - days--; - relativeResult = AddDaysToZonedDateTime(start, dtStart, timeZoneRec, calendar, days); - // may do disambiguation + if (sign === 1 && days > 0 && relativeResult.epochNs.greater(endNs)) { + days--; + relativeResult = AddDaysToZonedDateTime(start, dtStart, timeZoneRec, calendar, days); + // may do disambiguation + if (days > 0 && relativeResult.epochNs.greater(endNs)) { + throw new RangeError('inconsistent result from custom time zone getInstantFor()'); } } norm = TimeDuration.fromEpochNsDiff(endNs, relativeResult.epochNs); - let isOverflow = false; - let dayLengthNs; - do { - // calculate length of the next day (day that contains the time remainder) - const oneDayFarther = AddDaysToZonedDateTime( + // calculate length of the next day (day that contains the time remainder) + let oneDayFarther = AddDaysToZonedDateTime( + relativeResult.instant, + relativeResult.dateTime, + timeZoneRec, + calendar, + sign + ); + let dayLengthNs = TimeDuration.fromEpochNsDiff(oneDayFarther.epochNs, relativeResult.epochNs); + const oneDayLess = norm.subtract(dayLengthNs); + let isOverflow = oneDayLess.sign() * sign >= 0; + if (isOverflow) { + norm = oneDayLess; + relativeResult = oneDayFarther; + days += sign; + + // ensure there was no more overflow + oneDayFarther = AddDaysToZonedDateTime( relativeResult.instant, relativeResult.dateTime, timeZoneRec, @@ -3277,14 +3291,9 @@ export function NormalizedTimeDurationToDays(norm, zonedRelativeTo, timeZoneRec, ); dayLengthNs = TimeDuration.fromEpochNsDiff(oneDayFarther.epochNs, relativeResult.epochNs); - const oneDayLess = norm.subtract(dayLengthNs); - isOverflow = oneDayLess.sign() * sign >= 0; - if (isOverflow) { - norm = oneDayLess; - relativeResult = oneDayFarther; - days += sign; - } - } while (isOverflow); + isOverflow = norm.subtract(dayLengthNs).sign() * sign >= 0; + if (isOverflow) throw new RangeError('inconsistent result from custom time zone getPossibleInstantsFor()'); + } if (days !== 0 && MathSign(days) != sign) { throw new RangeError('Time zone or calendar converted nanoseconds into a number of days with the opposite sign'); } diff --git a/spec/zoneddatetime.html b/spec/zoneddatetime.html index eb56b8b2d7..6bb991f227 100644 --- a/spec/zoneddatetime.html +++ b/spec/zoneddatetime.html @@ -1446,23 +1446,22 @@

1. Else if _days_ < 0 and _timeSign_ < 0, then 1. Set _days_ to _days_ + 1. 1. Let _relativeResult_ be ? AddDaysToZonedDateTime(_startInstant_, _startDateTime_, _timeZoneRec_, _zonedRelativeTo_.[[Calendar]], _days_). - 1. If _sign_ is 1, then - 1. Repeat, while _days_ > 0 and ℝ(_relativeResult_.[[EpochNanoseconds]]) > _endNs_, - 1. Set _days_ to _days_ - 1. - 1. Set _relativeResult_ to ? AddDaysToZonedDateTime(_startInstant_, _startDateTime_, _timeZoneRec_, _zonedRelativeTo_.[[Calendar]], _days_). + 1. If _sign_ = 1, and _days_ > 0, and ℝ(_relativeResult_.[[EpochNanoseconds]]) > _endNs_, then + 1. Set _days_ to _days_ - 1. + 1. Set _relativeResult_ to ? AddDaysToZonedDateTime(_startInstant_, _startDateTime_, _timeZoneRec_, _zonedRelativeTo_.[[Calendar]], _days_). + 1. If _days_ > 0 and ℝ(_relativeResult_.[[EpochNanoseconds]]) > _endNs_, throw a *RangeError* exception. 1. Set _norm_ to NormalizedTimeDurationFromEpochNanosecondsDifference(_endNs_, _relativeResult_.[[EpochNanoseconds]]). - 1. Let _done_ be *false*. - 1. Let _dayLengthNs_ be ~unset~. - 1. Repeat, while _done_ is *false*, - 1. Let _oneDayFarther_ be ? AddDaysToZonedDateTime(_relativeResult_.[[Instant]], _relativeResult_.[[DateTime]], _timeZoneRec_, _zonedRelativeTo_.[[Calendar]], _sign_). - 1. Set _dayLengthNs_ to NormalizedTimeDurationFromEpochNanosecondsDifference(_oneDayFarther_.[[EpochNanoseconds]], _relativeResult_.[[EpochNanoseconds]]). - 1. Let _oneDayLess_ be ? SubtractNormalizedTimeDuration(_norm_, _dayLengthNs_). - 1. If NormalizedTimeDurationSign(_oneDayLess_) × _sign_ ≥ 0, then - 1. Set _norm_ to _oneDayLess_. - 1. Set _relativeResult_ to _oneDayFarther_. - 1. Set _days_ to _days_ + _sign_. - 1. Else, - 1. Set _done_ to *true*. + 1. Let _oneDayFarther_ be ? AddDaysToZonedDateTime(_relativeResult_.[[Instant]], _relativeResult_.[[DateTime]], _timeZoneRec_, _zonedRelativeTo_.[[Calendar]], _sign_). + 1. Let _dayLengthNs_ be NormalizedTimeDurationFromEpochNanosecondsDifference(_oneDayFarther.[[EpochNanoseconds]], _relativeResult_.[[EpochNanoseconds]]). + 1. Let _oneDayLess_ be ? SubtractNormalizedTimeDuration(_norm_, _dayLengthNs_). + 1. If NormalizedTimeDurationSign(_oneDayLess_) × _sign_ ≥ 0, then + 1. Set _norm_ to _oneDayLess_. + 1. Set _relativeResult_ to _oneDayFarther_. + 1. Set _days_ to _days_ + _sign_. + 1. Set _oneDayFarther_ to ? AddDaysToZonedDateTime(_relativeResult_.[[Instant]], _relativeResult_.[[DateTime]], _timeZoneRec_, _zonedRelativeTo_.[[Calendar]], _sign_). + 1. Set _dayLengthNs_ to NormalizedTimeDurationFromEpochNanosecondsDifference(_oneDayFarther.[[EpochNanoseconds]], _relativeResult_.[[EpochNanoseconds]]). + 1. If NormalizedTimeDurationSign(? SubtractNormalizedTimeDuration(_norm_, _dayLengthNs_)) × _sign_ ≥ 0, then + 1. Throw a *RangeError* exception. 1. If _days_ < 0 and _sign_ = 1, throw a *RangeError* exception. 1. If _days_ > 0 and _sign_ = -1, throw a *RangeError* exception. 1. If NormalizedTimeDurationSign(_norm_) = -1, then