core(driver): security errors are no longer a fatal or pageload error (…

…#8865)
GoogleChrome · Jun 3, 2019 · 93387cd · 93387cd
1 parent f8552d5
commit 93387cd
Show file tree

Hide file tree

Showing 3 changed files with 3 additions and 161 deletions.
diff --git a/lighthouse-cli/test/smokehouse/error-expectations.js b/lighthouse-cli/test/smokehouse/error-expectations.js
@@ -18,10 +18,10 @@ module.exports = [
     },
   },
   {
-    errorCode: 'INSECURE_DOCUMENT_REQUEST',
+    errorCode: undefined,
     lhr: {
       requestedUrl: 'https://expired.badssl.com',
-      finalUrl: 'https://expired.badssl.com',
+      finalUrl: 'https://expired.badssl.com/',
       audits: {},
     },
   },

diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js
@@ -903,53 +903,6 @@ class Driver {
     };
   }
 
-  /**
-   * Return a promise that resolves when an insecure security state is encountered
-   * and a method to cancel internal listeners.
-   * @return {{promise: Promise<string>, cancel: function(): void}}
-   * @private
-   */
-  _monitorForInsecureState() {
-    /** @type {(() => void)} */
-    let cancel = () => {
-      throw new Error('_monitorForInsecureState.cancel() called before it was defined');
-    };
-
-    const promise = new Promise((resolve, reject) => {
-      /**
-       * @param {LH.Crdp.Security.SecurityStateChangedEvent} event
-       */
-      const securityStateChangedListener = ({
-        securityState,
-        explanations,
-        schemeIsCryptographic,
-      }) => {
-        if (securityState === 'insecure' && schemeIsCryptographic) {
-          cancel();
-          const insecureDescriptions = explanations
-            .filter(exp => exp.securityState === 'insecure')
-            .map(exp => exp.description);
-          resolve(insecureDescriptions.join(' '));
-        }
-      };
-      let canceled = false;
-      cancel = () => {
-        if (canceled) return;
-        canceled = true;
-        this.off('Security.securityStateChanged', securityStateChangedListener);
-        // TODO(@patrickhulce): cancel() should really be a promise itself to handle things like this
-        this.sendCommand('Security.disable').catch(() => {});
-      };
-      this.on('Security.securityStateChanged', securityStateChangedListener);
-      this.sendCommand('Security.enable').catch(() => {});
-    });
-
-    return {
-      promise,
-      cancel,
-    };
-  }
-
   /**
    * Returns whether the page appears to be hung.
    * @return {Promise<boolean>}
@@ -1001,13 +954,6 @@ class Driver {
     // CPU listener. Resolves when the CPU has been idle for cpuQuietThresholdMs after network idle.
     let waitForCPUIdle = this._waitForNothing();
 
-    const monitorForInsecureState = this._monitorForInsecureState();
-    const securityCheckPromise = monitorForInsecureState.promise.then(securityMessages => {
-      return function() {
-        throw new LHError(LHError.errors.INSECURE_DOCUMENT_REQUEST, {securityMessages});
-      };
-    });
-
     // Wait for both load promises. Resolves on cleanup function the clears load
     // timeout timer.
     const loadPromise = Promise.all([
@@ -1044,9 +990,8 @@ class Driver {
       };
     });
 
-    // Wait for security issue, load or timeout and run the cleanup function the winner returns.
+    // Wait for load or timeout and run the cleanup function the winner returns.
     const cleanupFn = await Promise.race([
-      securityCheckPromise,
       loadPromise,
       maxTimeoutPromise,
     ]);
@@ -1056,7 +1001,6 @@ class Driver {
     waitForLoadEvent.cancel();
     waitForNetworkIdle.cancel();
     waitForCPUIdle.cancel();
-    monitorForInsecureState.cancel();
 
     await cleanupFn();
   }

diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js
@@ -675,108 +675,6 @@ describe('.gotoURL', () => {
       // Make sure we still cleaned up our listeners
       expect(driver._waitForLoadEvent.getMockCancelFn()).toHaveBeenCalled();
     });
-
-    it('does not reject when page is secure', async () => {
-      const secureSecurityState = {
-        explanations: [],
-        securityState: 'secure',
-      };
-
-      driver.on = driver.once = createMockOnceFn()
-        .mockEvent('Security.securityStateChanged', secureSecurityState);
-
-      const startUrl = 'https://www.example.com';
-      const loadOptions = {
-        waitForLoad: true,
-        passContext: {
-          settings: {
-            maxWaitForLoad: 1,
-          },
-        },
-      };
-
-      const loadPromise = driver.gotoURL(startUrl, loadOptions);
-      await flushAllTimersAndMicrotasks();
-      await loadPromise;
-    });
-
-    it('does not reject when page is insecure but http', async () => {
-      const secureSecurityState = {
-        explanations: [],
-        securityState: 'insecure',
-        schemeIsCryptographic: false,
-      };
-
-      driver.on = driver.once = createMockOnceFn()
-        .mockEvent('Security.securityStateChanged', secureSecurityState);
-
-      const startUrl = 'https://www.example.com';
-      const loadOptions = {
-        waitForLoad: true,
-        passContext: {
-          settings: {
-            maxWaitForLoad: 1,
-          },
-        },
-      };
-
-      const loadPromise = driver.gotoURL(startUrl, loadOptions);
-      await flushAllTimersAndMicrotasks();
-      await loadPromise;
-    });
-
-    it('rejects when page is insecure', async () => {
-      const insecureSecurityState = {
-        explanations: [
-          {
-            description: 'reason 1.',
-            securityState: 'insecure',
-          },
-          {
-            description: 'blah.',
-            securityState: 'info',
-          },
-          {
-            description: 'reason 2.',
-            securityState: 'insecure',
-          },
-        ],
-        securityState: 'insecure',
-        schemeIsCryptographic: true,
-      };
-
-      driver.on = driver.once = createMockOnceFn();
-
-      const startUrl = 'https://www.example.com';
-      const loadOptions = {
-        waitForLoad: true,
-        passContext: {
-          passConfig: {
-            networkQuietThresholdMs: 1,
-          },
-        },
-      };
-
-      // 2 assertions in the catch block and the 1 implicit in `findListener`
-      expect.assertions(3);
-
-      try {
-        const loadPromise = driver.gotoURL(startUrl, loadOptions);
-        await flushAllTimersAndMicrotasks();
-
-        // Use `findListener` instead of `mockEvent` so we can control exactly when the promise resolves
-        const listener = driver.on.findListener('Security.securityStateChanged');
-        listener(insecureSecurityState);
-        await flushAllTimersAndMicrotasks();
-        await loadPromise;
-      } catch (err) {
-        expect(err).toHaveProperty('code', 'INSECURE_DOCUMENT_REQUEST');
-        expect(err.friendlyMessage).toBeDisplayString(
-          'The URL you have provided does not have a valid security certificate. ' +
-          'reason 1. reason 2.'
-        );
-      }
-    });
   });
 });