Repository: GoogleChrome/lighthouse
Pull request #12681 (2021-06-22 17:29:01)

core(network-request): consider HSTS redirects secure
**Summary**
Often the HTTPS audit fails because the original URL audited was `http`, so even if it redirects then you still have the original redirect request over HTTP. We can kinda explain why this is desirable (though I'm not really sold it's worth the annoyance), but it's definitely not the case in HSTS internal redirects that happen in the browser, which is the whole point of HSTS. This applies to entire TLDs in some cases (`.app`/`.dev`), so we should handle those cases appropriately.


**Related Issues/PRs**
fixes https://github.com/GoogleChrome/lighthouse/issues/12674



Patch:
diff --git a/lighthouse-core/gather/gatherers/viewport-dimensions.js b/lighthouse-core/gather/gatherers/viewport-dimensions.js
index eebf63893..71df65998 100644
--- a/lighthouse-core/gather/gatherers/viewport-dimensions.js
+++ b/lighthouse-core/gather/gatherers/viewport-dimensions.js
@@ -30,7 +30,7 @@ function getViewportDimensions() {
 class ViewportDimensions extends FRGatherer {
   /** @type {LH.Gatherer.GathererMeta} */
   meta = {
-    supportedModes: ['snapshot', 'navigation'],
+    supportedModes: ['snapshot', 'timespan', 'navigation'],
   }
 
   /**
diff --git a/lighthouse-core/lib/network-request.js b/lighthouse-core/lib/network-request.js
index efbd3e51e..3ed49f21f 100644
--- a/lighthouse-core/lib/network-request.js
+++ b/lighthouse-core/lib/network-request.js
@@ -493,7 +493,23 @@ class NetworkRequest {
   static isSecureRequest(record) {
     return URL.isSecureScheme(record.parsedURL.scheme) ||
         URL.isSecureScheme(record.protocol) ||
-        URL.isLikeLocalhost(record.parsedURL.host);
+        URL.isLikeLocalhost(record.parsedURL.host) ||
+        NetworkRequest.isHstsRequest(record);
+  }
+
+  /**
+   * Returns whether the network request was an HSTS redirect request.
+   * @param {NetworkRequest} record
+   * @return {boolean}
+   */
+  static isHstsRequest(record) {
+    const destination = record.redirectDestination;
+    if (!destination) return false;
+
+    const reasonHeader = record.responseHeaders
+      .find(header => header.name === 'Non-Authoritative-Reason');
+    const reason = reasonHeader && reasonHeader.value;
+    return reason === 'HSTS' && NetworkRequest.isSecureRequest(destination);
   }
 }
 


Test patch:
diff --git a/lighthouse-core/test/config/config-test.js b/lighthouse-core/test/config/config-test.js
index 3d8b473af..e2b43d16e 100644
--- a/lighthouse-core/test/config/config-test.js
+++ b/lighthouse-core/test/config/config-test.js
@@ -1297,7 +1297,7 @@ describe('Config', () => {
 
       const expectedInstance = {
         meta: {
-          supportedModes: ['snapshot', 'navigation'],
+          supportedModes: ['snapshot', 'timespan', 'navigation'],
         },
       };
       assert.deepEqual(mergedJson[0].gatherers,
diff --git a/lighthouse-core/test/fraggle-rock/api-test-pptr.js b/lighthouse-core/test/fraggle-rock/api-test-pptr.js
index ef65f48b0..21a1ec3f6 100644
--- a/lighthouse-core/test/fraggle-rock/api-test-pptr.js
+++ b/lighthouse-core/test/fraggle-rock/api-test-pptr.js
@@ -121,7 +121,7 @@ describe('Fraggle Rock API', () => {
 
       const {auditResults, erroredAudits, failedAudits} = getAuditsBreakdown(lhr);
       // TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
-      expect(auditResults.length).toMatchInlineSnapshot(`38`);
+      expect(auditResults.length).toMatchInlineSnapshot(`40`);
 
       expect(erroredAudits).toHaveLength(0);
       expect(failedAudits.map(audit => audit.id)).toContain('errors-in-console');
diff --git a/lighthouse-core/test/lib/network-request-test.js b/lighthouse-core/test/lib/network-request-test.js
index f1403cb8f..1c926f196 100644
--- a/lighthouse-core/test/lib/network-request-test.js
+++ b/lighthouse-core/test/lib/network-request-test.js
@@ -287,6 +287,11 @@ describe('NetworkRequest', () => {
       expect(isSecureRequest({parsedURL: {scheme: 'http', host: '54.33.21.23'}})).toBe(false);
       expect(isSecureRequest({parsedURL: {scheme: 'ws', host: 'my-service.com'}})).toBe(false);
       expect(isSecureRequest({parsedURL: {scheme: '', host: 'google.com'}})).toBe(false);
+      expect(isSecureRequest({
+        parsedURL: {scheme: 'http', host: 'google.com'},
+        redirectDestination: {parsedURL: {scheme: 'https', host: 'google.com'}},
+        responseHeaders: [],
+      })).toBe(false);
     });
 
     it('correctly identifies secure records', () => {
@@ -300,6 +305,45 @@ describe('NetworkRequest', () => {
       expect(isSecureRequest({parsedURL: {scheme: '', host: ''}, protocol: 'blob'})).toBe(true);
       expect(isSecureRequest({parsedURL: {scheme: 'chrome', host: ''}})).toBe(true);
       expect(isSecureRequest({parsedURL: {scheme: 'chrome-extension', host: ''}})).toBe(true);
+      expect(isSecureRequest({
+        parsedURL: {scheme: 'http', host: 'google.com'},
+        redirectDestination: {parsedURL: {scheme: 'https', host: 'google.com'}},
+        responseHeaders: [{name: 'Non-Authoritative-Reason', value: 'HSTS'}],
+      })).toBe(true);
+    });
+  });
+
+  describe('#isHstsRequest', () => {
+    const isHstsRequest = NetworkRequest.isHstsRequest;
+
+    it('correctly identifies non-HSTS records', () => {
+      // missing a redirect destination
+      expect(isHstsRequest({
+        parsedURL: {scheme: 'http', host: 'google.com'},
+        responseHeaders: [{name: 'Non-Authoritative-Reason', value: 'HSTS'}],
+      })).toBe(false);
+
+      // no HSTS reason
+      expect(isHstsRequest({
+        parsedURL: {scheme: 'http', host: 'google.com'},
+        redirectDestination: {parsedURL: {scheme: 'https', host: 'google.com'}},
+        responseHeaders: [],
+      })).toBe(false);
+
+      // redirects to insecure
+      expect(isHstsRequest({
+        parsedURL: {scheme: 'http', host: 'google.com'},
+        redirectDestination: {parsedURL: {scheme: 'http', host: 'google.com'}},
+        responseHeaders: [{name: 'Non-Authoritative-Reason', value: 'HSTS'}],
+      })).toBe(false);
+    });
+
+    it('correctly identifies HSTS requests', () => {
+      expect(isHstsRequest({
+        parsedURL: {scheme: 'http', host: 'google.com'},
+        redirectDestination: {parsedURL: {scheme: 'https', host: 'google.com'}},
+        responseHeaders: [{name: 'Non-Authoritative-Reason', value: 'HSTS'}],
+      })).toBe(true);
     });
   });
 });