From 6dfa6993e579f1006fedd6308b288fd4b55949fb Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Thu, 5 Dec 2024 11:38:33 -0500 Subject: [PATCH] LibJS: Add workaround for invalid ISODateTimes in DifferenceISODateTime The assertions can be hit when Temporal.Duration.prototype.round / total are provided a PlainDate at the very limits of valid date-times. Tests were recently added to test262 which trip these assertions, thus we are now crashing in those tests. Let's throw a RangeError instead, as this is the behavior expected by the tests. --- Libraries/LibJS/Runtime/Temporal/PlainDateTime.cpp | 13 +++++++------ Libraries/LibJS/Runtime/Temporal/PlainDateTime.h | 2 +- .../Temporal/Duration/Duration.prototype.round.js | 8 ++++++++ .../Temporal/Duration/Duration.prototype.total.js | 8 ++++++++ 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/Libraries/LibJS/Runtime/Temporal/PlainDateTime.cpp b/Libraries/LibJS/Runtime/Temporal/PlainDateTime.cpp index d23def96084..6cdaf8ef018 100644 --- a/Libraries/LibJS/Runtime/Temporal/PlainDateTime.cpp +++ b/Libraries/LibJS/Runtime/Temporal/PlainDateTime.cpp @@ -286,13 +286,14 @@ ISODateTime round_iso_date_time(ISODateTime const& iso_date_time, u64 increment, } // 5.5.12 DifferenceISODateTime ( isoDateTime1, isoDateTime2, calendar, largestUnit ), https://tc39.es/proposal-temporal/#sec-temporal-differenceisodatetime -InternalDuration difference_iso_date_time(VM& vm, ISODateTime const& iso_date_time1, ISODateTime const& iso_date_time2, StringView calendar, Unit largest_unit) +ThrowCompletionOr difference_iso_date_time(VM& vm, ISODateTime const& iso_date_time1, ISODateTime const& iso_date_time2, StringView calendar, Unit largest_unit) { // 1. Assert: ISODateTimeWithinLimits(isoDateTime1) is true. - VERIFY(iso_date_time_within_limits(iso_date_time1)); - // 2. Assert: ISODateTimeWithinLimits(isoDateTime2) is true. - VERIFY(iso_date_time_within_limits(iso_date_time2)); + // FIXME: Spec issue: Rather than asserting, throw a RangeError. See: + // https://github.com/tc39/proposal-temporal/issues/3015 + if (!iso_date_time_within_limits(iso_date_time1) || !iso_date_time_within_limits(iso_date_time2)) + return vm.throw_completion(ErrorType::TemporalInvalidISODateTime); // 3. Let timeDuration be DifferenceTime(isoDateTime1.[[Time]], isoDateTime2.[[Time]]). auto time_duration = difference_time(iso_date_time1.time, iso_date_time2.time); @@ -344,7 +345,7 @@ ThrowCompletionOr difference_plain_date_time_with_rounding(VM& } // 2. Let diff be DifferenceISODateTime(isoDateTime1, isoDateTime2, calendar, largestUnit). - auto diff = difference_iso_date_time(vm, iso_date_time1, iso_date_time2, calendar, largest_unit); + auto diff = TRY(difference_iso_date_time(vm, iso_date_time1, iso_date_time2, calendar, largest_unit)); // 3. If smallestUnit is NANOSECOND and roundingIncrement = 1, return diff. if (smallest_unit == Unit::Nanosecond && rounding_increment == 1) @@ -367,7 +368,7 @@ ThrowCompletionOr difference_plain_date_time_with_total(VM& } // 2. Let diff be DifferenceISODateTime(isoDateTime1, isoDateTime2, calendar, unit). - auto diff = difference_iso_date_time(vm, iso_date_time1, iso_date_time2, calendar, unit); + auto diff = TRY(difference_iso_date_time(vm, iso_date_time1, iso_date_time2, calendar, unit)); // 3. If unit is NANOSECOND, return diff.[[Time]]. if (unit == Unit::Nanosecond) diff --git a/Libraries/LibJS/Runtime/Temporal/PlainDateTime.h b/Libraries/LibJS/Runtime/Temporal/PlainDateTime.h index 9b3f769118c..b7cc2e8481e 100644 --- a/Libraries/LibJS/Runtime/Temporal/PlainDateTime.h +++ b/Libraries/LibJS/Runtime/Temporal/PlainDateTime.h @@ -41,7 +41,7 @@ ThrowCompletionOr> create_temporal_date_time(VM&, ISODate String iso_date_time_to_string(ISODateTime const&, StringView calendar, SecondsStringPrecision::Precision, ShowCalendar); i8 compare_iso_date_time(ISODateTime const&, ISODateTime const&); ISODateTime round_iso_date_time(ISODateTime const&, u64 increment, Unit, RoundingMode); -InternalDuration difference_iso_date_time(VM&, ISODateTime const&, ISODateTime const&, StringView calendar, Unit largest_unit); +ThrowCompletionOr difference_iso_date_time(VM&, ISODateTime const&, ISODateTime const&, StringView calendar, Unit largest_unit); ThrowCompletionOr difference_plain_date_time_with_rounding(VM&, ISODateTime const&, ISODateTime const&, StringView calendar, Unit largest_unit, u64 rounding_increment, Unit smallest_unit, RoundingMode); ThrowCompletionOr difference_plain_date_time_with_total(VM&, ISODateTime const&, ISODateTime const&, StringView calendar, Unit); ThrowCompletionOr> difference_temporal_plain_date_time(VM&, DurationOperation, PlainDateTime const&, Value other, Value options); diff --git a/Libraries/LibJS/Tests/builtins/Temporal/Duration/Duration.prototype.round.js b/Libraries/LibJS/Tests/builtins/Temporal/Duration/Duration.prototype.round.js index b7523bafcf0..54777f912ab 100644 --- a/Libraries/LibJS/Tests/builtins/Temporal/Duration/Duration.prototype.round.js +++ b/Libraries/LibJS/Tests/builtins/Temporal/Duration/Duration.prototype.round.js @@ -185,6 +185,14 @@ describe("errors", () => { }).toThrowWithMessage(RangeError, "Largest unit must not be year"); }); + test("relativeTo with invalid date", () => { + const duration = new Temporal.Duration(0, 0, 0, 31); + + expect(() => { + duration.round({ smallestUnit: "minutes", relativeTo: "-271821-04-19" }); + }).toThrowWithMessage(RangeError, "Invalid ISO date time"); + }); + // Spec Issue: https://github.com/tc39/proposal-temporal/issues/2124 // Spec Fix: https://github.com/tc39/proposal-temporal/commit/66f7464aaec64d3cd21fb2ec37f6502743b9a730 test("balancing calendar units with largestUnit set to 'year' and relativeTo unset throws instead of crashing", () => { diff --git a/Libraries/LibJS/Tests/builtins/Temporal/Duration/Duration.prototype.total.js b/Libraries/LibJS/Tests/builtins/Temporal/Duration/Duration.prototype.total.js index 2190d40bede..32ed14c9e1a 100644 --- a/Libraries/LibJS/Tests/builtins/Temporal/Duration/Duration.prototype.total.js +++ b/Libraries/LibJS/Tests/builtins/Temporal/Duration/Duration.prototype.total.js @@ -85,4 +85,12 @@ describe("errors", () => { duration.total({ unit: "second" }); }).toThrowWithMessage(RangeError, "Largest unit must not be year"); }); + + test("relativeTo with invalid date", () => { + const duration = new Temporal.Duration(0, 0, 0, 31); + + expect(() => { + duration.total({ unit: "minute", relativeTo: "-271821-04-19" }); + }).toThrowWithMessage(RangeError, "Invalid ISO date time"); + }); });