core: bail if encounter insecure ssl cert, to avoid hanging forever

lighthouse-core/gather/driver.js

@@ -849,6 +849,24 @@ class Driver {
     });
   }
 
+  /**
+   * @param {number} [timeout]
+   * @return {Promise<LH.Crdp.Security.SecurityStateChangedEvent>}
+   */
+  getSecurityState(timeout = 1000) {
+    return new Promise((resolve, reject) => {
+      const err = new LHError(LHError.errors.SECURITY_STATE_TIMEOUT);
+      const asyncTimeout = setTimeout((_ => reject(err)), timeout);
+
+      this.sendCommand('Security.enable');
+      this.once('Security.securityStateChanged', (e) => {
+        clearTimeout(asyncTimeout);
+        resolve(e);
+        this.sendCommand('Security.disable').catch(reject);
+      });
+    });
+  }
+
   /**
    * @param {string} name The name of API whose permission you wish to query
    * @return {Promise<string>} The state of permissions, resolved in a promise.

lighthouse-core/gather/gather-runner.js

@@ -146,11 +146,12 @@ class GatherRunner {
 
   /**
    * Returns an error if the original network request failed or wasn't found.
+   * @param {Driver} driver
    * @param {string} url The URL of the original requested page.
    * @param {Array<LH.Artifacts.NetworkRequest>} networkRecords
-   * @return {LHError|undefined}
+   * @return {Promise<LHError|undefined>}
    */
