Fix overflow affecting invalid ISO string

It was decided in #260 that overflow: 'constrain' should not affect invalid values coming from ISO strings, but it seems that it crept back in during a refactor. Fix this and add a test so that it doesn't regress again.
tc39 · Nov 16, 2020 · 9fc3bf1 · 9fc3bf1
1 parent 1f93827
commit 9fc3bf1
Show file tree

Hide file tree

Showing 12 changed files with 44 additions and 14 deletions.
diff --git a/polyfill/lib/ecmascript.mjs b/polyfill/lib/ecmascript.mjs
@@ -961,7 +961,7 @@ export const ES = ObjectAssign({}, ES2020, {
       return ES.DateFromFields(calendar, fields, constructor, overflow);
     }
     let { year, month, day, calendar } = ES.ParseTemporalDateString(ES.ToString(item));
-    ({ year, month, day } = ES.RegulateDate(year, month, day, overflow));
+    ES.RejectDate(year, month, day);
     if (calendar === undefined) calendar = ES.GetISO8601Calendar();
     calendar = ES.ToTemporalCalendar(calendar);
     let result = new constructor(year, month, day, calendar);
@@ -1021,6 +1021,7 @@ export const ES = ObjectAssign({}, ES2020, {
         nanosecond,
         calendar
       } = ES.ParseTemporalDateTimeString(ES.ToString(item)));
+      ES.RejectDateTime(year, month, day, hour, minute, second, millisecond, microsecond, nanosecond);
       if (calendar === undefined) calendar = ES.GetISO8601Calendar();
       calendar = ES.ToTemporalCalendar(calendar);
     }
@@ -1102,8 +1103,8 @@ export const ES = ObjectAssign({}, ES2020, {
       return ES.MonthDayFromFields(calendar, fields, constructor, overflow);
     }
 
-    let { month, day, referenceISOYear, calendar } = ES.ParseTemporalMonthDayString(ES.ToString(item));
-    ({ month, day } = ES.RegulateMonthDay(month, day, overflow));
+    let { month, day, referenceISOYear = 1972, calendar } = ES.ParseTemporalMonthDayString(ES.ToString(item));
+    ES.RejectDate(referenceISOYear, month, day);
     if (calendar === undefined) calendar = ES.GetISO8601Calendar();
     calendar = ES.ToTemporalCalendar(calendar);
     if (referenceISOYear === undefined) referenceISOYear = 1972;
@@ -1123,23 +1124,24 @@ export const ES = ObjectAssign({}, ES2020, {
         }
       }
       ({ hour, minute, second, millisecond, microsecond, nanosecond } = ES.ToTemporalTimeRecord(item));
+      ({ hour, minute, second, millisecond, microsecond, nanosecond } = ES.RegulateTime(
+        hour,
+        minute,
+        second,
+        millisecond,
+        microsecond,
+        nanosecond,
+        overflow
+      ));
     } else {
       ({ hour, minute, second, millisecond, microsecond, nanosecond, calendar } = ES.ParseTemporalTimeString(
         ES.ToString(item)
       ));
+      ES.RejectTime(hour, minute, second, millisecond, microsecond, nanosecond);
       if (calendar !== undefined && calendar !== 'iso8601') {
         throw new RangeError('PlainTime can only have iso8601 calendar');
       }
     }
-    ({ hour, minute, second, millisecond, microsecond, nanosecond } = ES.RegulateTime(
-      hour,
-      minute,
-      second,
-      millisecond,
-      microsecond,
-      nanosecond,
-      overflow
-    ));
     let result = new constructor(hour, minute, second, millisecond, microsecond, nanosecond);
     if (!ES.IsTemporalTime(result)) throw new TypeError('invalid result');
     return result;
@@ -1156,7 +1158,7 @@ export const ES = ObjectAssign({}, ES2020, {
     }
 
     let { year, month, referenceISODay = 1, calendar } = ES.ParseTemporalYearMonthString(ES.ToString(item));
-    ({ year, month } = ES.RegulateYearMonth(year, month, overflow));
+    ES.RejectDate(year, month, referenceISODay);
     if (calendar === undefined) calendar = ES.GetISO8601Calendar();
     calendar = ES.ToTemporalCalendar(calendar);
     let result = new constructor(year, month, calendar, referenceISODay);

diff --git a/polyfill/test/plaindate.mjs b/polyfill/test/plaindate.mjs
@@ -986,6 +986,9 @@ describe('Date', () => {
           );
         });
       });
