From 928dbc4e3817c68430744b18533c12d3f0e13de7 Mon Sep 17 00:00:00 2001 From: Daniel Rey Lopez Date: Sat, 25 Mar 2017 22:42:53 +0000 Subject: [PATCH 1/2] fix: Resolve value of env variables before invoking cross-spawn #90 --- src/index.js | 4 ++-- src/variable.js | 38 +++++++++++++++++++++++++++++++++----- src/variable.test.js | 28 ++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 7 deletions(-) diff --git a/src/index.js b/src/index.js index e7734ef..1086830 100644 --- a/src/index.js +++ b/src/index.js @@ -26,9 +26,9 @@ function crossEnv(args) { function getCommandArgsAndEnvVars(args) { const envVars = getEnvVars() - const commandArgs = args.map(commandConvert) + const commandArgs = args.slice() const command = getCommand(commandArgs, envVars) - return [command, commandArgs, envVars] + return [commandConvert(command), commandArgs.map(commandConvert), envVars] } function getCommand(commandArgs, envVars) { diff --git a/src/variable.js b/src/variable.js index 4012cc4..92d66a7 100644 --- a/src/variable.js +++ b/src/variable.js @@ -1,16 +1,15 @@ import isWindows from 'is-windows' /** - * Converts an environment variable value to be appropriate for the current OS. - * It will transform UNIX-style list values to Windows-style. + * This will transform UNIX-style list values to Windows-style. * For example, the value of the $PATH variable "/usr/bin:/usr/local/bin:." * will become "/usr/bin;/usr/local/bin;." on Windows. - * @param {String} originalValue Original value of the env variable + * @param {String} varValue Original value of the env variable * @returns {String} Converted value */ -export default function varValueConvert(originalValue) { +function replaceListDelimiters(varValue) { const targetSeparator = isWindows() ? ';' : ':' - return originalValue.replace(/(\\*):/g, (match, backslashes) => { + return varValue.replace(/(\\*):/g, (match, backslashes) => { if (backslashes.length % 2) { // Odd number of backslashes preceding it means it's escaped, // remove 1 backslash and return the rest as-is @@ -19,3 +18,32 @@ export default function varValueConvert(originalValue) { return backslashes + targetSeparator }) } + +/** + * This will attempt to resolve the value of any env variables that are inside + * this string. For example, it will transform this: + * cross-env FOO=$NODE_ENV echo $FOO + * Into this: + * FOO=development echo $FOO + * (Or whatever value the variable NODE_ENV has) + * Note that this function is only called with the right-side portion of the + * env var assignment, so in that example, this function would transform + * the string "$NODE_ENV" into "development" + * @param {String} varValue Original value of the env variable + * @returns {String} Converted value + */ +function resolveEnvVars(varValue) { + const envUnixRegex = /\$(\w+)|\${(\w+)}/g // $my_var or ${my_var} + return varValue.replace(envUnixRegex, (_, varName, altVarName) => { + return process.env[varName || altVarName] || '' + }) +} + +/** + * Converts an environment variable value to be appropriate for the current OS. + * @param {String} originalValue Original value of the env variable + * @returns {String} Converted value + */ +export default function varValueConvert(originalValue) { + return resolveEnvVars(replaceListDelimiters(originalValue)) +} diff --git a/src/variable.test.js b/src/variable.test.js index 41c8602..b6fa3a4 100644 --- a/src/variable.test.js +++ b/src/variable.test.js @@ -2,9 +2,16 @@ import isWindowsMock from 'is-windows' import varValueConvert from './variable' beforeEach(() => { + process.env.VAR1 = 'value1' + process.env.VAR2 = 'value2' isWindowsMock.__mock.reset() }) +afterEach(() => { + delete process.env.VAR1 + delete process.env.VAR2 +}) + test(`doesn't affect simple variable values`, () => { isWindowsMock.__mock.returnValue = true expect(varValueConvert('foo')).toBe('foo') @@ -44,3 +51,24 @@ test(`converts multiple separators`, () => { isWindowsMock.__mock.returnValue = true expect(varValueConvert('foo:bar:baz')).toBe('foo;bar;baz') }) + +test(`resolves an env variable value`, () => { + isWindowsMock.__mock.returnValue = true + expect(varValueConvert('foo-$VAR1')).toBe('foo-value1') +}) + +test(`resolves an env variable value with curly syntax`, () => { + isWindowsMock.__mock.returnValue = true + // eslint-disable-next-line no-template-curly-in-string + expect(varValueConvert('foo-${VAR1}')).toBe('foo-value1') +}) + +test(`resolves multiple env variable values`, () => { + isWindowsMock.__mock.returnValue = true + expect(varValueConvert('foo-$VAR1-$VAR2')).toBe('foo-value1-value2') +}) + +test(`resolves an env variable value for non-existant variable`, () => { + isWindowsMock.__mock.returnValue = true + expect(varValueConvert('foo-$VAR_POTATO')).toBe('foo-') +}) From 53776d3949210ca32bcdb9ab744db38bd7644884 Mon Sep 17 00:00:00 2001 From: Daniel Rey Lopez Date: Mon, 27 Mar 2017 00:16:57 +0100 Subject: [PATCH 2/2] Refactored the main parsing loop --- src/index.js | 47 ++++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/src/index.js b/src/index.js index 1086830..0e223d0 100644 --- a/src/index.js +++ b/src/index.js @@ -7,13 +7,17 @@ module.exports = crossEnv const envSetterRegex = /(\w+)=('(.+)'|"(.+)"|(.+))/ function crossEnv(args) { - const [command, commandArgs, env] = getCommandArgsAndEnvVars(args) + const [envSetters, command, commandArgs] = parseCommand(args) if (command) { - const proc = spawn(command, commandArgs, { - stdio: 'inherit', - shell: true, - env, - }) + const proc = spawn( + commandConvert(command), + commandArgs.map(commandConvert), + { + stdio: 'inherit', + shell: true, + env: getEnvVars(envSetters), + }, + ) process.on('SIGTERM', () => proc.kill('SIGTERM')) process.on('SIGINT', () => proc.kill('SIGINT')) process.on('SIGBREAK', () => proc.kill('SIGBREAK')) @@ -24,30 +28,31 @@ function crossEnv(args) { return null } -function getCommandArgsAndEnvVars(args) { - const envVars = getEnvVars() - const commandArgs = args.slice() - const command = getCommand(commandArgs, envVars) - return [commandConvert(command), commandArgs.map(commandConvert), envVars] -} - -function getCommand(commandArgs, envVars) { - while (commandArgs.length) { - const shifted = commandArgs.shift() - const match = envSetterRegex.exec(shifted) +function parseCommand(args) { + const envSetters = {} + let command = null + let commandArgs = [] + for (let i = 0; i < args.length; i++) { + const match = envSetterRegex.exec(args[i]) if (match) { - envVars[match[1]] = varValueConvert(match[3] || match[4] || match[5]) + envSetters[match[1]] = match[3] || match[4] || match[5] } else { - return shifted + // No more env setters, the rest of the line must be the command and args + command = args[i] + commandArgs = args.slice(i + 1) + break } } - return null + return [envSetters, command, commandArgs] } -function getEnvVars() { +function getEnvVars(envSetters) { const envVars = Object.assign({}, process.env) if (process.env.APPDATA) { envVars.APPDATA = process.env.APPDATA } + Object.keys(envSetters).forEach(varName => { + envVars[varName] = varValueConvert(envSetters[varName]) + }) return envVars }