✨ Update proxy.js to auto strip double quotes/spaces if any (#1283)

packages/client/src/proxy.js

@@ -3,6 +3,7 @@ import tls from 'tls';
 import http from 'http';
 import https from 'https';
 import logger from '@percy/logger';
+import { stripQuotesAndSpaces } from '@percy/env/utils';
 
 const CRLF = '\r\n';
 const STATUS_REG = /^HTTP\/1.[01] (\d*)/;
@@ -65,9 +66,11 @@ export function getProxy(options) {
     (process.env.https_proxy || process.env.HTTPS_PROXY)) ||
     (process.env.http_proxy || process.env.HTTP_PROXY);
 
-  let shouldProxy = !!proxyUrl && !hostnameMatches((
-    process.env.no_proxy || process.env.NO_PROXY
-  ), href(options));
+  let shouldProxy = !!proxyUrl && !hostnameMatches(
+    stripQuotesAndSpaces(process.env.no_proxy || process.env.NO_PROXY)
+    , href(options));
+
+  if (proxyUrl && typeof proxyUrl === 'string') { proxyUrl = stripQuotesAndSpaces(proxyUrl); }
 
   if (shouldProxy) {
     proxyUrl = new URL(proxyUrl);

packages/client/test/unit/request.test.js

@@ -381,6 +381,20 @@ describe('Unit / Request', () => {
             expect(proxy.connects).toEqual([]);
           });
 
+          it('successfully proxies requests when `noProxy` is null or not string', async () => {
+            process.env.no_proxy = null;
+            proxyAgentFor.cache.clear();
+
+            await expectAsync(server.request('/test'))
+              .toBeResolvedTo('test proxied');
+
+            process.env.no_proxy = 1234;
+            proxyAgentFor.cache.clear();
+
+            await expectAsync(server.request('/test'))
+              .toBeResolvedTo('test proxied');
+          });
+
           it('does not proxy requests if the `noProxy` option is truthy', async () => {
             await expectAsync(server.request('/test', { noProxy: true }))
               .toBeResolvedTo('test');

packages/env/src/utils.js

@@ -62,3 +62,12 @@ export function github({ GITHUB_EVENT_PATH }) {
 
   return (github.payload ||= {});
 }
+
+// auto strip double quotes/spaces if any
+export function stripQuotesAndSpaces(line) {
+  if (typeof line === 'string') {
+    const regex = /^["\s"]+|["\s"]+$/g;
+    const strippedLine = line.replace(regex, '');
+    return strippedLine;
+  } else { return null; };
+}

packages/env/test/stripQuotesAndSpaces.test.js

@@ -0,0 +1,31 @@
+import { stripQuotesAndSpaces } from '@percy/env/utils';
+
+describe('stripQuotesAndSpaces', () => {
+  it('should remove leading and trailing spaces', () => {
+    const line = '  this is a line with spaces  ';
+    const expected = 'this is a line with spaces';
+    const actual = stripQuotesAndSpaces(line);
+    expect(actual).toBe(expected);
+  });
+
+  it('should remove leading and trailing double quotes', () => {
+    const line = '"this is a line with double quotes"';
+    const expected = 'this is a line with double quotes';
+    const actual = stripQuotesAndSpaces(line);
+    expect(actual).toBe(expected);
+  });
+
+  it('should remove both leading and trailing spaces and double quotes', () => {
+    const line = '" this is a line with spaces and double quotes  "';
+    const expected = 'this is a line with spaces and double quotes';
+    const actual = stripQuotesAndSpaces(line);
+    expect(actual).toBe(expected);
+  });
+
+  it('should return null if line is null', () => {
+    const line = null;
+    const expected = null;
+    const actual = stripQuotesAndSpaces(line);
+    expect(actual).toBe(expected);
+  });
+});