From 96b197ef46105b8b7b0bce6823fcd190860bed21 Mon Sep 17 00:00:00 2001 From: Daniel Bertalan Date: Thu, 29 Jun 2023 20:48:52 +0200 Subject: [PATCH] LibJS/Temporal: Perform floating point arithmetic in RoundTime The valid range for temporal values (`nsMinInstant`/`nsMaxInstant`) means performing nanosecond-valued integers could lead to an overflow. NB: Only the `roundingMode: "day"` case was affected, as all others were already performing the division on floating-point `fractional_second` values. I'm adding `.0` suffixes everywhere to make this fact clearer. This adds a few local tests as well, as those are tested with sanitizers enabled by default, unlike test262. --- .../LibJS/Runtime/Temporal/PlainTime.cpp | 6 ++--- .../PlainDateTime.prototype.round.js | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/Temporal/PlainTime.cpp b/Userland/Libraries/LibJS/Runtime/Temporal/PlainTime.cpp index 66800821a4b..52f8dbef304 100644 --- a/Userland/Libraries/LibJS/Runtime/Temporal/PlainTime.cpp +++ b/Userland/Libraries/LibJS/Runtime/Temporal/PlainTime.cpp @@ -559,17 +559,17 @@ DaysAndTime round_time(u8 hour, u8 minute, u8 second, u16 millisecond, u16 micro day_length_ns = ns_per_day; // b. Let quantity be (((((hour × 60 + minute) × 60 + second) × 1000 + millisecond) × 1000 + microsecond) × 1000 + nanosecond) / dayLengthNs. - quantity = (((((hour * 60 + minute) * 60 + second) * 1000 + millisecond) * 1000 + microsecond) * 1000 + nanosecond) / *day_length_ns; + quantity = (((((hour * 60.0 + minute) * 60.0 + second) * 1000.0 + millisecond) * 1000.0 + microsecond) * 1000.0 + nanosecond) / *day_length_ns; } // 4. Else if unit is "hour", then else if (unit == "hour"sv) { // a. Let quantity be (fractionalSecond / 60 + minute) / 60 + hour. - quantity = (fractional_second / 60 + minute) / 60 + hour; + quantity = (fractional_second / 60.0 + minute) / 60.0 + hour; } // 5. Else if unit is "minute", then else if (unit == "minute"sv) { // a. Let quantity be fractionalSecond / 60 + minute. - quantity = fractional_second / 60 + minute; + quantity = fractional_second / 60.0 + minute; } // 6. Else if unit is "second", then else if (unit == "second"sv) { diff --git a/Userland/Libraries/LibJS/Tests/builtins/Temporal/PlainDateTime/PlainDateTime.prototype.round.js b/Userland/Libraries/LibJS/Tests/builtins/Temporal/PlainDateTime/PlainDateTime.prototype.round.js index e525cda5254..8c83877ae2e 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/Temporal/PlainDateTime/PlainDateTime.prototype.round.js +++ b/Userland/Libraries/LibJS/Tests/builtins/Temporal/PlainDateTime/PlainDateTime.prototype.round.js @@ -88,6 +88,28 @@ describe("correct behavior", () => { plainDateTime.round("minute").equals(plainDateTime.round({ smallestUnit: "minute" })) ).toBeTrue(); }); + + test("range boundary conditions", () => { + // PlainDateTime can represent a point of time ±10**8 days from the epoch. + const min = new Temporal.PlainDateTime(-271821, 4, 19, 0, 0, 0, 0, 0, 1); + const max = new Temporal.PlainDateTime(275760, 9, 13, 23, 59, 59, 999, 999, 999); + + ["day", "hour", "minute", "second", "millisecond", "microsecond"].forEach(smallestUnit => { + expect(() => { + min.round({ smallestUnit, roundingMode: "floor" }); + }).toThrow(RangeError); + expect(() => { + min.round({ smallestUnit, roundingMode: "ceil" }); + }).not.toThrow(); + + expect(() => { + max.round({ smallestUnit, roundingMode: "floor" }); + }).not.toThrow(); + expect(() => { + max.round({ smallestUnit, roundingMode: "ceil" }); + }).toThrow(RangeError); + }); + }); }); describe("errors", () => {