diff --git a/lighthouse-cli/test/smokehouse/error-expectations.js b/lighthouse-cli/test/smokehouse/error-expectations.js index 04d804e5a613..55afc640f430 100644 --- 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 index 93f267a8ae85..a07c4d6d6295 100644 --- 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, 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} @@ -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 index d96ee191b1b9..dfe5ed6ba6af 100644 --- 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.' - ); - } - }); }); });