- Revision
- 294306
- Author
- ross.kirsl...@sony.com
- Date
- 2022-05-16 23:17:19 -0700 (Mon, 16 May 2022)
Log Message
ISO8601::parseCalendarTime must throw in cases of time/date ambiguity
https://bugs.webkit.org/show_bug.cgi?id=240498
Reviewed by Yusuke Suzuki.
The CalendarTime production (https://tc39.es/proposal-temporal/#prod-CalendarTime) goes to a lot of trouble
to ensure that ambiguous parses are not allowed; this patch ensures that our implementation does so too.
In short:
- CalendarTime is basically TimeDesignator[opt] TimeSpec TimeZone[opt] Calendar[opt].
- When we don't have a TimeDesignator or a Calendar, we're in danger of ambiguity:
YYYY-MM, YYYYMM, MM-DD, MMDD look the same as HHMM-UU, HHMMSS, HH-UU, HHMM (where UU is a UTC offset).
- The solution is to attempt to reparse the string as one of these date formats and throw if we succeed.
* stress/temporal-plaintime.js: Adjust expectations.
* test262/expectations.yaml: Mark 10 test cases as passing.
* runtime/ISO8601.cpp:
(JSC::ISO8601::isAmbiguousCalendarTime):
(JSC::ISO8601::parseCalendarTime):
(JSC::ISO8601::daysInMonth):
* runtime/ISO8601.h:
Canonical link: https://commits.webkit.org/250634@main
Modified Paths
Diff
Modified: trunk/JSTests/ChangeLog (294305 => 294306)
--- trunk/JSTests/ChangeLog 2022-05-17 06:11:13 UTC (rev 294305)
+++ trunk/JSTests/ChangeLog 2022-05-17 06:17:19 UTC (rev 294306)
@@ -1,3 +1,13 @@
+2022-05-16 Ross Kirsling <ross.kirsl...@sony.com>
+
+ ISO8601::parseCalendarTime must throw in cases of time/date ambiguity
+ https://bugs.webkit.org/show_bug.cgi?id=240498
+
+ Reviewed by Yusuke Suzuki.
+
+ * stress/temporal-plaintime.js: Adjust expectations.
+ * test262/expectations.yaml: Mark 10 test cases as passing.
+
2022-05-15 Angelos Oikonomopoulos <ange...@igalia.com>
Skip flaky test on ARM
Modified: trunk/JSTests/stress/temporal-plaintime.js (294305 => 294306)
--- trunk/JSTests/stress/temporal-plaintime.js 2022-05-17 06:11:13 UTC (rev 294305)
+++ trunk/JSTests/stress/temporal-plaintime.js 2022-05-17 06:17:19 UTC (rev 294306)
@@ -95,13 +95,16 @@
}
shouldBe(String(Temporal.PlainTime.from('03')), `03:00:00`);
-shouldBe(String(Temporal.PlainTime.from('0314')), `03:14:00`);
+shouldBe(String(Temporal.PlainTime.from('T0314')), `03:14:00`);
+shouldThrow(() => Temporal.PlainTime.from('0314'), RangeError);
shouldBe(String(Temporal.PlainTime.from('031415')), `03:14:15`);
shouldBe(String(Temporal.PlainTime.from('03:14')), `03:14:00`);
shouldBe(String(Temporal.PlainTime.from('03:14:15')), `03:14:15`);
shouldBe(String(Temporal.PlainTime.from('03:24:30')), `03:24:30`);
-shouldBe(String(Temporal.PlainTime.from('2020-01')), `20:20:00`); // -01 UTC offset
-shouldBe(String(Temporal.PlainTime.from('01-01')), `01:00:00`); // -01 UTC offset
+shouldBe(String(Temporal.PlainTime.from('T2020-01')), `20:20:00`); // -01 UTC offset
+shouldThrow(() => Temporal.PlainTime.from('2020-01'), RangeError);
+shouldBe(String(Temporal.PlainTime.from('T01-01')), `01:00:00`); // -01 UTC offset
+shouldThrow(() => Temporal.PlainTime.from('01-01'), RangeError);
shouldBe(String(Temporal.PlainTime.from('03:24:30[u-ca=japanese]')), `03:24:30`);
shouldBe(String(Temporal.PlainTime.from('03:24:30+01:00[Europe/Brussels][u-ca=japanese]')), `03:24:30`);
shouldBe(String(Temporal.PlainTime.from('03:24:30+01:00[u-ca=japanese]')), `03:24:30`);
Modified: trunk/JSTests/test262/expectations.yaml (294305 => 294306)
--- trunk/JSTests/test262/expectations.yaml 2022-05-17 06:11:13 UTC (rev 294305)
+++ trunk/JSTests/test262/expectations.yaml 2022-05-17 06:17:19 UTC (rev 294306)
@@ -1065,9 +1065,6 @@
test/built-ins/Temporal/Instant/prototype/toString/timezone-string-multiple-offsets.js:
default: 'RangeError: argument needs to be UTC offset string, TimeZone identifier, or temporal Instant string'
strict mode: 'RangeError: argument needs to be UTC offset string, TimeZone identifier, or temporal Instant string'
-test/built-ins/Temporal/PlainTime/compare/argument-string-time-designator-required-for-disambiguation.js:
- default: 'Test262Error: 2021-12 is ambiguous and requires T prefix (first argument) Expected a RangeError to be thrown but no exception was thrown at all'
- strict mode: 'Test262Error: 2021-12 is ambiguous and requires T prefix (first argument) Expected a RangeError to be thrown but no exception was thrown at all'
test/built-ins/Temporal/PlainTime/compare/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)')"
@@ -1074,9 +1071,6 @@
test/built-ins/Temporal/PlainTime/from/argument-plaindatetime.js:
default: "TypeError: undefined is not a constructor (evaluating 'new Temporal.PlainDateTime(2000, 5, 2, 12, 34, 56, 987, 654, 321, calendar)')"
strict mode: "TypeError: undefined is not a constructor (evaluating 'new Temporal.PlainDateTime(2000, 5, 2, 12, 34, 56, 987, 654, 321, calendar)')"
-test/built-ins/Temporal/PlainTime/from/argument-string-time-designator-required-for-disambiguation.js:
- default: 'Test262Error: 2021-12 is ambiguous and requires T prefix Expected a RangeError to be thrown but no exception was thrown at all'
- strict mode: 'Test262Error: 2021-12 is ambiguous and requires T prefix Expected a RangeError to be thrown but no exception was thrown at all'
test/built-ins/Temporal/PlainTime/from/argument-string-with-calendar.js:
default: 'Test262Error: Expected a RangeError to be thrown but no exception was thrown at all'
strict mode: 'Test262Error: Expected a RangeError to be thrown but no exception was thrown at all'
@@ -1083,15 +1077,9 @@
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/equals/argument-string-time-designator-required-for-disambiguation.js:
- default: 'Test262Error: 2021-12 is ambiguous and requires T prefix Expected a RangeError to be thrown but no exception was thrown at all'
- strict mode: 'Test262Error: 2021-12 is ambiguous and requires T prefix Expected a RangeError to be thrown but no exception was thrown at all'
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-string-time-designator-required-for-disambiguation.js:
- default: 'Test262Error: 2021-12 is ambiguous and requires T prefix Expected a RangeError to be thrown but no exception was thrown at all'
- strict mode: 'Test262Error: 2021-12 is ambiguous and requires T prefix Expected a RangeError to be thrown but no exception was thrown at all'
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)')"
@@ -1101,9 +1089,6 @@
test/built-ins/Temporal/PlainTime/prototype/since/roundingmode-floor.js:
default: 'Test262Error: hours hours result Expected SameValue(«5», «4») to be true'
strict mode: 'Test262Error: hours hours result Expected SameValue(«5», «4») to be true'
-test/built-ins/Temporal/PlainTime/prototype/until/argument-string-time-designator-required-for-disambiguation.js:
- default: 'Test262Error: 2021-12 is ambiguous and requires T prefix Expected a RangeError to be thrown but no exception was thrown at all'
- strict mode: 'Test262Error: 2021-12 is ambiguous and requires T prefix Expected a RangeError to be thrown but no exception was thrown at all'
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)')"
Modified: trunk/Source/_javascript_Core/ChangeLog (294305 => 294306)
--- trunk/Source/_javascript_Core/ChangeLog 2022-05-17 06:11:13 UTC (rev 294305)
+++ trunk/Source/_javascript_Core/ChangeLog 2022-05-17 06:17:19 UTC (rev 294306)
@@ -1,3 +1,25 @@
+2022-05-16 Ross Kirsling <ross.kirsl...@sony.com>
+
+ ISO8601::parseCalendarTime must throw in cases of time/date ambiguity
+ https://bugs.webkit.org/show_bug.cgi?id=240498
+
+ Reviewed by Yusuke Suzuki.
+
+ The CalendarTime production (https://tc39.es/proposal-temporal/#prod-CalendarTime) goes to a lot of trouble
+ to ensure that ambiguous parses are not allowed; this patch ensures that our implementation does so too.
+
+ In short:
+ - CalendarTime is basically TimeDesignator[opt] TimeSpec TimeZone[opt] Calendar[opt].
+ - When we don't have a TimeDesignator or a Calendar, we're in danger of ambiguity:
+ YYYY-MM, YYYYMM, MM-DD, MMDD look the same as HHMM-UU, HHMMSS, HH-UU, HHMM (where UU is a UTC offset)
+ - The solution is to attempt to reparse the string as one of these date formats and throw if we succeed.
+
+ * runtime/ISO8601.cpp:
+ (JSC::ISO8601::isAmbiguousCalendarTime):
+ (JSC::ISO8601::parseCalendarTime):
+ (JSC::ISO8601::daysInMonth):
+ * runtime/ISO8601.h:
+
2022-05-16 Mark Lam <mark....@apple.com>
Add ARM64 disassembler support for symbolic dumping of some well known constants.
Modified: trunk/Source/_javascript_Core/runtime/ISO8601.cpp (294305 => 294306)
--- trunk/Source/_javascript_Core/runtime/ISO8601.cpp 2022-05-17 06:11:13 UTC (rev 294305)
+++ trunk/Source/_javascript_Core/runtime/ISO8601.cpp 2022-05-17 06:17:19 UTC (rev 294306)
@@ -782,31 +782,6 @@
return std::tuple { WTFMove(plainTime.value()), std::nullopt };
}
-// https://tc39.es/proposal-temporal/#sec-temporal-isodaysinmonth
-unsigned daysInMonth(int32_t year, unsigned month)
-{
- switch (month) {
- case 1:
- case 3:
- case 5:
- case 7:
- case 8:
- case 10:
- case 12:
- return 31;
- case 4:
- case 6:
- case 9:
- case 11:
- return 30;
- case 2:
- if (isLeapYear(year))
- return 29;
- return 28;
- }
- return 0;
-}
-
template<typename CharacterType>
static std::optional<PlainDate> parseDate(StringParsingBuffer<CharacterType>& buffer)
{
@@ -1039,14 +1014,76 @@
});
}
+template<typename CharacterType>
+static bool isAmbiguousCalendarTime(StringParsingBuffer<CharacterType>& buffer)
+{
+ auto length = buffer.lengthRemaining();
+ ASSERT(length > 1);
+
+ // There is no ambiguity if we have a TimeDesignator.
+ if (toASCIIUpper(*buffer) == 'T')
+ return false;
+
+ // The string is known to be valid as `TimeSpec TimeZone[opt]`, so DateExtendedYear and TwoDashes are not possible.
+ // Actual possibilities are `DateFourDigitYear -[opt] DateMonth` and `DateMonth -[opt] DateDay`, i.e. YYYY-MM, YYYYMM, MM-DD, MMDD.
+ ASSERT(isASCIIDigit(buffer[0]) && isASCIIDigit(buffer[1]));
+
+ unsigned monthPartLength = 2;
+ switch (length) {
+ case 7:
+ if (!isASCIIDigit(buffer[2]) || !isASCIIDigit(buffer[3]) || buffer[4] != '-' || !isASCIIDigit(buffer[5]) || !isASCIIDigit(buffer[6]))
+ return false;
+ buffer.advanceBy(5);
+ break;
+ case 6:
+ if (!isASCIIDigit(buffer[2]) || !isASCIIDigit(buffer[3]) || !isASCIIDigit(buffer[4]) || !isASCIIDigit(buffer[5]))
+ return false;
+ buffer.advanceBy(4);
+ break;
+ case 5:
+ if (buffer[2] != '-' || !isASCIIDigit(buffer[3]) || !isASCIIDigit(buffer[4]))
+ return false;
+ monthPartLength++;
+ break;
+ case 4:
+ if (!isASCIIDigit(buffer[2]) || !isASCIIDigit(buffer[3]))
+ return false;
+ break;
+ default:
+ return false;
+ }
+
+ // Any YYYY is valid, we just need to check the MM and DD.
+ unsigned month = (buffer[0] - '0') * 10 + (buffer[1] - '0');
+ if (!month || month > 12)
+ return false;
+
+ buffer.advanceBy(monthPartLength);
+ if (buffer.hasCharactersRemaining()) {
+ unsigned day = (buffer[0] - '0') * 10 + (buffer[1] - '0');
+ if (!day || day > daysInMonth(month))
+ return false;
+ }
+
+ return true;
+}
+
std::optional<std::tuple<PlainTime, std::optional<TimeZoneRecord>, std::optional<CalendarRecord>>> parseCalendarTime(StringView string)
{
- return readCharactersForParsing(string, [](auto buffer) -> std::optional<std::tuple<PlainTime, std::optional<TimeZoneRecord>, std::optional<CalendarRecord>>> {
+ auto tuple = readCharactersForParsing(string, [](auto buffer) -> std::optional<std::tuple<PlainTime, std::optional<TimeZoneRecord>, std::optional<CalendarRecord>>> {
auto result = parseCalendarTime(buffer);
if (!buffer.atEnd())
return std::nullopt;
return result;
});
+
+ // Without a calendar, we need to verify that the parse isn't ambiguous with DateSpecYearMonth or DateSpecMonthDay.
+ if (tuple && !std::get<2>(tuple.value())) {
+ if (readCharactersForParsing(string, [](auto buffer) -> bool { return isAmbiguousCalendarTime(buffer); }))
+ return std::nullopt;
+ }
+
+ return tuple;
}
std::optional<std::tuple<PlainDate, std::optional<PlainTime>, std::optional<TimeZoneRecord>>> parseDateTime(StringView string)
@@ -1154,11 +1191,18 @@
{ 31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 }
};
+// https://tc39.es/proposal-temporal/#sec-temporal-isodaysinmonth
uint8_t daysInMonth(int32_t year, uint8_t month)
{
- return daysInMonths[!!isLeapYear(year)][month - 1];
+ return daysInMonths[isLeapYear(year)][month - 1];
}
+uint8_t daysInMonth(uint8_t month)
+{
+ constexpr unsigned isLeapYear = 1;
+ return daysInMonths[isLeapYear][month - 1];
+}
+
// https://tc39.es/proposal-temporal/#sec-temporal-formattimezoneoffsetstring
String formatTimeZoneOffsetString(int64_t offset)
{
Modified: trunk/Source/_javascript_Core/runtime/ISO8601.h (294305 => 294306)
--- trunk/Source/_javascript_Core/runtime/ISO8601.h 2022-05-17 06:11:13 UTC (rev 294305)
+++ trunk/Source/_javascript_Core/runtime/ISO8601.h 2022-05-17 06:17:19 UTC (rev 294306)
@@ -303,11 +303,11 @@
uint8_t weeksInYear(int32_t year);
uint8_t weekOfYear(PlainDate);
uint8_t daysInMonth(int32_t year, uint8_t month);
+uint8_t daysInMonth(uint8_t month);
String formatTimeZoneOffsetString(int64_t);
String temporalTimeToString(PlainTime, std::tuple<Precision, unsigned> precision);
String temporalDateToString(PlainDate);
String monthCode(uint32_t);
-unsigned daysInMonth(int32_t year, unsigned month);
bool isValidDuration(const Duration&);
