core(lightwallet): stricter verification of budget types (#8727)

lighthouse-core/config/budget.js

@@ -5,10 +5,36 @@
  */
 'use strict';
 
+/**
+ * @param {unknown} arr
+ * @return {arr is Array<Record<string, unknown>>}
+ */
+function isArrayOfUnknownObjects(arr) {
+  return Array.isArray(arr) && arr.every(isObjectOfUnknownProperties);
+}
+
+/**
+ * @param {unknown} val
+ * @return {val is Record<string, unknown>}
+ */
+function isObjectOfUnknownProperties(val) {
+  return typeof val === 'object' && val !== null && !Array.isArray(val);
+}
+
+/**
+ * Returns whether `val` is numeric. Will not coerce to a number. `NaN` will
+ * return false, however ±Infinity will return true.
+ * @param {unknown} val
+ * @return {val is number}
+ */
+function isNumber(val) {
+  return typeof val === 'number' && !isNaN(val);
+}
+
 class Budget {
   /**
    * Asserts that obj has no own properties, throwing a nice error message if it does.
-   * Plugin and object name are included for nicer logging.
+   * `objectName` is included for nicer logging.
    * @param {Record<string, unknown>} obj
    * @param {string} objectName
    */
@@ -21,13 +47,14 @@ class Budget {
   }
 
   /**
-   * @param {LH.Budget.ResourceBudget} resourceBudget
+   * @param {Record<string, unknown>} resourceBudget
    * @return {LH.Budget.ResourceBudget}
    */
   static validateResourceBudget(resourceBudget) {
     const {resourceType, budget, ...invalidRest} = resourceBudget;
     Budget.assertNoExcessProperties(invalidRest, 'Resource Budget');
 
+    /** @type {Array<LH.Budget.ResourceType>} */
     const validResourceTypes = [
       'total',
       'document',
@@ -39,46 +66,49 @@ class Budget {
       'other',
       'third-party',
     ];
-    if (!validResourceTypes.includes(resourceBudget.resourceType)) {
-      throw new Error(`Invalid resource type: ${resourceBudget.resourceType}. \n` +
+    // Assume resourceType is an allowed string, throw if not.
+    if (!validResourceTypes.includes(/** @type {LH.Budget.ResourceType} */ (resourceType))) {
+      throw new Error(`Invalid resource type: ${resourceType}. \n` +
         `Valid resource types are: ${ validResourceTypes.join(', ') }`);
     }
-    if (isNaN(resourceBudget.budget)) {
-      throw new Error('Invalid budget: ${resourceBudget.budget}');
+    if (!isNumber(budget)) {
+      throw new Error(`Invalid budget: ${budget}`);
     }
     return {
-      resourceType,
+      resourceType: /** @type {LH.Budget.ResourceType} */ (resourceType),
       budget,
     };
   }
 
   /**
-   * @param {LH.Budget.TimingBudget} timingBudget
+   * @param {Record<string, unknown>} timingBudget
    * @return {LH.Budget.TimingBudget}
    */
   static validateTimingBudget(timingBudget) {
     const {metric, budget, tolerance, ...invalidRest} = timingBudget;
     Budget.assertNoExcessProperties(invalidRest, 'Timing Budget');
 
+    /** @type {Array<LH.Budget.TimingMetric>} */
     const validTimingMetrics = [
       'first-contentful-paint',
       'first-cpu-idle',
       'interactive',
       'first-meaningful-paint',
       'estimated-input-latency',
     ];
-    if (!validTimingMetrics.includes(timingBudget.metric)) {
-      throw new Error(`Invalid timing metric: ${timingBudget.metric}. \n` +
+    // Assume metric is an allowed string, throw if not.
+    if (!validTimingMetrics.includes(/** @type {LH.Budget.TimingMetric} */ (metric))) {
+      throw new Error(`Invalid timing metric: ${metric}. \n` +
         `Valid timing metrics are: ${validTimingMetrics.join(', ')}`);
     }
-    if (isNaN(timingBudget.budget)) {
-      throw new Error('Invalid budget: ${timingBudget.budget}');
+    if (!isNumber(budget)) {
+      throw new Error(`Invalid budget: ${budget}`);
     }
-    if (timingBudget.tolerance !== undefined && isNaN(timingBudget.tolerance)) {
-      throw new Error('Invalid tolerance: ${timingBudget.tolerance}');
+    if (typeof tolerance !== 'undefined' && !isNumber(tolerance)) {
+      throw new Error(`Invalid tolerance: ${tolerance}`);
     }
     return {
-      metric,
+      metric: /** @type {LH.Budget.TimingMetric} */ (metric),
       budget,
       tolerance,
     };
@@ -87,43 +117,44 @@ class Budget {
   /**
    * More info on the Budget format:
    * https://github.com/GoogleChrome/lighthouse/issues/6053#issuecomment-428385930
-   * @param {Array<LH.Budget>} budgetArr
+   * @param {unknown} budgetJson
    * @return {Array<LH.Budget>}
    */
-  static initializeBudget(budgetArr) {
-    /** @type {Array<LH.Budget>} */
-    const budgets = [];
+  static initializeBudget(budgetJson) {
+    // Clone to prevent modifications of original and to deactivate any live properties.
+    budgetJson = JSON.parse(JSON.stringify(budgetJson));
+    if (!isArrayOfUnknownObjects(budgetJson)) {
+      throw new Error('Budget file is not defined as an array of budgets.');
+    }
 
-    budgetArr.forEach((b) => {
+    const budgets = budgetJson.map((b, index) => {
       /** @type {LH.Budget} */
       const budget = {};
 
       const {resourceSizes, resourceCounts, timings, ...invalidRest} = b;
       Budget.assertNoExcessProperties(invalidRest, 'Budget');
 
-      if (b.resourceSizes !== undefined) {
-        budget.resourceSizes = b.resourceSizes.map((r) => {
-          return Budget.validateResourceBudget(r);
-        });
+      if (isArrayOfUnknownObjects(resourceSizes)) {
+        budget.resourceSizes = resourceSizes.map(Budget.validateResourceBudget);
+      } else if (resourceSizes !== undefined) {
+        throw new Error(`Invalid resourceSizes entry in budget at index ${index}`);
       }
 
-      if (b.resourceCounts !== undefined) {
-        budget.resourceCounts = b.resourceCounts.map((r) => {
-          return Budget.validateResourceBudget(r);
-        });
+      if (isArrayOfUnknownObjects(resourceCounts)) {
+        budget.resourceCounts = resourceCounts.map(Budget.validateResourceBudget);
+      } else if (resourceCounts !== undefined) {
+        throw new Error(`Invalid resourceCounts entry in budget at index ${index}`);
       }
 
-      if (b.timings !== undefined) {
-        budget.timings = b.timings.map((t) => {
-          return Budget.validateTimingBudget(t);
-        });
+      if (isArrayOfUnknownObjects(timings)) {
+        budget.timings = timings.map(Budget.validateTimingBudget);
+      } else if (timings !== undefined) {
+        throw new Error(`Invalid timings entry in budget at index ${index}`);
       }
-      budgets.push({
-        resourceSizes,
-        resourceCounts,
-        timings,
-      });
+
+      return budget;
     });
+
     return budgets;
   }
 }

lighthouse-core/test/config/budget-test.js

@@ -43,7 +43,7 @@ describe('Budget', () => {
           {
             metric: 'first-contentful-paint',
             budget: 1000,
-            tolerance: 500,
+            // tolerance is optional, so not defined here.
           },
         ],
       },
@@ -74,55 +74,103 @@ describe('Budget', () => {
 
     // Sets timings correctly
     assert.equal(budgets[0].timings.length, 2);
+    assert.deepStrictEqual(budgets[0].timings[0], {
+      metric: 'interactive',
+      budget: 2000,
+      tolerance: 1000,
+    });
     assert.equal(budgets[0].timings[1].metric, 'first-contentful-paint');
     assert.equal(budgets[0].timings[1].budget, 1000);
-    assert.equal(budgets[0].timings[1].tolerance, 500);
 
     // Does not set unsupplied budgets
     assert.equal(budgets[1].timings, null);
   });
 
+  it('accepts an empty array', () => {
+    const budgets = Budget.initializeBudget([]);
+    assert.deepStrictEqual(budgets, []);
+  });
+
   it('throws error if an unsupported budget property is used', () => {
     budget[0].sizes = [];
-    assert.throws(_ => Budget.initializeBudget(budget), /[sizes]/);
+    assert.throws(_ => Budget.initializeBudget(budget),
+      /Budget has unrecognized properties: \[sizes\]/);
+  });
+
+  describe('top-level validation', () => {
+    it('throws when provided an invalid budget array', () => {
+      assert.throws(_ => Budget.initializeBudget(55),
+        /Budget file is not defined as an array of budgets/);
+
+      assert.throws(_ => Budget.initializeBudget(['invalid123']),
+        /Budget file is not defined as an array of budgets/);
+
+      assert.throws(_ => Budget.initializeBudget([null]),
+        /Budget file is not defined as an array of budgets/);
+    });
+
+    it('throws when budget contains invalid resourceSizes entry', () => {
+      budget[0].resourceSizes = 55;
+      assert.throws(_ => Budget.initializeBudget(budget),
+        /^Error: Invalid resourceSizes entry in budget at index 0$/);
+    });
+
+    it('throws when budget contains invalid resourceCounts entry', () => {
+      budget[0].resourceCounts = 'A string';
+      assert.throws(_ => Budget.initializeBudget(budget),
+        /^Error: Invalid resourceCounts entry in budget at index 0$/);
+    });
+
+    it('throws when budget contains invalid timings entry', () => {
+      budget[1].timings = false;
+      assert.throws(_ => Budget.initializeBudget(budget),
+        /^Error: Invalid timings entry in budget at index 1$/);
+    });
   });
 
   describe('resource budget validation', () => {
     it('throws when an invalid resource type is supplied', () => {
       budget[0].resourceSizes[0].resourceType = 'movies';
-      assert.throws(_ => Budget.initializeBudget(budget), /Invalid resource type/);
+      assert.throws(_ => Budget.initializeBudget(budget),
+        // eslint-disable-next-line max-len
+        /Invalid resource type: movies. \nValid resource types are: total, document,/);
     });
 
     it('throws when an invalid budget is supplied', () => {
       budget[0].resourceSizes[0].budget = '100 MB';
-      assert.throws(_ => Budget.initializeBudget(budget), /Invalid budget/);
+      assert.throws(_ => Budget.initializeBudget(budget), /Invalid budget: 100 MB/);
     });
 
     it('throws when an invalid property is supplied', () => {
       budget[0].resourceSizes[0].browser = 'Chrome';
-      assert.throws(_ => Budget.initializeBudget(budget), /[browser]/);
+      assert.throws(_ => Budget.initializeBudget(budget),
+        /Resource Budget has unrecognized properties: \[browser\]/);
     });
   });
 
   describe('timing budget validation', () => {
     it('throws when an invalid metric is supplied', () => {
-      budget[0].timings[0].metric = 'lastMeaningfulPaint';
-      assert.throws(_ => Budget.initializeBudget(budget), /Invalid timing metric/);
+      budget[0].timings[0].metric = 'medianMeaningfulPaint';
+      assert.throws(_ => Budget.initializeBudget(budget),
+        // eslint-disable-next-line max-len
+        /Invalid timing metric: medianMeaningfulPaint. \nValid timing metrics are: first-contentful-paint, /);
     });
 
     it('throws when an invalid budget is supplied', () => {
       budget[0].timings[0].budget = '100KB';
-      assert.throws(_ => Budget.initializeBudget(budget), /Invalid budget/);
+      assert.throws(_ => Budget.initializeBudget(budget), /Invalid budget: 100KB/);
     });
 
     it('throws when an invalid tolerance is supplied', () => {
       budget[0].timings[0].tolerance = '100ms';
-      assert.throws(_ => Budget.initializeBudget(budget), /Invalid tolerance/);
+      assert.throws(_ => Budget.initializeBudget(budget), /Invalid tolerance: 100ms/);
     });
 
     it('throws when an invalid property is supplied', () => {
       budget[0].timings[0].device = 'Phone';
-      assert.throws(_ => Budget.initializeBudget(budget), /[device]/);
+      budget[0].timings[0].location = 'The middle somewhere, I don\'t know';
+      assert.throws(_ => Budget.initializeBudget(budget),
+        /Timing Budget has unrecognized properties: \[device, location\]/);
     });
   });
 });

lighthouse-core/test/config/config-test.js

@@ -761,7 +761,7 @@ describe('Config', () => {
 
       it('should throw when provided an invalid budget', () => {
         assert.throws(() => new Config({settings: {budgets: ['invalid123']}}),
-          /Budget has unrecognized properties/);
+          /Budget file is not defined as an array of budgets/);
       });
     });