-  static getPageLoadError(url, networkRecords) {
+  static async getPageLoadError(driver, url, networkRecords) {
     const mainRecord = networkRecords.find(record => {
       // record.url is actual request url, so needs to be compared without any URL fragment.
       return URL.equalWithExcludedFragments(record.url, url);
@@ -165,6 +166,20 @@ class GatherRunner {
     } else if (mainRecord.hasErrorStatusCode()) {
       errorDef = {...LHError.errors.ERRORED_DOCUMENT_REQUEST};
       errorDef.message += ` Status code: ${mainRecord.statusCode}.`;
+    } else if (!mainRecord.finished) {
+      // could be security error
+      const securityState = await driver.getSecurityState();
+      if (securityState.securityState === 'insecure') {
+        errorDef = {...LHError.errors.INSECURE_DOCUMENT_REQUEST};
+        const insecureDescriptions = securityState.explanations
+          .filter(exp => exp.securityState === 'insecure')
+          .map(exp => exp.description)
+          .join(' ');
+        errorDef.message += ` Insecure: ${insecureDescriptions}`;
+      } else {
+        // Not sure what the error is. Could be just a redirect. For now,
+        // treat as no error.
+      }
     }
 
     if (errorDef) {
@@ -251,7 +266,7 @@ class GatherRunner {
    * object containing trace and network data.
    * @param {LH.Gatherer.PassContext} passContext
    * @param {Partial<GathererResults>} gathererResults
-   * @return {Promise<LH.Gatherer.LoadData>}
+   * @return {Promise<[LH.Gatherer.LoadData, LH.LighthouseError | undefined]>}
    */
   static async afterPass(passContext, gathererResults) {
     const driver = passContext.driver;
@@ -271,7 +286,8 @@ class GatherRunner {
     const networkRecords = NetworkRecorder.recordsFromLogs(devtoolsLog);
     log.verbose('statusEnd', status);
 
-    let pageLoadError = GatherRunner.getPageLoadError(passContext.url, networkRecords);
+    let pageLoadError = await GatherRunner.getPageLoadError(driver,
+      passContext.url, networkRecords);
     // If the driver was offline, a page load error is expected, so do not save it.
     if (!driver.online) pageLoadError = undefined;
 
@@ -288,8 +304,12 @@ class GatherRunner {
       trace,
     };
 
-    // Disable throttling so the afterPass analysis isn't throttled
-    await driver.setThrottling(passContext.settings, {useThrottling: false});
+    if (!pageLoadError) {
+      // Disable throttling so the afterPass analysis isn't throttled
+      // This will hang if there was a security error. But, there is no
+      // need to throttle if there is such an error. See #6287
+      await driver.setThrottling(passContext.settings, {useThrottling: false});
+    }
 
     for (const gathererDefn of gatherers) {
       const gatherer = gathererDefn.instance;
@@ -313,7 +333,7 @@ class GatherRunner {
     }
 
     // Resolve on tracing data using passName from config.
-    return passData;
+    return [passData, pageLoadError];
   }
 
   /**
@@ -410,7 +430,8 @@ class GatherRunner {
         await driver.setThrottling(options.settings, passConfig);
         await GatherRunner.beforePass(passContext, gathererResults);
         await GatherRunner.pass(passContext, gathererResults);
-        const passData = await GatherRunner.afterPass(passContext, gathererResults);
+        const [passData, pageLoadError] =
+          await GatherRunner.afterPass(passContext, gathererResults);
 
         // Save devtoolsLog, but networkRecords are discarded and not added onto artifacts.
         baseArtifacts.devtoolsLogs[passConfig.passName] = passData.devtoolsLog;
@@ -435,6 +456,11 @@ class GatherRunner {
           baseArtifacts.URL.finalUrl = passContext.url;
           firstPass = false;
         }
+
+        if (pageLoadError && pageLoadError.code === LHError.errors.INSECURE_DOCUMENT_REQUEST.code) {
+          // Some protocol commands will hang, so let's just bail. See #6287
+          break;
+        }
       }
       const resetStorage = !options.settings.disableStorageReset;
       if (resetStorage) await driver.clearDataForOrigin(options.requestedUrl);

lighthouse-core/lib/lh-error.js

@@ -142,6 +142,12 @@ const ERRORS = {
     message: strings.pageLoadFailed,
     lhrRuntimeError: true,
   },
+  /* Used when security error prevents page load. */
+  INSECURE_DOCUMENT_REQUEST: {
+    code: 'INSECURE_DOCUMENT_REQUEST',
+    message: strings.pageLoadFailedInsecure,
+    lhrRuntimeError: true,
+  },
 
   // Protocol internal failures
   TRACING_ALREADY_STARTED: {
@@ -169,6 +175,12 @@ const ERRORS = {
     message: strings.requestContentTimeout,
   },
 
+  // Protocol timeout failures
+  SECURITY_STATE_TIMEOUT: {
+    code: 'SECURITY_STATE_TIMEOUT',
+    message: strings.securityStateTimeout,
+  },
+
   // URL parsing failures
   INVALID_URL: {
     code: 'INVALID_URL',

lighthouse-core/lib/strings.js

@@ -11,7 +11,9 @@ module.exports = {
   badTraceRecording: `Something went wrong with recording the trace over your page load. Please run Lighthouse again.`,
   pageLoadTookTooLong: `Your page took too long to load. Please follow the opportunities in the report to reduce your page load time, and then try re-running Lighthouse.`,
   pageLoadFailed: `Lighthouse was unable to reliably load the page you requested. Make sure you are testing the correct URL and that the server is properly responding to all requests.`,
+  pageLoadFailedInsecure: `The URL you have provided does not have a valid security certificate.`,
   internalChromeError: `An internal Chrome error occurred. Please restart Chrome and try re-running Lighthouse.`,
   requestContentTimeout: 'Fetching resource content has exceeded the allotted time',
+  securityStateTimeout: 'Fetching security state has exceeded the allotted time',
   urlInvalid: `The URL you have provided appears to be invalid.`,
 };

lighthouse-core/test/gather/fake-driver.js

@@ -81,6 +81,11 @@ const fakeDriver = {
   setExtraHTTPHeaders() {
     return Promise.resolve();
   },
+  getSecurityState() {
+    return Promise.resolve({
+      securityState: 'secure',
+    });
+  },
 };
 
 module.exports = fakeDriver;

lighthouse-core/test/gather/gather-runner-test.js

@@ -493,10 +493,11 @@ describe('GatherRunner', function() {
       ],
     };
 
-    return GatherRunner.afterPass({url, driver, passConfig}, {TestGatherer: []}).then(passData => {
-      assert.equal(calledTrace, true);
-      assert.equal(passData.trace, fakeTraceData);
-    });
+    return GatherRunner.afterPass({url, driver, passConfig}, {TestGatherer: []})
+      .then(([passData]) => {
+        assert.equal(calledTrace, true);
+        assert.equal(passData.trace, fakeTraceData);
+      });
   });
 
   it('tells the driver to begin devtoolsLog collection', () => {
@@ -543,10 +544,11 @@ describe('GatherRunner', function() {
       ],
     };
 
-    return GatherRunner.afterPass({url, driver, passConfig}, {TestGatherer: []}).then(vals => {
-      assert.equal(calledDevtoolsLogCollect, true);
-      assert.strictEqual(vals.devtoolsLog[0], fakeDevtoolsMessage);
-    });
+    return GatherRunner.afterPass({url, driver, passConfig}, {TestGatherer: []})
+      .then(([passData]) => {
+        assert.equal(calledDevtoolsLogCollect, true);
+        assert.strictEqual(passData.devtoolsLog[0], fakeDevtoolsMessage);
+      });
   });
 
   it('does as many passes as are required', () => {
@@ -625,58 +627,89 @@ describe('GatherRunner', function() {
   });
 
   describe('#getPageLoadError', () => {
-    it('passes when the page is loaded', () => {
+    it('passes when the page is loaded', async () => {
       const url = 'http://the-page.com';
       const mainRecord = new NetworkRequest();
       mainRecord.url = url;
-      assert.ok(!GatherRunner.getPageLoadError(url, [mainRecord]));
+      assert.ok(!await GatherRunner.getPageLoadError(fakeDriver, url, [mainRecord]));
     });
 
-    it('passes when the page is loaded, ignoring any fragment', () => {
+    it('passes when the page is loaded, ignoring any fragment', async () => {
       const url = 'http://example.com/#/page/list';
       const mainRecord = new NetworkRequest();
       mainRecord.url = 'http://example.com';
-      assert.ok(!GatherRunner.getPageLoadError(url, [mainRecord]));
+      assert.ok(!await GatherRunner.getPageLoadError(fakeDriver, url, [mainRecord]));
     });
 
-    it('fails when page fails to load', () => {
+    it('fails when page fails to load', async () => {
       const url = 'http://the-page.com';
       const mainRecord = new NetworkRequest();
       mainRecord.url = url;
       mainRecord.failed = true;
       mainRecord.localizedFailDescription = 'foobar';
-      const error = GatherRunner.getPageLoadError(url, [mainRecord]);
+      const error = await GatherRunner.getPageLoadError(fakeDriver, url, [mainRecord]);
       assert.equal(error.message, 'FAILED_DOCUMENT_REQUEST');
       assert.ok(/^Lighthouse was unable to reliably load/.test(error.friendlyMessage));
     });
 
-    it('fails when page times out', () => {
+    it('fails when page times out', async () => {
       const url = 'http://the-page.com';
       const records = [];
-      const error = GatherRunner.getPageLoadError(url, records);
+      const error = await GatherRunner.getPageLoadError(fakeDriver, url, records);
       assert.equal(error.message, 'NO_DOCUMENT_REQUEST');
       assert.ok(/^Lighthouse was unable to reliably load/.test(error.friendlyMessage));
     });
 
-    it('fails when page returns with a 404', () => {
+    it('fails when page returns with a 404', async () => {
       const url = 'http://the-page.com';
       const mainRecord = new NetworkRequest();
       mainRecord.url = url;
       mainRecord.statusCode = 404;
-      const error = GatherRunner.getPageLoadError(url, [mainRecord]);
+      const error = await GatherRunner.getPageLoadError(fakeDriver, url, [mainRecord]);
       assert.equal(error.message, 'ERRORED_DOCUMENT_REQUEST');
       assert.ok(/^Lighthouse was unable to reliably load/.test(error.friendlyMessage));
     });
 
-    it('fails when page returns with a 500', () => {
+    it('fails when page returns with a 500', async () => {
       const url = 'http://the-page.com';
       const mainRecord = new NetworkRequest();
       mainRecord.url = url;
       mainRecord.statusCode = 500;
-      const error = GatherRunner.getPageLoadError(url, [mainRecord]);
+      const error = await GatherRunner.getPageLoadError(fakeDriver, url, [mainRecord]);
       assert.equal(error.message, 'ERRORED_DOCUMENT_REQUEST');
       assert.ok(/^Lighthouse was unable to reliably load/.test(error.friendlyMessage));
     });
+
+    it('fails when page is insecure', async () => {
+      const mockDriver = Object.assign({}, fakeDriver, {
+        async getSecurityState() {
+          return {
+            explanations: [
+              {
+                description: 'reason 1.',
+                securityState: 'insecure',
+              },
+              {
+                description: 'blah.',
+                securityState: 'info',
+              },
+              {
+                description: 'reason 2.',
+                securityState: 'insecure',
+              },
+            ],
+            securityState: 'insecure',
+          };
+        },
+      });
+      const url = 'http://the-page.com';
+      const mainRecord = new NetworkRequest();
+      mainRecord.url = url;
+      const error = await GatherRunner.getPageLoadError(mockDriver, url, [mainRecord]);
+      assert.equal(error.message, 'INSECURE_DOCUMENT_REQUEST');
+      /* eslint-disable-next-line max-len */
+      assert.equal(error.friendlyMessage, 'The URL you have provided does not have a valid security certificate. Insecure: reason 1. reason 2.');
+    });
   });
 
   describe('artifact collection', () => {