Skip to content

Commit

Permalink
fix: Inability to download with redirects through a proxy (#19310)
Browse files Browse the repository at this point in the history 
* Fixes inability to download with redirects through a proxy

* lint fixes

* t

* tests are passing

* tests pass and works

* a little cleanup

* oops, @cypress/request

* nuding the ci system

* Fix race condition in verify code

* address comments on pull request

* a test with multiple layers of redirects

* Add tests and mechanism for redirect loop error

* Make informative error message

* Remove environment config option for now

* test assertion pattern

Co-authored-by: David Ihnen <[email protected]>
Co-authored-by: Matt Henkes <[email protected]>
Co-authored-by: Emily Rohrbough <[email protected]>
  • Loading branch information
@DavidIAm @davidiamaf
@mjhenkes @emilyrohrbough
4 people committed Jan 5, 2022
1 parent 85813e7 commit df5687c
Show file tree
Hide file tree
Showing 4 changed files with 227 additions and 40 deletions.
97 changes: 57 additions & 40 deletions cli/lib/tasks/download.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const arch = require('arch')
const la = require('lazy-ass')
const is = require('check-more-types')
const os = require('os')
const url = require('url')
const Url = require('url')
const path = require('path')
const debug = require('debug')('cypress:cli')
const request = require('@cypress/request')
Expand All @@ -16,6 +16,7 @@ const fs = require('../fs')
const util = require('../util')

const defaultBaseUrl = 'https://download.cypress.io/'
const defaultMaxRedirects = 10

const getProxyForUrlWithNpmConfig = (url) => {
return getProxyForUrl(url) ||
Expand Down Expand Up @@ -61,7 +62,7 @@ const getCA = () => {
}

const prepend = (urlPath) => {
const endpoint = url.resolve(getBaseUrl(), urlPath)
const endpoint = Url.resolve(getBaseUrl(), urlPath)
const platform = os.platform()

return `${endpoint}?platform=${platform}&arch=${arch()}`
Expand Down Expand Up @@ -186,7 +187,17 @@ const verifyDownloadedFile = (filename, expectedSize, expectedChecksum) => {
// downloads from given url
// return an object with
// {filename: ..., downloaded: true}
const downloadFromUrl = ({ url, downloadDestination, progress, ca }) => {
const downloadFromUrl = ({ url, downloadDestination, progress, ca, version, redirectTTL = defaultMaxRedirects }) => {
if (redirectTTL <= 0) {
return Promise.reject(new Error(
stripIndent`
Failed downloading the Cypress binary.
There were too many redirects. The default allowance is ${defaultMaxRedirects}.
Maybe you got stuck in a redirect loop?
`,
))
}

return new Promise((resolve, reject) => {
const proxy = getProxyForUrlWithNpmConfig(url)

Expand All @@ -196,32 +207,17 @@ const downloadFromUrl = ({ url, downloadDestination, progress, ca }) => {
downloadDestination,
})

let redirectVersion

const reqOptions = {
url,
proxy,
followRedirect (response) {
const version = response.headers['x-version']

debug('redirect version:', version)
if (version) {
// set the version in options if we have one.
// this insulates us from potential redirect
// problems where version would be set to undefined.
redirectVersion = version
}

// yes redirect
return true
},
}

if (ca) {
debug('using custom CA details from npm config')
reqOptions.agentOptions = { ca }
}

const reqOptions = {
uri: url,
...(proxy ? { proxy } : {}),
...(ca ? { agentOptions: { ca } } : {}),
method: 'GET',
followRedirect: false,
}
const req = request(reqOptions)

// closure
Expand Down Expand Up @@ -256,8 +252,17 @@ const downloadFromUrl = ({ url, downloadDestination, progress, ca }) => {
// response headers
started = new Date()

// if our status code does not start with 200
if (!/^2/.test(response.statusCode)) {
if (/^3/.test(response.statusCode)) {
const redirectVersion = response.headers['x-version']
const redirectUrl = response.headers.location

debug('redirect version:', redirectVersion)
debug('redirect url:', redirectUrl)
downloadFromUrl({ url: redirectUrl, progress, ca, downloadDestination, version: redirectVersion, redirectTTL: redirectTTL - 1 })
.then(resolve).catch(reject)

// if our status code does not start with 200
} else if (!/^2/.test(response.statusCode)) {
debug('response code %d', response.statusCode)

const err = new Error(
Expand All @@ -269,9 +274,30 @@ const downloadFromUrl = ({ url, downloadDestination, progress, ca }) => {
)

reject(err)
// status codes here are all 2xx
} else {
// We only enable this pipe connection when we know we've got a successful return
// and handle the completion with verify and resolve
// there was a possible race condition between end of request and close of writeStream
// that is made ordered with this Promise.all
Promise.all([new Promise((r) => {
return response.pipe(fs.createWriteStream(downloadDestination).on('close', r))
}), new Promise((r) => response.on('end', r))])
.then(() => {
debug('downloading finished')
verifyDownloadedFile(downloadDestination, expectedSize,
expectedChecksum)
.then(() => debug('verified'))
.then(() => resolve(version))
.catch(reject)
})
}
})
.on('error', reject)
.on('error', (e) => {
if (e.code === 'ECONNRESET') return // sometimes proxies give ECONNRESET but we don't care

reject(e)
})
.on('progress', (state) => {
// total time we've elapsed
// starting on our first progress notification
Expand All @@ -285,16 +311,6 @@ const downloadFromUrl = ({ url, downloadDestination, progress, ca }) => {
// send up our percent and seconds remaining
progress.onProgress(percentage, util.secsRemaining(eta))
})
// save this download here
.pipe(fs.createWriteStream(downloadDestination))
.on('finish', () => {
debug('downloading finished')

verifyDownloadedFile(downloadDestination, expectedSize, expectedChecksum)
.then(() => {
return resolve(redirectVersion)
}, reject)
})
})
}

Expand All @@ -304,7 +320,7 @@ const downloadFromUrl = ({ url, downloadDestination, progress, ca }) => {
* @param [string] downloadDestination Local filename to save as
*/
const start = (opts) => {
let { version, downloadDestination, progress } = opts
let { version, downloadDestination, progress, redirectTTL } = opts

if (!downloadDestination) {
la(is.unemptyString(downloadDestination), 'missing download dir', opts)
Expand All @@ -330,7 +346,8 @@ const start = (opts) => {
return getCA()
})
.then((ca) => {
return downloadFromUrl({ url, downloadDestination, progress, ca })
return downloadFromUrl({ url, downloadDestination, progress, ca, version,
...(redirectTTL ? { redirectTTL } : {}) })
})
.catch((err) => {
return prettyDownloadErr(err, version)
Expand Down
162 changes: 162 additions & 0 deletions cli/test/lib/tasks/download_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,157 @@ describe('lib/tasks/download', function () {
})
})

it('handles quadruple redirect with response x-version to the latest if present', function () {
nock('https://aws.amazon.com')
.get('/some.zip')
.reply(200, () => {
return fs.createReadStream(examplePath)
})

nock('https://aws.amazon.com')
.get('/someone.zip')
.query(true)
.reply(302, undefined, {
Location: 'https://aws.amazon.com/somebody.zip',
'x-version': '0.11.2',
})

nock('https://aws.amazon.com')
.get('/something.zip')
.query(true)
.reply(302, undefined, {
Location: 'https://aws.amazon.com/some.zip',
'x-version': '0.11.4',
})

nock('https://aws.amazon.com')
.get('/somebody.zip')
.query(true)
.reply(302, undefined, {
Location: 'https://aws.amazon.com/something.zip',
'x-version': '0.11.3',
})

nock('https://download.cypress.io')
.get('/desktop/1.2.3')
.query(true)
.reply(302, undefined, {
Location: 'https://aws.amazon.com/someone.zip',
'x-version': '0.11.1',
})

return download.start(this.options).then((responseVersion) => {
expect(responseVersion).to.eq('0.11.4')
})
})

it('errors on too many redirects', function () {
nock('https://aws.amazon.com')
.get('/some.zip')
.reply(200, () => {
return fs.createReadStream(examplePath)
})

nock('https://download.cypress.io')
.get('/desktop/1.2.3')
.query(true)
.reply(302, undefined, {
Location: 'https://aws.amazon.com/someone.zip',
'x-version': '0.11.1',
})

nock('https://aws.amazon.com')
.get('/someone.zip')
.query(true)
.reply(302, undefined, {
Location: 'https://aws.amazon.com/somebody.zip',
'x-version': '0.11.2',
})

nock('https://aws.amazon.com')
.get('/somebody.zip')
.query(true)
.reply(302, undefined, {
Location: 'https://aws.amazon.com/something.zip',
'x-version': '0.11.3',
})

nock('https://aws.amazon.com')
.get('/something.zip')
.query(true)
.reply(302, undefined, {
Location: 'https://aws.amazon.com/somewhat.zip',
'x-version': '0.11.4',
})

nock('https://aws.amazon.com')
.get('/somewhat.zip')
.query(true)
.reply(302, undefined, {
Location: 'https://aws.amazon.com/sometime.zip',
'x-version': '0.11.5',
})

nock('https://aws.amazon.com')
.get('/sometime.zip')
.query(true)
.reply(302, undefined, {
Location: 'https://aws.amazon.com/somewhen.zip',
'x-version': '0.11.6',
})

nock('https://aws.amazon.com')
.get('/somewhen.zip')
.query(true)
.reply(302, undefined, {
Location: 'https://aws.amazon.com/somewise.zip',
'x-version': '0.11.7',
})

nock('https://aws.amazon.com')
.get('/somewise.zip')
.query(true)
.reply(302, undefined, {
Location: 'https://aws.amazon.com/someways.zip',
'x-version': '0.11.8',
})

nock('https://aws.amazon.com')
.get('/someways.zip')
.query(true)
.reply(302, undefined, {
Location: 'https://aws.amazon.com/somerset.zip',
'x-version': '0.11.9',
})

nock('https://aws.amazon.com')
.get('/somerset.zip')
.query(true)
.reply(302, undefined, {
Location: 'https://aws.amazon.com/somedeal.zip',
'x-version': '0.11.10',
})

nock('https://aws.amazon.com')
.get('/somedeal.zip')
.query(true)
.reply(302, undefined, {
Location: 'https://aws.amazon.com/some.zip',
'x-version': '0.11.11',
})

return download.start(this.options).then(() => expect(true).to.equal(false)).catch((error) => {
expect(error).to.be.instanceof(Error)
expect(error.message).to.contain('redirect loop')
})
.then(() => {
// Double check to make sure that raising redirectTTL changes result
download.start({ ...this.options, redirectTTL: 12 }).then((responseVersion) => {
expect(responseVersion).to.eq('0.11.11')
})
})
})

it('can specify cypress version in arguments', function () {
this.options.version = '0.13.0'

Expand Down Expand Up @@ -318,6 +469,17 @@ describe('lib/tasks/download', function () {

beforeEach(function () {
this.env = _.clone(process.env)
// prevent ambient environment masking of environment variables referenced in this test

;([
'CYPRESS_DOWNLOAD_USE_CA', 'NO_PROXY', 'http_proxy',
'https_proxy', 'npm_config_ca', 'npm_config_cafile',
'npm_config_https_proxy', 'npm_config_proxy',
]).forEach((e) => {
delete process.env[e.toLowerCase()]
delete process.env[e.toUpperCase()]
})

// add a default no_proxy which does not match the testUri
process.env.NO_PROXY = 'localhost,.org'
})
Expand Down
3 changes: 3 additions & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@
"watch": "lerna exec yarn watch --parallel --stream",
"prepare": "husky install"
},
"dependencies": {
"nvm": "0.0.4"
},
"devDependencies": {
"@cypress/bumpercar": "2.0.12",
"@cypress/commit-message-install": "3.1.3",
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28963,6 +28963,11 @@ number-is-nan@^1.0.0:
resolved "https://registry.yarnpkg.com/number-is-nan/-/number-is-nan-1.0.1.tgz#097b602b53422a522c1afb8790318336941a011d"
integrity sha1-CXtgK1NCKlIsGvuHkDGDNpQaAR0=

[email protected]:
version "0.0.4"
resolved "https://registry.yarnpkg.com/nvm/-/nvm-0.0.4.tgz#38a178e9d31b283508c92d15c9da861d1a9210bc"
integrity sha1-OKF46dMbKDUIyS0VydqGHRqSELw=

nwsapi@^2.0.7, nwsapi@^2.0.9, nwsapi@^2.1.3:
version "2.2.0"
resolved "https://registry.yarnpkg.com/nwsapi/-/nwsapi-2.2.0.tgz#204879a9e3d068ff2a55139c2c772780681a38b7"
Expand Down

3 comments on commit df5687c

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on df5687c Jan 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the linux x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/9.2.1/circle-develop-df5687c65d82e0591256df2dea727e5680baeb82/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on df5687c Jan 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the darwin x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/9.2.1/circle-develop-df5687c65d82e0591256df2dea727e5680baeb82/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on df5687c Jan 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the win32 x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/9.2.1/circle-develop-df5687c65d82e0591256df2dea727e5680baeb82/cypress.tgz

Please sign in to comment.