Repository: moment/luxon
Pull request #576 (2019-09-22 02:28:41)

Fix for invalid Datetime when not in valid ECMA DateTime range
As reported in #563 this problem occurs when with the DateTime.plus a duration of seconds is added which will result in a DateTime Object outside of the ECMA DateTime range.

Reproduction Code 
```js
const orgDate = DateTime.utc(1970,1,1,0,0,0,0); 
const maxRangeDays = 1E8 // max range of Dates in days
const secsPerDay = 24*60*60; // secs per day

// When adding Seconds
// d.invalid is null - expected to have reason "invalid input"
// d.c contains NaN - expected to be null
const d = orgDate.plus({seconds: maxRangeDays  * secsPerDay +1})

// When adding days
// e.invalid is has reason "invalid input"
// e.c contains null
const e = orgDate.plus({day: maxRangeDays  +1})
```

The reason why the seconds did not result within an invalid result is because:

datetime.js -> adjustTime 

In this function two different ways are used to create the returning ts property.
Duration Properties of type "hours, minutes, seconds, milliseconds" are parsed by Durations.fromObject -> as("milliseconds")
whereas Duration Properties of type "year,month,day" are parsed by the objToLocalTS.

In the case we have only  "year,month,day" the function objToLocalTS already returns an invalid object with NaN for the properties, whereas the "timepart" durations are passed as an number to the constructor of DateTime.

Because there is no "test" if the Object created in the constructor by function tsToObj is valid, the new DateTime returned appears valid (but it isnt because its full of NaN and the Date,UTC in the tsToObj already returned invalid date!).

Because of this, there was an additional check in the constructor added of the DateTime class.
This invalidates the c and o, if the returned Object from tsToObj is not valid anymore.


For further refactoring task i would take the discussion for the functions:
DateTime.tsToObj
DateTime.objToLocalTS

IMHO this functions should throw an error which can then be catched if Date.XXX is not giving back a successfull result.


icambron: This makes sense to me. Thanks!

Patch:
diff --git a/src/datetime.js b/src/datetime.js
index a57553ad..eba810d6 100644
--- a/src/datetime.js
+++ b/src/datetime.js
@@ -370,11 +370,12 @@ export default class DateTime {
    * @access private
    */
   constructor(config) {
-    const zone = config.zone || Settings.defaultZone,
-      invalid =
-        config.invalid ||
-        (Number.isNaN(config.ts) ? new Invalid("invalid input") : null) ||
-        (!zone.isValid ? unsupportedZone(zone) : null);
+    const zone = config.zone || Settings.defaultZone;
+
+    let invalid =
+      config.invalid ||
+      (Number.isNaN(config.ts) ? new Invalid("invalid input") : null) ||
+      (!zone.isValid ? unsupportedZone(zone) : null);
     /**
      * @access private
      */
@@ -384,8 +385,15 @@ export default class DateTime {
       o = null;
     if (!invalid) {
       const unchanged = config.old && config.old.ts === this.ts && config.old.zone.equals(zone);
-      c = unchanged ? config.old.c : tsToObj(this.ts, zone.offset(this.ts));
-      o = unchanged ? config.old.o : zone.offset(this.ts);
+
+      if (unchanged) {
+        [c, o] = [config.old.c, config.old.o];
+      } else {
+        c = tsToObj(this.ts, zone.offset(this.ts));
+        invalid = Number.isNaN(c.year) ? new Invalid("invalid input") : null;
+        c = invalid ? null : c;
+        o = invalid ? null : zone.offset(this.ts);
+      }
     }
 
     /**


Test patch:
diff --git a/test/datetime/math.test.js b/test/datetime/math.test.js
index bdd9cd64..9902a84d 100644
--- a/test/datetime/math.test.js
+++ b/test/datetime/math.test.js
@@ -96,6 +96,16 @@ test("DateTime#plus works across the 100 barrier", () => {
   expect(d.day).toBe(2);
 });
 
+test("DateTime#plus renders invalid when out of max. datetime range using days", () => {
+  const d = DateTime.utc(1970, 1, 1, 0, 0, 0, 0).plus({ day: 1e8 + 1 });
+  expect(d.isValid).toBe(false);
+});
+
+test("DateTime#plus renders invalid when out of max. datetime range using seconds", () => {
+  const d = DateTime.utc(1970, 1, 1, 0, 0, 0, 0).plus({ second: 1e8 * 24 * 60 * 60 + 1 });
+  expect(d.isValid).toBe(false);
+});
+
 //------
 // #minus()
 //------
@@ -144,6 +154,16 @@ test("DateTime#minus works across the 100 barrier", () => {
   expect(d.day).toBe(31);
 });
 
+test("DateTime#minus renders invalid when out of max. datetime range using days", () => {
+  const d = DateTime.utc(1970, 1, 1, 0, 0, 0, 0).minus({ day: 1e8 + 1 });
+  expect(d.isValid).toBe(false);
+});
+
+test("DateTime#minus renders invalid when out of max. datetime range using seconds", () => {
+  const d = DateTime.utc(1970, 1, 1, 0, 0, 0, 0).minus({ second: 1e8 * 24 * 60 * 60 + 1 });
+  expect(d.isValid).toBe(false);
+});
+
 //------
 // #startOf()
 //------