+      it('constrain has no effect on invalid ISO string', () => {
+        throws(() => PlainDate.from('2020-13-34', { overflow: 'constrain' }), RangeError);
+      });
     });
   });
   describe('Date.compare works', () => {

diff --git a/polyfill/test/plaindatetime.mjs b/polyfill/test/plaindatetime.mjs
@@ -1376,6 +1376,9 @@ describe('DateTime', () => {
       const leap = { year: 2016, month: 12, day: 31, hour: 23, minute: 59, second: 60 };
       it('reject leap second', () => throws(() => PlainDateTime.from(leap, { overflow: 'reject' }), RangeError));
       it('constrain leap second', () => equal(`${PlainDateTime.from(leap)}`, '2016-12-31T23:59:59'));
+      it('constrain has no effect on invalid ISO string', () => {
+        throws(() => PlainDateTime.from('2020-13-34T24:60', { overflow: 'constrain' }), RangeError);
+      });
     });
     it('variant time separators', () => {
       equal(`${PlainDateTime.from('1976-11-18t15:23Z')}`, '1976-11-18T15:23:00');

diff --git a/polyfill/test/plainmonthday.mjs b/polyfill/test/plainmonthday.mjs
@@ -107,6 +107,9 @@ describe('MonthDay', () => {
             );
           });
         });
+        it('constrain has no effect on invalid ISO string', () => {
+          throws(() => PlainMonthDay.from('13-34', { overflow: 'constrain' }), RangeError);
+        });
       });
       describe('Leap day', () => {
         ['reject', 'constrain'].forEach((overflow) =>

diff --git a/polyfill/test/plaintime.mjs b/polyfill/test/plaintime.mjs
@@ -1284,6 +1284,9 @@ describe('Time', () => {
       const leap = { hour: 23, minute: 59, second: 60 };
       it('reject leap second', () => throws(() => PlainTime.from(leap, { overflow: 'reject' }), RangeError));
       it('constrain leap second', () => equal(`${PlainTime.from(leap)}`, '23:59:59'));
+      it('constrain has no effect on invalid ISO string', () => {
+        throws(() => PlainTime.from('24:60', { overflow: 'constrain' }), RangeError);
+      });
     });
     it('object must contain at least one correctly-spelled property', () => {
       throws(() => PlainTime.from({}), TypeError);

diff --git a/polyfill/test/plainyearmonth.mjs b/polyfill/test/plainyearmonth.mjs
@@ -133,6 +133,9 @@ describe('YearMonth', () => {
             );
           });
         });
+        it('constrain has no effect on invalid ISO string', () => {
+          throws(() => PlainYearMonth.from('2020-13', { overflow: 'constrain' }), RangeError);
+        });
       });
       it('object must contain at least the required correctly-spelled properties', () => {
         throws(() => PlainYearMonth.from({}), TypeError);

diff --git a/polyfill/test/zoneddatetime.mjs b/polyfill/test/zoneddatetime.mjs
@@ -400,6 +400,9 @@ describe('ZonedDateTime', () => {
     });
     it('no junk at end of string', () =>
       throws(() => ZonedDateTime.from('1976-11-18T15:23:30.123456789-08:00[America/Los_Angeles]junk'), RangeError));
+    it('constrain has no effect on invalid ISO string', () => {
+      throws(() => ZonedDateTime.from('2020-13-34T24:60[America/Los_Angeles]', { overflow: 'constrain' }), RangeError);
+    });
     describe('Offset option', () => {
       it("{ offset: 'reject' } throws if offset does not match offset time zone", () => {
         throws(() => ZonedDateTime.from('2020-03-08T01:00-04:00[-08:00]'), RangeError);

diff --git a/spec/plaindate.html b/spec/plaindate.html
@@ -755,6 +755,8 @@ <h1>ToTemporalDate ( _item_ [ , _constructor_ [ , _overflow_ ] ] )</h1>
           1. Return ? DateFromFields(_calendar_, _fields_, _constructor_, _overflow_).
         1. Let _string_ be ? ToString(_item_).
         1. Let _result_ be ? ParseTemporalDateString(_string_).
+        1. If ! ValidateDate(_result_.[[Year]], _result_.[[Month]], _result_.[[Day]]) is *false*, then
+          1. Throw a *RangeError* exception.
         1. Let _calendar_ be _result_.[[Calendar]].
         1. If _calendar_ is *undefined*, set _calendar_ to ? GetISO8601Calendar().
         1. Set _calendar_ to ? ToTemporalCalendar(_calendar_).

diff --git a/spec/plaindatetime.html b/spec/plaindatetime.html
@@ -957,6 +957,8 @@ <h1>ToTemporalDateTime ( _item_ [ , _constructor_ [ , _overflow_ ] ] )</h1>
         1. Else,
           1. Let _string_ be ? ToString(_item_).
           1. Let _result_ be ? ParseTemporalDateTimeString(_string_).
+          1. If ! ValidateDateTime(__.[[Year]], __.[[Month]], __.[[Day]], __.[[Hour]], __.[[Minute]], __.[[Second]], __.[[Millisecond]], __.[[Microsecond]], __.[[Nanosecond]]) is *false*, then
+            1. Throw a *RangeError* exception.
         1. Let _calendar_ be _result_.[[Calendar]].
         1. If _calendar_ is *undefined*, set _calendar_ to ? GetISO8601Calendar().
         1. Set _calendar_ to ? ToTemporalCalendar(_calendar_).

diff --git a/spec/plainmonthday.html b/spec/plainmonthday.html
@@ -377,6 +377,8 @@ <h1>ToTemporalMonthDay ( _item_ [ , _constructor_ [ , _overflow_ ] ] )</h1>
         1. Let _result_ be ? ParseTemporalMonthDayString(_string_).
         1. Let _referenceISOYear_ be the first leap year after the Unix epoch (1972).
         1. <mark>TODO: get referenceISOYear from string</mark>.
+        1. If ! ValidateDate(_referenceISOYear_, _result_.[[Month]], _result_.[[Day]]) is *false*, then
+          1. Throw a *RangeError* exception.
         1. Let _calendar_ be _result_.[[Calendar]].
         1. If _calendar_ is *undefined*, set _calendar_ to ? GetISO8601Calendar().
         1. Set _calendar_ to ? ToTemporalCalendar(_calendar_).

diff --git a/spec/plaintime.html b/spec/plaintime.html
@@ -657,12 +657,14 @@ <h1>ToTemporalTime ( _item_ [ , _constructor_ [ , _overflow_ ] ] )</h1>
             1. If ? CalendarToString(_calendar_) is not *"iso8601"*, then
               1. Throw a *RangeError* exception.
           1. Let _result_ be ? ToTemporalTimeRecord(_item_).
+          1. Set _result_ to ? RegulateTime(_result_.[[Hour]], _result_.[[Minute]], _result_.[[Second]], _result_.[[Millisecond]], _result_.[[Microsecond]], _result_.[[Nanosecond]], _overflow_).
         1. Else,
           1. Let _string_ be ? ToString(_item_).
           1. Let _result_ be ? ParseTemporalTimeString(_string_).
+          1. If ! ValidateTime(_result_.[[Hour]], _result_.[[Minute]], _result_.[[Second]], _result_.[[Millisecond]], _result_.[[Microsecond]], _result_.[[Nanosecond]]) is *false*, then
+            1. Throw a *RangeError* exception.
           1. If _result_.[[Calendar]] is not one of *undefined* or *"iso8601"*, then
             1. Throw a *RangeError* exception.
-        1. Set _result_ to ? RegulateTime(_result_.[[Hour]], _result_.[[Minute]], _result_.[[Second]], _result_.[[Millisecond]], _result_.[[Microsecond]], _result_.[[Nanosecond]], _overflow_).
         1. Return ? CreateTemporalTimeFromStatic(_constructor_, _result_.[[Hour]], _result_.[[Minute]], _result_.[[Second]], _result_.[[Millisecond]], _result_.[[Microsecond]], _result_.[[Nanosecond]]).
       </emu-alg>
     </emu-clause>

diff --git a/spec/plainyearmonth.html b/spec/plainyearmonth.html
@@ -572,6 +572,8 @@ <h1>ToTemporalYearMonth ( _item_ [ , _constructor_ [ , _overflow_ ] ] )</h1>
         1. Let _result_ be ? ParseTemporalYearMonthString(_string_).
         1. Let _referenceISODay_ be 1.
         1. <mark>TODO: get referenceISODay from string</mark>.
+        1. If ! ValidateDate(_result_.[[Year]], _result_.[[Month]], _referenceISODay_) is *false*, then
+          1. Throw a *RangeError* exception.
         1. Let _calendar_ be _result_.[[Calendar]].
         1. If _calendar_ is *undefined*, set _calendar_ to ? GetISO8601Calendar().
         1. Set _calendar_ to ? ToTemporalCalendar(_calendar_).