Skip to content

Commit 96b197e

Browse files
BertalanDawesomekling
authored andcommitted
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.
1 parent 1dce199 commit 96b197e

File tree

2 files changed

+25
-3
lines changed

2 files changed

+25
-3
lines changed

Userland/Libraries/LibJS/Runtime/Temporal/PlainTime.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -559,17 +559,17 @@ DaysAndTime round_time(u8 hour, u8 minute, u8 second, u16 millisecond, u16 micro
559559
day_length_ns = ns_per_day;
560560

561561
// b. Let quantity be (((((hour × 60 + minute) × 60 + second) × 1000 + millisecond) × 1000 + microsecond) × 1000 + nanosecond) / dayLengthNs.
562-
quantity = (((((hour * 60 + minute) * 60 + second) * 1000 + millisecond) * 1000 + microsecond) * 1000 + nanosecond) / *day_length_ns;
562+
quantity = (((((hour * 60.0 + minute) * 60.0 + second) * 1000.0 + millisecond) * 1000.0 + microsecond) * 1000.0 + nanosecond) / *day_length_ns;
563563
}
564564
// 4. Else if unit is "hour", then
565565
else if (unit == "hour"sv) {
566566
// a. Let quantity be (fractionalSecond / 60 + minute) / 60 + hour.
567-
quantity = (fractional_second / 60 + minute) / 60 + hour;
567+
quantity = (fractional_second / 60.0 + minute) / 60.0 + hour;
568568
}
569569
// 5. Else if unit is "minute", then
570570
else if (unit == "minute"sv) {
571571
// a. Let quantity be fractionalSecond / 60 + minute.
572-
quantity = fractional_second / 60 + minute;
572+
quantity = fractional_second / 60.0 + minute;
573573
}
574574
// 6. Else if unit is "second", then
575575
else if (unit == "second"sv) {

Userland/Libraries/LibJS/Tests/builtins/Temporal/PlainDateTime/PlainDateTime.prototype.round.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,28 @@ describe("correct behavior", () => {
8888
plainDateTime.round("minute").equals(plainDateTime.round({ smallestUnit: "minute" }))
8989
).toBeTrue();
9090
});
91+
92+
test("range boundary conditions", () => {
93+
// PlainDateTime can represent a point of time ±10**8 days from the epoch.
94+
const min = new Temporal.PlainDateTime(-271821, 4, 19, 0, 0, 0, 0, 0, 1);
95+
const max = new Temporal.PlainDateTime(275760, 9, 13, 23, 59, 59, 999, 999, 999);
96+
97+
["day", "hour", "minute", "second", "millisecond", "microsecond"].forEach(smallestUnit => {
98+
expect(() => {
99+
min.round({ smallestUnit, roundingMode: "floor" });
100+
}).toThrow(RangeError);
101+
expect(() => {
102+
min.round({ smallestUnit, roundingMode: "ceil" });
103+
}).not.toThrow();
104+
105+
expect(() => {
106+
max.round({ smallestUnit, roundingMode: "floor" });
107+
}).not.toThrow();
108+
expect(() => {
109+
max.round({ smallestUnit, roundingMode: "ceil" });
110+
}).toThrow(RangeError);
111+
});
112+
});
91113
});
92114

93115
describe("errors", () => {

0 commit comments

Comments
 (0)