fix(cli): Respect NO_PROXY on cypress download

cli/lib/tasks/download.js

@@ -9,19 +9,17 @@ const request = require('@cypress/request')
 const Promise = require('bluebird')
 const requestProgress = require('request-progress')
 const { stripIndent } = require('common-tags')
+const getProxyForUrl = require('proxy-from-env').getProxyForUrl
 
 const { throwFormErrorText, errors } = require('../errors')
 const fs = require('../fs')
 const util = require('../util')
 
 const defaultBaseUrl = 'https://download.cypress.io/'
 
-const getProxyUrl = () => {
-  return process.env.HTTPS_PROXY ||
-    process.env.https_proxy ||
+const getProxyForUrlWithNpmConfig = (url) => {
+  return getProxyForUrl(url) ||
     process.env.npm_config_https_proxy ||
-    process.env.HTTP_PROXY ||
-    process.env.http_proxy ||
     process.env.npm_config_proxy ||
     null
 }
@@ -205,7 +203,7 @@ const verifyDownloadedFile = (filename, expectedSize, expectedChecksum) => {
 // {filename: ..., downloaded: true}
 const downloadFromUrl = ({ url, downloadDestination, progress, ca }) => {
   return new Promise((resolve, reject) => {
-    const proxy = getProxyUrl()
+    const proxy = getProxyForUrlWithNpmConfig(url)
 
     debug('Downloading package', {
       url,
@@ -357,6 +355,6 @@ const start = (opts) => {
 module.exports = {
   start,
   getUrl,
-  getProxyUrl,
+  getProxyForUrlWithNpmConfig,
   getCA,
 }

cli/test/lib/tasks/download_spec.js

@@ -313,28 +313,60 @@ describe('lib/tasks/download', function () {
   })
 
   context('with proxy env vars', () => {
+    const testUriHttp = 'http://anything.com'
+    const testUriHttps = 'https://anything.com'
+
     beforeEach(function () {
       this.env = _.clone(process.env)
+      // add a default no_proxy which does not match the testUri
+      process.env.NO_PROXY = 'localhost,.org'
     })
 
     afterEach(function () {
       process.env = this.env
     })
 
-    it('prefers https_proxy over http_proxy', () => {
-      process.env.HTTP_PROXY = 'foo'
-      expect(download.getProxyUrl()).to.eq('foo')
-      process.env.https_proxy = 'bar'
-      expect(download.getProxyUrl()).to.eq('bar')
+    it('uses http_proxy on http request', () => {
+      process.env.http_proxy = 'http://foo'
+      expect(download.getProxyForUrlWithNpmConfig(testUriHttp)).to.eq('http://foo')
+    })
+
+    it('ignores http_proxy on https request', () => {
+      process.env.http_proxy = 'http://foo'
+      expect(download.getProxyForUrlWithNpmConfig(testUriHttps)).to.eq(null)
+      process.env.https_proxy = 'https://bar'
+      expect(download.getProxyForUrlWithNpmConfig(testUriHttps)).to.eq('https://bar')
     })
 
     it('falls back to npm_config_proxy', () => {
-      process.env.npm_config_proxy = 'foo'
-      expect(download.getProxyUrl()).to.eq('foo')
-      process.env.npm_config_https_proxy = 'bar'
-      expect(download.getProxyUrl()).to.eq('bar')
-      process.env.https_proxy = 'baz'
-      expect(download.getProxyUrl()).to.eq('baz')
+      process.env.npm_config_proxy = 'http://foo'
+      expect(download.getProxyForUrlWithNpmConfig(testUriHttps)).to.eq('http://foo')
+      process.env.npm_config_https_proxy = 'https://bar'
+      expect(download.getProxyForUrlWithNpmConfig(testUriHttps)).to.eq('https://bar')
+      process.env.https_proxy = 'https://baz'
+      expect(download.getProxyForUrlWithNpmConfig(testUriHttps)).to.eq('https://baz')
+    })
+
+    it('respects no_proxy on http and https requests', () => {
+      process.env.NO_PROXY = 'localhost,.com'
+
+      process.env.http_proxy = 'http://foo'
+      process.env.https_proxy = 'https://bar'
+
+      expect(download.getProxyForUrlWithNpmConfig(testUriHttp)).to.eq(null)
+      expect(download.getProxyForUrlWithNpmConfig(testUriHttps)).to.eq(null)
+    })
+
+    it('ignores no_proxy for npm proxy configs, prefers https over http', () => {
+      process.env.NO_PROXY = 'localhost,.com'
+
+      process.env.npm_config_proxy = 'http://foo'
+      expect(download.getProxyForUrlWithNpmConfig(testUriHttp)).to.eq('http://foo')
+      expect(download.getProxyForUrlWithNpmConfig(testUriHttps)).to.eq('http://foo')
+
+      process.env.npm_config_https_proxy = 'https://bar'
+      expect(download.getProxyForUrlWithNpmConfig(testUriHttp)).to.eq('https://bar')
+      expect(download.getProxyForUrlWithNpmConfig(testUriHttps)).to.eq('https://bar')
     })
   })