Share 'no fail on server issues' logic from JA3 tests with lintcert tests

pimterry · pimterry · commit ae41416183e2 · 2022-08-12T17:01:11.000+02:00
diff --git a/test/ca.spec.ts b/test/ca.spec.ts
@@ -2,7 +2,7 @@ import * as https from 'https';
 import * as path from 'path';
 import * as forge from 'node-forge';
 
-import { expect, fetch, nodeOnly } from "./test-utils";
+import { expect, fetch, ignoreNetworkError, nodeOnly } from "./test-utils";
 import * as fs from '../src/util/fs';
 
 import { CA, generateCACertificate } from '../src/util/tls';
@@ -60,17 +60,18 @@ nodeOnly(() => {
         });
 
         it("should be able to generate a CA certificate that passes lintcert checks", async function () {
-            this.retries(3); // Lintcert can have intermittent connectivity blips
-
             const caCertificate = await caCertificatePromise;
 
             const { cert } = caCertificate;
 
-            const response = await fetch('https://crt.sh/lintcert', {
-                method: 'POST',
-                headers: { 'content-type': 'application/x-www-form-urlencoded' },
-                body: new URLSearchParams({'b64cert': cert})
-            });
+            const response = await ignoreNetworkError(
+                fetch('https://crt.sh/lintcert', {
+                    method: 'POST',
+                    headers: { 'content-type': 'application/x-www-form-urlencoded' },
+                    body: new URLSearchParams({'b64cert': cert})
+                }),
+                { context: this }
+            );
 
             const lintOutput = await response.text();
 
@@ -88,7 +89,6 @@ nodeOnly(() => {
 
         it("should generate CA certs that can be used to create domain certs that pass lintcert checks", async function () {
             this.timeout(5000); // Large cert + remote request can make this slow
-            this.retries(3); // Lintcert can have intermittent connectivity blips
 
             const caCertificate = await caCertificatePromise;
             const ca = new CA(caCertificate.key, caCertificate.cert, 2048);
@@ -99,11 +99,14 @@ nodeOnly(() => {
             const certData = forge.pki.certificateFromPem(cert);
             expect((certData.getExtension('subjectAltName') as any).altNames[0].value).to.equal('httptoolkit.tech');
 
-            const response = await fetch('https://crt.sh/lintcert', {
-                method: 'POST',
-                headers: { 'content-type': 'application/x-www-form-urlencoded' },
-                body: new URLSearchParams({'b64cert': cert})
-            });
+            const response = await ignoreNetworkError(
+                fetch('https://crt.sh/lintcert', {
+                    method: 'POST',
+                    headers: { 'content-type': 'application/x-www-form-urlencoded' },
+                    body: new URLSearchParams({'b64cert': cert})
+                }),
+                { context: this }
+            );
 
             expect(response.status).to.equal(200);
             const lintOutput = await response.text();
@@ -129,7 +132,6 @@ nodeOnly(() => {
 
         it("should generate wildcard certs that pass lintcert checks for invalid subdomain names", async function () {
             this.timeout(5000); // Large cert + remote request can make this slow
-            this.retries(3); // Lintcert can have intermittent connectivity blips
 
             const caCertificate = await caCertificatePromise;
             const ca = new CA(caCertificate.key, caCertificate.cert, 2048);
@@ -139,11 +141,14 @@ nodeOnly(() => {
             const certData = forge.pki.certificateFromPem(cert);
             expect((certData.getExtension('subjectAltName') as any).altNames[0].value).to.equal('*.httptoolkit.tech');
 
-            const response = await fetch('https://crt.sh/lintcert', {
-                method: 'POST',
-                headers: { 'content-type': 'application/x-www-form-urlencoded' },
-                body: new URLSearchParams({'b64cert': cert})
-            });
+            const response = await ignoreNetworkError(
+                fetch('https://crt.sh/lintcert', {
+                    method: 'POST',
+                    headers: { 'content-type': 'application/x-www-form-urlencoded' },
+                    body: new URLSearchParams({'b64cert': cert})
+                }),
+                { context: this }
+            );
 
             expect(response.status).to.equal(200);
             const lintOutput = await response.text();
diff --git a/test/integration/proxy.spec.ts b/test/integration/proxy.spec.ts
@@ -23,7 +23,7 @@ import {
     DestroyableServer,
     H2_TLS_ON_TLS_SUPPORTED,
     OLD_TLS_SUPPORTED,
-    delay
+    ignoreNetworkError
 } from "../test-utils";
 import { CA } from "../../src/util/tls";
 import { isLocalIPv6Available } from "../../src/util/socket-util";
@@ -1039,24 +1039,16 @@ nodeOnly(() => {
 
                     await server.forAnyRequest().thenPassThrough();
 
-                    let response = await Promise.race([
+                    let response = await ignoreNetworkError( // External service, can be unreliable, c'est la vie
                         request.get("https://ja3er.com/json", {
                             headers: {
                                 // The hash will get recorded with the user agent that's used - we don't want the database
                                 // to fill up with records that make it clear it's Mockttp's fingerprint!
                                 'user-agent': 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:103.0) Gecko/20100101 Firefox/103.0'
                             }
                         }),
-                        delay(4000).then(() => { throw new Error('timeout'); })
-                    ]).catch(e => e);
-
-                    if (response instanceof Error) {
-                        // ja3er.com is often unavailable. This is annoying but hard to avoid. To handle
-                        // this for now, we just skip the test if the server is unavailable. We only
-                        // fail when we get a real response with a bad fingerprint.
-                        console.warn('Skipping JA3 test due to network error:', response);
-                        return this.skip();
-                    }
+                        { context: this, timeout: 4000 }
+                    );
 
                     const ja3Hash = JSON.parse(response).ja3_hash;
 
diff --git a/test/test-utils.ts b/test/test-utils.ts
@@ -13,6 +13,7 @@ import {
     FormData as FormDataPolyfill,
     File as FilePolyfill
 } from "formdata-node";
+import { RequestPromise } from 'request-promise-native';
 
 import chai = require("chai");
 import chaiAsPromised = require("chai-as-promised");
@@ -87,6 +88,38 @@ export function nodeOnly(body: Function) {
     if (isNode) body();
 }
 
+// Wrap a test promise that might fail due to irrelevant remote network issues, and it'll skip the test
+// if there's a timeout or 502 response (but still throw any other errors). This allows us to write tests
+// that will fail if a remote server explicitly rejects something, but make them resilient to the remote
+// server simply being entirely unavailable.
+export async function ignoreNetworkError<T extends RequestPromise | Promise<Response>>(request: T, options: {
+    context: Mocha.Context,
+    timeout?: number
+}): Promise<T> {
+    const TimeoutError = new Error('timeout');
+
+    const result = await Promise.race([
+        request.catch(e => e),
+        delay(options.timeout ?? 1000).then(() => { throw TimeoutError })
+    ]).catch(error => {
+        console.log(error);
+        if (error === TimeoutError) {
+            console.warn(`Skipping test due to network error: ${error.message || error}`);
+            if ('abort' in request) request.abort();
+            throw options.context.skip();
+        } else {
+            throw error;
+        }
+    });
+
+    if ((result as any).status === 502) {
+        console.warn('Skipping test due to remote 502 response');
+        throw options.context.skip();
+    }
+
+    return result;
+}
+
 const TOO_LONG_HEADER_SIZE = 1024 * (isNode ? 16 : 160) + 1;
 export const TOO_LONG_HEADER_VALUE = _.range(TOO_LONG_HEADER_SIZE).map(() => "X").join("");
 
