Bug 1831322 - Strengthen Date checks in Places. r=Standard8

Differential Revision: https://phabricator.services.mozilla.com/D177160

toolkit/components/places/History.sys.mjs

@@ -661,8 +661,13 @@ export var History = Object.freeze({
    * Throw if an object is not a Date object.
    */
   ensureDate(arg) {
-    if (!arg || typeof arg != "object" || arg.constructor.name != "Date") {
-      throw new TypeError("Expected a Date, got " + arg);
+    if (
+      !arg ||
+      typeof arg != "object" ||
+      arg.constructor.name != "Date" ||
+      isNaN(arg)
+    ) {
+      throw new TypeError("Expected a valid Date, got " + arg);
     }
   },
 

toolkit/components/places/PlacesUtils.sys.mjs

@@ -185,8 +185,10 @@ const BOOKMARK_VALIDATORS = Object.freeze({
   index: simpleValidateFunc(
     v => Number.isInteger(v) && v >= PlacesUtils.bookmarks.DEFAULT_INDEX
   ),
-  dateAdded: simpleValidateFunc(v => v.constructor.name == "Date"),
-  lastModified: simpleValidateFunc(v => v.constructor.name == "Date"),
+  dateAdded: simpleValidateFunc(v => v.constructor.name == "Date" && !isNaN(v)),
+  lastModified: simpleValidateFunc(
+    v => v.constructor.name == "Date" && !isNaN(v)
+  ),
   type: simpleValidateFunc(
     v =>
       Number.isInteger(v) &&
@@ -527,7 +529,10 @@ export var PlacesUtils = {
    * @return microseconds from the epoch.
    */
   toPRTime(date) {
-    if (typeof date != "number" && date.constructor.name != "Date") {
+    if (
+      (typeof date != "number" && date.constructor.name != "Date") ||
+      isNaN(date)
+    ) {
       throw new Error("Invalid value passed to toPRTime");
     }
     return date * 1000;
@@ -541,7 +546,7 @@ export var PlacesUtils = {
    * @return a Date object.
    */
   toDate(time) {
-    if (typeof time != "number") {
+    if (typeof time != "number" || isNaN(time)) {
       throw new Error("Invalid value passed to toDate");
     }
     return new Date(parseInt(time / 1000));

toolkit/components/places/tests/bookmarks/test_bookmarks_insert.js

@@ -62,6 +62,10 @@ add_task(async function invalid_input_throws() {
     () => PlacesUtils.bookmarks.insert({ dateAdded: Date.now() }),
     /Invalid value for property 'dateAdded'/
   );
+  Assert.throws(
+    () => PlacesUtils.bookmarks.insert({ dateAdded: new Date(NaN) }),
+    /Invalid value for property 'dateAdded'/
+  );
 
   Assert.throws(
     () => PlacesUtils.bookmarks.insert({ lastModified: -10 }),
@@ -75,9 +79,12 @@ add_task(async function invalid_input_throws() {
     () => PlacesUtils.bookmarks.insert({ lastModified: Date.now() }),
     /Invalid value for property 'lastModified'/
   );
-  let time = new Date();
+  Assert.throws(
+    () => PlacesUtils.bookmarks.insert({ lastModified: new Date(NaN) }),
+    /Invalid value for property 'lastModified'/
+  );
 
-  let past = new Date(time - 86400000);
+  let past = new Date(Date.now() - 86400000);
   Assert.throws(
     () => PlacesUtils.bookmarks.insert({ lastModified: past }),
     /Invalid value for property 'lastModified'/

toolkit/components/places/tests/bookmarks/test_bookmarks_insertTree.js

@@ -60,6 +60,11 @@ add_task(async function invalid_input_rejects() {
     () => PlacesUtils.bookmarks.insertTree(tree),
     /Invalid value for property 'dateAdded'/
   );
+  tree.children = [{ dateAdded: new Date(NaN) }];
+  await Assert.throws(
+    () => PlacesUtils.bookmarks.insertTree(tree),
+    /Invalid value for property 'dateAdded'/
+  );
 
   tree.children = [{ lastModified: -10 }];
   await Assert.throws(
@@ -77,7 +82,7 @@ add_task(async function invalid_input_rejects() {
     /Invalid value for property 'lastModified'/
   );
 
-  let time = new Date();
+  let time = Date.now();
   let future = new Date(time + 86400000);
   tree.children = [{ dateAdded: future, lastModified: time }];
   await Assert.throws(

toolkit/components/places/tests/history/test_removeByFilter.js

@@ -355,11 +355,15 @@ add_task(async function test_error_cases() {
   );
   Assert.throws(
     () => PlacesUtils.history.removeVisitsByFilter({ beginDate: "now" }),
-    /TypeError: Expected a Date/
+    /TypeError: Expected a valid Date/
   );
   Assert.throws(
     () => PlacesUtils.history.removeByFilter({ beginDate: Date.now() }),
-    /TypeError: Expected a Date/
+    /TypeError: Expected a valid Date/
+  );
+  Assert.throws(
+    () => PlacesUtils.history.removeByFilter({ beginDate: new Date(NaN) }),
+    /TypeError: Expected a valid Date/
   );
   Assert.throws(
     () =>

toolkit/components/places/tests/history/test_removeVisitsByFilter.js

@@ -302,11 +302,16 @@ add_task(async function test_error_cases() {
   );
   Assert.throws(
     () => PlacesUtils.history.removeVisitsByFilter({ beginDate: "now" }),
-    /TypeError: Expected a Date/
+    /TypeError: Expected a valid Date/
   );
   Assert.throws(
     () => PlacesUtils.history.removeVisitsByFilter({ beginDate: Date.now() }),
-    /TypeError: Expected a Date/
+    /TypeError: Expected a valid Date/
+  );
+  Assert.throws(
+    () =>
+      PlacesUtils.history.removeVisitsByFilter({ beginDate: new Date(NaN) }),
+    /TypeError: Expected a valid Date/
   );
   Assert.throws(
     () =>

toolkit/components/places/tests/unit/test_frecency_origins_recalc.js

@@ -40,9 +40,9 @@ add_task(async function test() {
   );
 
   // Remove only one visit (otherwise the page would be orphaned).
-  await PlacesUtils.history.removeByFilter({
-    beginDate: new Date(now - 10000),
-    endDate: new Date(now + 10000),
+  await PlacesUtils.history.removeVisitsByFilter({
+    beginDate: new Date(now.valueOf() - 10000),
+    endDate: new Date(now.valueOf() + 10000),
   });
   Assert.equal(
     (

toolkit/components/places/tests/unit/test_utils_timeConversion.js

@@ -0,0 +1,51 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Check time conversion utils.
+ */
+
+add_task(async function toDate() {
+  Assert.throws(() => PlacesUtils.toDate(), /Invalid value/, "Should throw");
+  Assert.throws(() => PlacesUtils.toDate(NaN), /Invalid value/, "Should throw");
+  Assert.throws(
+    () => PlacesUtils.toDate(null),
+    /Invalid value/,
+    "Should throw"
+  );
+  Assert.throws(() => PlacesUtils.toDate("1"), /Invalid value/, "Should throw");
+
+  const now = Date.now();
+  const usecs = now * 1000;
+  Assert.deepEqual(PlacesUtils.toDate(usecs), new Date(now));
+});
+
+add_task(async function toPRTime() {
+  Assert.throws(() => PlacesUtils.toPRTime(), /TypeError/, "Should throw");
+  Assert.throws(() => PlacesUtils.toPRTime(null), /TypeError/, "Should throw");
+  Assert.throws(
+    () => PlacesUtils.toPRTime({}),
+    /Invalid value/,
+    "Should throw"
+  );
+  Assert.throws(
+    () => PlacesUtils.toPRTime(NaN),
+    /Invalid value/,
+    "Should throw"
+  );
+  Assert.throws(
+    () => PlacesUtils.toPRTime(new Date(NaN)),
+    /Invalid value/,
+    "Should throw"
+  );
+  Assert.throws(
+    () => PlacesUtils.toPRTime(new URL("https://test.moz")),
+    /Invalid value/,
+    "Should throw"
+  );
+
+  const now = Date.now();
+  const usecs = now * 1000;
+  Assert.strictEqual(PlacesUtils.toPRTime(now), usecs);
+  Assert.strictEqual(PlacesUtils.toPRTime(new Date(now)), usecs);
+});

toolkit/components/places/tests/unit/xpcshell.ini

@@ -110,5 +110,6 @@ support-files = noRoot.sqlite
 [test_utils_backups_create.js]
 [test_utils_backups_hasRecent.js]
 [test_utils_getURLsForContainerNode.js]
+[test_utils_timeConversion.js]
 [test_visitsInDB.js]
 [test_get_query_param_sql_function.js]