diff --git a/cli/lib/tasks/install.js b/cli/lib/tasks/install.js index 4f7b74922ada..992a35dfe5f7 100644 --- a/cli/lib/tasks/install.js +++ b/cli/lib/tasks/install.js @@ -18,9 +18,9 @@ const { throwFormErrorText, errors } = require('../errors') const alreadyInstalledMsg = () => { if (!util.isPostInstall()) { - logger.log(stripIndent` + logger.log(stripIndent` Skipping installation: - + Pass the ${chalk.yellow('--force')} option if you'd like to reinstall anyway. `) } @@ -153,9 +153,12 @@ const start = (options = {}) => { // let this environment variable reset the binary version we need if (util.getEnv('CYPRESS_INSTALL_BINARY')) { - const envVarVersion = util.getEnv('CYPRESS_INSTALL_BINARY') + // because passed file paths are often double quoted + // and might have extra whitespace around, be robust and trim the string + const trimAndRemoveDoubleQuotes = true + const envVarVersion = util.getEnv('CYPRESS_INSTALL_BINARY', trimAndRemoveDoubleQuotes) - debug('using environment variable CYPRESS_INSTALL_BINARY %s', envVarVersion) + debug('using environment variable CYPRESS_INSTALL_BINARY "%s"', envVarVersion) if (envVarVersion === '0') { debug('environment variable CYPRESS_INSTALL_BINARY = 0, skipping install') diff --git a/cli/lib/util.js b/cli/lib/util.js index 40a189e4813b..9db9677c8a84 100644 --- a/cli/lib/util.js +++ b/cli/lib/util.js @@ -97,6 +97,27 @@ function printNodeOptions (log = debug) { } } +/** + * Removes double quote characters + * from the start and end of the given string IF they are both present + * + * @example + ``` + dequote('"foo"') + // returns string 'foo' + dequote('foo') + // returns string 'foo' + ``` + */ +const dequote = (str) => { + la(is.string(str), 'expected a string to remove double quotes', str) + if (str.length > 1 && str[0] === '"' && str[str.length - 1] === '"') { + return str.substr(1, str.length - 2) + } + + return str +} + const util = { normalizeModuleOptions, @@ -175,6 +196,8 @@ const util = { process.exit(1) }, + dequote, + titleize (...args) { // prepend first arg with space // and pad so that all messages line up @@ -262,31 +285,39 @@ const util = { return path.join(process.cwd(), '..', '..', filename) }, - getEnv (varName) { + getEnv (varName, trim) { + la(is.unemptyString(varName), 'expected environment variable name, not', varName) + const envVar = process.env[varName] const configVar = process.env[`npm_config_${varName}`] const packageConfigVar = process.env[`npm_package_config_${varName}`] + let result + if (envVar) { debug(`Using ${varName} from environment variable`) - return envVar - } - - if (configVar) { + result = envVar + } else if (configVar) { debug(`Using ${varName} from npm config`) - return configVar - } - - if (packageConfigVar) { + result = configVar + } else if (packageConfigVar) { debug(`Using ${varName} from package.json config`) - return packageConfigVar + result = packageConfigVar } - return undefined - + // environment variables are often set double quotes to escape characters + // and on Windows it can lead to weird things: for example + // set FOO="C:\foo.txt" && node -e "console.log('>>>%s<<<', process.env.FOO)" + // will print + // >>>"C:\foo.txt" <<< + // see https://github.com/cypress-io/cypress/issues/4506#issuecomment-506029942 + // so for sanity sake we should first trim whitespace characters and remove + // double quotes around environment strings if the caller is expected to + // use this environment string as a file path + return trim ? dequote(_.trim(result)) : result }, getCacheDir () { diff --git a/cli/test/lib/tasks/install_spec.js b/cli/test/lib/tasks/install_spec.js index 1a8574f9ba23..cba567ffe2d7 100644 --- a/cli/test/lib/tasks/install_spec.js +++ b/cli/test/lib/tasks/install_spec.js @@ -99,7 +99,49 @@ describe('/lib/tasks/install', function () { }) }) - it('can install local binary zip file without download', function () { + it('trims environment variable before installing', function () { + // note how the version has extra spaces around it on purpose + const filename = '/tmp/local/file.zip' + const version = ` ${filename} ` + + process.env.CYPRESS_INSTALL_BINARY = version + // internally, the variable should be trimmed and just filename checked + sinon.stub(fs, 'pathExistsAsync').withArgs(filename).resolves(true) + + const installDir = state.getVersionDir() + + return install.start() + .then(() => { + expect(unzip.start).to.be.calledWithMatch({ + zipFilePath: filename, + installDir, + }) + }) + }) + + it('removes double quotes around the environment variable before installing', function () { + // note how the version has extra spaces around it on purpose + // and there are double quotes + const filename = '/tmp/local/file.zip' + const version = ` "${filename}" ` + + process.env.CYPRESS_INSTALL_BINARY = version + // internally, the variable should be trimmed, double quotes removed + // and just filename checked against the file system + sinon.stub(fs, 'pathExistsAsync').withArgs(filename).resolves(true) + + const installDir = state.getVersionDir() + + return install.start() + .then(() => { + expect(unzip.start).to.be.calledWithMatch({ + zipFilePath: filename, + installDir, + }) + }) + }) + + it('can install local binary zip file without download from absolute path', function () { const version = '/tmp/local/file.zip' process.env.CYPRESS_INSTALL_BINARY = version diff --git a/cli/test/lib/util_spec.js b/cli/test/lib/util_spec.js index e248252d1ab8..ce3bfe5fc382 100644 --- a/cli/test/lib/util_spec.js +++ b/cli/test/lib/util_spec.js @@ -380,6 +380,32 @@ describe('util', () => { }) }) + describe('dequote', () => { + it('removes double quotes', () => { + expect(util.dequote('"foo"')).to.equal('foo') + }) + + it('keeps single quotes', () => { + expect(util.dequote('\'foo\'')).to.equal('\'foo\'') + }) + + it('keeps unbalanced double quotes', () => { + expect(util.dequote('"foo')).to.equal('"foo') + }) + + it('keeps inner double quotes', () => { + expect(util.dequote('a"b"c')).to.equal('a"b"c') + }) + + it('passes empty strings', () => { + expect(util.dequote('')).to.equal('') + }) + + it('keeps single double quote character', () => { + expect(util.dequote('"')).to.equal('"') + }) + }) + describe('.getEnv', () => { it('reads from package.json config', () => { process.env.npm_package_config_CYPRESS_FOO = 'bar' @@ -403,5 +429,46 @@ describe('util', () => { process.env.npm_config_CYPRESS_FOO = 'bloop' expect(util.getEnv('CYPRESS_FOO')).to.eql('bloop') }) + it('throws on non-string name', () => { + expect(() => { + util.getEnv() + }).to.throw() + + expect(() => { + util.getEnv(42) + }).to.throw() + }) + + context('with trim = true', () => { + it('trims returned string', () => { + process.env.FOO = ' bar ' + expect(util.getEnv('FOO', true)).to.equal('bar') + }) + + it('removes quotes from the returned string', () => { + process.env.FOO = ' "bar" ' + expect(util.getEnv('FOO', true)).to.equal('bar') + }) + + it('removes only single level of double quotes', () => { + process.env.FOO = ' ""bar"" ' + expect(util.getEnv('FOO', true)).to.equal('"bar"') + }) + + it('keeps unbalanced double quote', () => { + process.env.FOO = ' "bar ' + expect(util.getEnv('FOO', true)).to.equal('"bar') + }) + + it('trims but does not remove single quotes', () => { + process.env.FOO = ' \'bar\' ' + expect(util.getEnv('FOO', true)).to.equal('\'bar\'') + }) + + it('keeps whitespace inside removed quotes', () => { + process.env.FOO = '"foo.txt "' + expect(util.getEnv('FOO', true)).to.equal('foo.txt ') + }) + }) }) })