Skip to content
Permalink
Browse files
[JSC] Fractional part of an ISO8601 Duration string must not be inter…
…preted as floating-point

https://bugs.webkit.org/show_bug.cgi?id=241940

Reviewed by Yusuke Suzuki.

The last time part of an ISO8601 Duration string is allowed to have 1 to 9 digits beyond the decimal point.
If this fractional part is interpreted using floating-point arithmetic, off-by-one errors can occur.
(https://tc39.es/proposal-temporal/#sec-temporal-parsetemporaldurationstring)

Contrary to the spec, test262 tests had been expecting IEEE-754-aligned results.
Tests were corrected in tc39/test262#3544; this patch corrects our implementation.

* JSTests/test262/expectations.yaml:
Mark 16 test cases as passing.

* Source/JavaScriptCore/runtime/ISO8601.cpp:
(JSC::ISO8601::handleFraction):

Canonical link: https://commits.webkit.org/251809@main
  • Loading branch information
rkirsling committed Jun 23, 2022
1 parent 1b9d289 commit 92e204d23f40144bdb3908c60f2d4fb66e5f6ae9
Showing 2 changed files with 19 additions and 55 deletions.
@@ -831,9 +831,6 @@ test/built-ins/ShadowRealm/prototype/evaluate/throws-typeerror-wrap-throwing.js:
test/built-ins/ShadowRealm/prototype/importValue/throws-if-exportname-not-string.js:
default: 'Test262Error: Expected a TypeError but got a Test262Error'
strict mode: 'Test262Error: Expected a TypeError but got a Test262Error'
test/built-ins/Temporal/Duration/compare/argument-string-negative-fractional-units.js:
default: 'Test262Error: negative fractional hours (first argument) Expected SameValue(«1», «0») to be true'
strict mode: 'Test262Error: negative fractional hours (first argument) Expected SameValue(«1», «0») to be true'
test/built-ins/Temporal/Duration/compare/basic.js:
default: 'RangeError: Cannot compare a duration of years, months, or weeks without a relativeTo option'
strict mode: 'RangeError: Cannot compare a duration of years, months, or weeks without a relativeTo option'
@@ -879,12 +876,6 @@ test/built-ins/Temporal/Duration/compare/timezone-wrong-type.js:
test/built-ins/Temporal/Duration/compare/twenty-five-hour-day.js:
default: 'TypeError: Right side of assignment cannot be destructured'
strict mode: 'TypeError: Right side of assignment cannot be destructured'
test/built-ins/Temporal/Duration/from/argument-string-negative-fractional-units.js:
default: 'Test262Error: negative fractional hours nanoseconds result Expected SameValue(«-799», «-800») to be true'
strict mode: 'Test262Error: negative fractional hours nanoseconds result Expected SameValue(«-799», «-800») to be true'
test/built-ins/Temporal/Duration/prototype/add/argument-string-negative-fractional-units.js:
default: 'Test262Error: negative fractional hours nanoseconds result Expected SameValue(«-799», «-800») to be true'
strict mode: 'Test262Error: negative fractional hours nanoseconds result Expected SameValue(«-799», «-800») to be true'
test/built-ins/Temporal/Duration/prototype/add/calendar-dateadd-called-with-plaindate-instance.js:
default: 'RangeError: Cannot add a duration of years, months, or weeks without a relativeTo option'
strict mode: 'RangeError: Cannot add a duration of years, months, or weeks without a relativeTo option'
@@ -978,9 +969,6 @@ test/built-ins/Temporal/Duration/prototype/round/timezone-wrong-type.js:
test/built-ins/Temporal/Duration/prototype/round/year-zero.js:
default: 'Test262Error: reject minus zero as extended year Expected a RangeError but got a Error'
strict mode: 'Test262Error: reject minus zero as extended year Expected a RangeError but got a Error'
test/built-ins/Temporal/Duration/prototype/subtract/argument-string-negative-fractional-units.js:
default: 'Test262Error: negative fractional hours nanoseconds result Expected SameValue(«799», «800») to be true'
strict mode: 'Test262Error: negative fractional hours nanoseconds result Expected SameValue(«799», «800») to be true'
test/built-ins/Temporal/Duration/prototype/subtract/calendar-dateadd-called-with-plaindate-instance.js:
default: 'RangeError: Cannot subtract a duration of years, months, or weeks without a relativeTo option'
strict mode: 'RangeError: Cannot subtract a duration of years, months, or weeks without a relativeTo option'
@@ -1065,15 +1053,9 @@ test/built-ins/Temporal/Duration/prototype/total/timezone-wrong-type.js:
test/built-ins/Temporal/Duration/prototype/total/unit-plurals-accepted-string.js:
default: 'TypeError: Right hand side of instanceof is not an object'
strict mode: 'TypeError: Right hand side of instanceof is not an object'
test/built-ins/Temporal/Instant/prototype/add/argument-string-negative-fractional-units.js:
default: 'Test262Error: negative fractional hours Expected SameValue(«999911555595557201», «999911555595557200») to be true'
strict mode: 'Test262Error: negative fractional hours Expected SameValue(«999911555595557201», «999911555595557200») to be true'
test/built-ins/Temporal/Instant/prototype/since/largestunit.js:
default: 'Test262Error: does not include higher units than necessary (largest unit unspecified) nanoseconds result Expected SameValue(«40», «101») to be true'
strict mode: 'Test262Error: does not include higher units than necessary (largest unit unspecified) nanoseconds result Expected SameValue(«40», «101») to be true'
test/built-ins/Temporal/Instant/prototype/subtract/argument-string-negative-fractional-units.js:
default: 'Test262Error: negative fractional hours Expected SameValue(«1000088444404442799», «1000088444404442800») to be true'
strict mode: 'Test262Error: negative fractional hours Expected SameValue(«1000088444404442799», «1000088444404442800») to be true'
test/built-ins/Temporal/Instant/prototype/toJSON/timezone-getoffsetnanosecondsfor-not-callable.js:
default: 'Test262Error: Uncallable undefined getOffsetNanosecondsFor should throw TypeError Expected a TypeError to be thrown but no exception was thrown at all'
strict mode: 'Test262Error: Uncallable undefined getOffsetNanosecondsFor should throw TypeError Expected a TypeError to be thrown but no exception was thrown at all'
@@ -1101,18 +1083,12 @@ test/built-ins/Temporal/PlainTime/from/argument-string-with-calendar.js:
test/built-ins/Temporal/PlainTime/from/argument-zoneddatetime-timezone-getoffsetnanosecondsfor-not-callable.js:
default: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
strict mode: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
test/built-ins/Temporal/PlainTime/prototype/add/argument-string-negative-fractional-units.js:
default: 'Test262Error: negative fractional hours nanosecond result Expected SameValue(«201», «200») to be true'
strict mode: 'Test262Error: negative fractional hours nanosecond result Expected SameValue(«201», «200») to be true'
test/built-ins/Temporal/PlainTime/prototype/equals/argument-zoneddatetime-timezone-getoffsetnanosecondsfor-not-callable.js:
default: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
strict mode: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
test/built-ins/Temporal/PlainTime/prototype/since/argument-zoneddatetime-timezone-getoffsetnanosecondsfor-not-callable.js:
default: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
strict mode: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
test/built-ins/Temporal/PlainTime/prototype/subtract/argument-string-negative-fractional-units.js:
default: 'Test262Error: negative fractional hours nanosecond result Expected SameValue(«799», «800») to be true'
strict mode: 'Test262Error: negative fractional hours nanosecond result Expected SameValue(«799», «800») to be true'
test/built-ins/Temporal/PlainTime/prototype/until/argument-zoneddatetime-timezone-getoffsetnanosecondsfor-not-callable.js:
default: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
strict mode: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
@@ -70,50 +70,38 @@ static int32_t parseDecimalInt32(const CharType* characters, unsigned length)
// https://tc39.es/proposal-temporal/#sec-temporal-durationhandlefractions
static void handleFraction(Duration& duration, int factor, StringView fractionString, TemporalUnit fractionType)
{
ASSERT(fractionString.length() && fractionString.length() <= 9 && fractionString.isAllASCII());
auto fractionLength = fractionString.length();
ASSERT(fractionLength && fractionLength <= 9 && fractionString.isAllASCII());
ASSERT(fractionType == TemporalUnit::Hour || fractionType == TemporalUnit::Minute || fractionType == TemporalUnit::Second);

if (fractionType == TemporalUnit::Second) {
Vector<LChar, 9> padded(9, '0');
for (unsigned i = 0; i < fractionString.length(); i++)
padded[i] = fractionString[i];
duration.setMilliseconds(factor * parseDecimalInt32(padded.data(), 3));
duration.setMicroseconds(factor * parseDecimalInt32(padded.data() + 3, 3));
duration.setNanoseconds(factor * parseDecimalInt32(padded.data() + 6, 3));
return;
}
Vector<LChar, 9> padded(9, '0');
for (unsigned i = 0; i < fractionLength; i++)
padded[i] = fractionString[i];

double fraction = factor * parseInt(fractionString, 10) / std::pow(10, fractionString.length());
int64_t fraction = static_cast<int64_t>(factor) * parseDecimalInt32(padded.data(), 9);
if (!fraction)
return;

static constexpr int64_t divisor = 1'000'000'000LL;
if (fractionType == TemporalUnit::Hour) {
fraction *= 60;
duration.setMinutes(std::trunc(fraction));
fraction = std::fmod(fraction, 1);
duration.setMinutes(fraction / divisor);
fraction %= divisor;
if (!fraction)
return;
}

fraction *= 60;
duration.setSeconds(std::trunc(fraction));
fraction = std::fmod(fraction, 1);
if (!fraction)
return;

fraction *= 1000;
duration.setMilliseconds(std::trunc(fraction));
fraction = std::fmod(fraction, 1);
if (!fraction)
return;

fraction *= 1000;
duration.setMicroseconds(std::trunc(fraction));
fraction = std::fmod(fraction, 1);
if (!fraction)
return;
if (fractionType != TemporalUnit::Second) {
fraction *= 60;
duration.setSeconds(fraction / divisor);
fraction %= divisor;
if (!fraction)
return;
}

duration.setNanoseconds(std::trunc(fraction * 1000));
duration.setMilliseconds(fraction / nsPerMillisecond);
duration.setMicroseconds(fraction % nsPerMillisecond / nsPerMicrosecond);
duration.setNanoseconds(fraction % nsPerMicrosecond);
}

// ParseTemporalDurationString ( isoString )

0 comments on commit 92e204d

Please sign in to comment.