diff --git a/.eslintrc b/.eslintrc index f34b41d..6edac35 100644 --- a/.eslintrc +++ b/.eslintrc @@ -1,7 +1,7 @@ { "root": true, "extends": [ - "@satazor/eslint-config/es5", + "@satazor/eslint-config/es6", "@satazor/eslint-config/addons/node" ] -} \ No newline at end of file +} diff --git a/.travis.yml b/.travis.yml index 57a922d..8c7c0f3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,6 @@ language: node_js node_js: - - '0.10' - - '0.12' - '4' - '6' - - '7' + - 'node' + - 'lts/*' diff --git a/CHANGELOG.md b/CHANGELOG.md index f1298a8..dc1d8bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,20 @@ +## 6.0.0 - 2017-11-11 + +- Remove NodeJS v0.10 and v0.12 support +- Add `^` to also escape Windows metachars: + - Fix a bug that made it impossible to escape an argument that contained quotes followed by `>` or other special chars, e.g.: `"foo|bar"`, fixes [#82](https://github.com/IndigoUnited/node-cross-spawn/issues/82) + - Fix a bug were a command containing `%x%` would be replaced with the contents of the `x` environment variable, fixes [#51](https://github.com/IndigoUnited/node-cross-spawn/issues/51) +- Fix `options` argument being mutated + + +## 5.1.1 - 2017-02-26 + +- Fix `options.shell` support for NodeJS [v4.8](https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V4.md#4.8.0) + +## 5.0.1 - 2016-11-04 + +- Fix `options.shell` support for NodeJS v7 + ## 5.0.0 - 2016-10-30 - Add support for `options.shell` diff --git a/README.md b/README.md index 0664183..eca6083 100644 --- a/README.md +++ b/README.md @@ -23,10 +23,6 @@ A cross platform solution to node's spawn and spawnSync. `$ npm install cross-spawn` -If you are using `spawnSync` on node 0.10 or older, you will also need to install `spawn-sync`: - -`$ npm install spawn-sync` - ## Why @@ -35,7 +31,7 @@ Node has issues when using spawn on Windows: - It ignores [PATHEXT](https://github.com/joyent/node/issues/2318) - It does not support [shebangs](http://pt.wikipedia.org/wiki/Shebang) - No `options.shell` support on node `= 6 and >= 4.8 -var supportsShellOption = parseInt(process.version.substr(1).split('.')[0], 10) >= 6 || - parseInt(process.version.substr(1).split('.')[0], 10) === 4 && parseInt(process.version.substr(1).split('.')[1], 10) >= 8; +const supportsShellOption = + parseInt(process.version.substr(1).split('.')[0], 10) >= 6 || + (parseInt(process.version.substr(1).split('.')[0], 10) === 4 && parseInt(process.version.substr(1).split('.')[1], 10) >= 8); -function parseNonShell(parsed) { - var shebang; - var needsShell; - var applyQuotes; +function detectShebang(parsed) { + parsed.file = resolveCommand(parsed.command) || resolveCommand(parsed.command, true); + + const shebang = parsed.file && readShebang(parsed.file); + + if (shebang) { + parsed.args.unshift(parsed.file); + parsed.command = shebang; + + return resolveCommand(shebang) || resolveCommand(shebang, true); + } + + return parsed.file; +} +function parseNonShell(parsed) { if (!isWin) { return parsed; } // Detect & add support for shebangs - parsed.file = resolveCommand(parsed.command); - parsed.file = parsed.file || resolveCommand(parsed.command, true); - shebang = parsed.file && readShebang(parsed.file); + const commandFile = detectShebang(parsed); - if (shebang) { - parsed.args.unshift(parsed.file); - parsed.command = shebang; - needsShell = hasEmptyArgumentBug || !skipShellRegExp.test(resolveCommand(shebang) || resolveCommand(shebang, true)); - } else { - needsShell = hasEmptyArgumentBug || !skipShellRegExp.test(parsed.file); - } + // We don't need a shell if the command filename is an executable + const needsShell = !isExecutableRegExp.test(commandFile); // If a shell is required, use cmd.exe and take care of escaping everything correctly - if (needsShell) { + // Note that `forceShell` is an hidden option used only in tests + if (parsed.options.forceShell || needsShell) { // Escape command & arguments - applyQuotes = (parsed.command !== 'echo'); // Do not quote arguments for the special "echo" command - parsed.command = escapeCommand(parsed.command); - parsed.args = parsed.args.map(function (arg) { - return escapeArgument(arg, applyQuotes); - }); - - // Make use of cmd.exe - parsed.args = ['/d', '/s', '/c', '"' + parsed.command + (parsed.args.length ? ' ' + parsed.args.join(' ') : '') + '"']; + parsed.command = escape.command(parsed.command); + parsed.args = parsed.args.map(escape.argument); + + const shellCommand = [parsed.command].concat(parsed.args).join(' '); + + parsed.args = ['/d', '/s', '/c', `"${shellCommand}"`]; parsed.command = process.env.comspec || 'cmd.exe'; parsed.options.windowsVerbatimArguments = true; // Tell node's spawn that the arguments are already escaped } @@ -54,19 +56,17 @@ function parseNonShell(parsed) { } function parseShell(parsed) { - var shellCommand; - // If node supports the shell option, there's no need to mimic its behavior if (supportsShellOption) { return parsed; } // Mimic node shell option, see: https://github.com/nodejs/node/blob/b9f6a2dc059a1062776133f3d4fd848c4da7d150/lib/child_process.js#L335 - shellCommand = [parsed.command].concat(parsed.args).join(' '); + const shellCommand = [parsed.command].concat(parsed.args).join(' '); if (isWin) { parsed.command = typeof parsed.options.shell === 'string' ? parsed.options.shell : process.env.comspec || 'cmd.exe'; - parsed.args = ['/d', '/s', '/c', '"' + shellCommand + '"']; + parsed.args = ['/d', '/s', '/c', `"${shellCommand}"`]; parsed.options.windowsVerbatimArguments = true; // Tell node's spawn that the arguments are already escaped } else { if (typeof parsed.options.shell === 'string') { @@ -86,8 +86,6 @@ function parseShell(parsed) { // ------------------------------------------------ function parse(command, args, options) { - var parsed; - // Normalize arguments, similar to nodejs if (args && !Array.isArray(args)) { options = args; @@ -95,13 +93,13 @@ function parse(command, args, options) { } args = args ? args.slice(0) : []; // Clone array to avoid changing the original - options = options || {}; + options = Object.assign({}, options); // Clone object to avoid changing the original // Build our parsed object - parsed = { - command: command, - args: args, - options: options, + const parsed = { + command, + args, + options, file: undefined, original: command, }; diff --git a/lib/util/escape.js b/lib/util/escape.js new file mode 100644 index 0000000..ef15fa4 --- /dev/null +++ b/lib/util/escape.js @@ -0,0 +1,37 @@ +'use strict'; + +const metaCharsRegExp = /([()\][%!^"`<>&|;,\s*?])/g; + +function escapeCommand(arg) { + // Escape meta chars + arg = arg.replace(metaCharsRegExp, '^$1'); + + return arg; +} + +function escapeArgument(arg) { + // Convert to string + arg = `${arg}`; + + // Sequence of backslashes followed by a double quote: + // double up all the backslashes and escape the double quote + arg = arg.replace(/(\\*)"/g, '$1$1\\"'); + + // Sequence of backslashes followed by the end of the string + // (which will become a double quote later): + // double up all the backslashes + arg = arg.replace(/(\\*)$/, '$1$1'); + + // All other backslashes occur literally + + // Quote the whole thing: + arg = `"${arg}"`; + + // Escape meta chars + arg = arg.replace(metaCharsRegExp, '^$1'); + + return arg; +} + +module.exports.command = escapeCommand; +module.exports.argument = escapeArgument; diff --git a/lib/util/escapeArgument.js b/lib/util/escapeArgument.js deleted file mode 100644 index 367263f..0000000 --- a/lib/util/escapeArgument.js +++ /dev/null @@ -1,30 +0,0 @@ -'use strict'; - -function escapeArgument(arg, quote) { - // Convert to string - arg = '' + arg; - - // If we are not going to quote the argument, - // escape shell metacharacters, including double and single quotes: - if (!quote) { - arg = arg.replace(/([()%!^<>&|;,"'\s])/g, '^$1'); - } else { - // Sequence of backslashes followed by a double quote: - // double up all the backslashes and escape the double quote - arg = arg.replace(/(\\*)"/g, '$1$1\\"'); - - // Sequence of backslashes followed by the end of the string - // (which will become a double quote later): - // double up all the backslashes - arg = arg.replace(/(\\*)$/, '$1$1'); - - // All other backslashes occur literally - - // Quote the whole thing: - arg = '"' + arg + '"'; - } - - return arg; -} - -module.exports = escapeArgument; diff --git a/lib/util/escapeCommand.js b/lib/util/escapeCommand.js deleted file mode 100644 index d9c25b2..0000000 --- a/lib/util/escapeCommand.js +++ /dev/null @@ -1,12 +0,0 @@ -'use strict'; - -var escapeArgument = require('./escapeArgument'); - -function escapeCommand(command) { - // Do not escape if this command is not dangerous.. - // We do this so that commands like "echo" or "ifconfig" work - // Quoting them, will make them unaccessible - return /^[a-z0-9_-]+$/i.test(command) ? command : escapeArgument(command, true); -} - -module.exports = escapeCommand; diff --git a/lib/util/hasEmptyArgumentBug.js b/lib/util/hasEmptyArgumentBug.js deleted file mode 100644 index 9f2eba6..0000000 --- a/lib/util/hasEmptyArgumentBug.js +++ /dev/null @@ -1,18 +0,0 @@ -'use strict'; - -// See: https://github.com/IndigoUnited/node-cross-spawn/pull/34#issuecomment-221623455 -function hasEmptyArgumentBug() { - var nodeVer; - - if (process.platform !== 'win32') { - return false; - } - - nodeVer = process.version.substr(1).split('.').map(function (num) { - return parseInt(num, 10); - }); - - return (nodeVer[0] === 0 && nodeVer[1] < 12); -} - -module.exports = hasEmptyArgumentBug(); diff --git a/lib/util/readShebang.js b/lib/util/readShebang.js index 2cf3541..031fefc 100644 --- a/lib/util/readShebang.js +++ b/lib/util/readShebang.js @@ -1,23 +1,22 @@ 'use strict'; -var fs = require('fs'); -var LRU = require('lru-cache'); -var shebangCommand = require('shebang-command'); +const fs = require('fs'); +const LRU = require('lru-cache'); +const shebangCommand = require('shebang-command'); -var shebangCache = new LRU({ max: 50, maxAge: 30 * 1000 }); // Cache just for 30sec +const shebangCache = new LRU({ max: 50, maxAge: 30 * 1000 }); // Cache just for 30sec function readShebang(command) { - var buffer; - var fd; - var shebang; + let shebang = shebangCache.get(command); // Check if it is in the cache first - if (shebangCache.has(command)) { - return shebangCache.get(command); + if (shebang) { + return shebang; } // Read the first 150 bytes from the file - buffer = new Buffer(150); + let fd; + const buffer = new Buffer(150); try { fd = fs.openSync(command, 'r'); diff --git a/lib/util/resolveCommand.js b/lib/util/resolveCommand.js index b7a9490..0ff0893 100644 --- a/lib/util/resolveCommand.js +++ b/lib/util/resolveCommand.js @@ -1,20 +1,20 @@ 'use strict'; -var path = require('path'); -var which = require('which'); -var LRU = require('lru-cache'); +const path = require('path'); +const which = require('which'); +const LRU = require('lru-cache'); -var commandCache = new LRU({ max: 50, maxAge: 30 * 1000 }); // Cache just for 30sec +const commandCache = new LRU({ max: 50, maxAge: 30 * 1000 }); // Cache just for 30sec function resolveCommand(command, noExtension) { - var resolved; - noExtension = !!noExtension; - resolved = commandCache.get(command + '!' + noExtension); + + const cacheId = `${command}!${noExtension}`; + let resolved = commandCache.get(cacheId); // Check if its resolved in the cache - if (commandCache.has(command)) { - return commandCache.get(command); + if (resolved) { + return resolved; } try { @@ -23,7 +23,7 @@ function resolveCommand(command, noExtension) { which.sync(command, { pathExt: path.delimiter + (process.env.PATHEXT || '') }); } catch (e) { /* empty */ } - commandCache.set(command + '!' + noExtension, resolved); + commandCache.set(cacheId, resolved); return resolved; } diff --git a/package.json b/package.json index 7c10c97..bf29379 100644 --- a/package.json +++ b/package.json @@ -46,8 +46,11 @@ "expect.js": "^0.3.0", "glob": "^7.0.0", "mkdirp": "^0.5.1", - "mocha": "^3.0.2", + "mocha": "^4.0.0", "once": "^1.4.0", "rimraf": "^2.5.0" + }, + "engines": { + "node": ">=4.8" } } diff --git a/test/.eslintrc b/test/.eslintrc index 32a5d9a..92c6c5b 100644 --- a/test/.eslintrc +++ b/test/.eslintrc @@ -5,6 +5,7 @@ "rules": { "no-invalid-this": 0, "max-nested-callbacks": 0, - "callback-return": 0 + "callback-return": 0, + "prefer-template": 0, } -} \ No newline at end of file +} diff --git a/test/fixtures/%CD% b/test/fixtures/%CD% new file mode 100755 index 0000000..051009c --- /dev/null +++ b/test/fixtures/%CD% @@ -0,0 +1,3 @@ +#!/bin/bash + +echo special diff --git a/test/fixtures/%CD%.bat b/test/fixtures/%CD%.bat new file mode 100755 index 0000000..f248e1f --- /dev/null +++ b/test/fixtures/%CD%.bat @@ -0,0 +1,2 @@ +@echo off +echo special diff --git a/test/fixtures/echo.js b/test/fixtures/echo.js index 133540b..b14f68d 100755 --- a/test/fixtures/echo.js +++ b/test/fixtures/echo.js @@ -1,7 +1,7 @@ 'use strict'; -var args = process.argv.slice(2); +const args = process.argv.slice(2); -args.forEach(function (arg, index) { +args.forEach((arg, index) => { process.stdout.write(arg + (index < args.length - 1 ? '\n' : '')); }); diff --git a/test/fixtures/win-ppid.js b/test/fixtures/win-ppid.js index e9d03ff..8702fbc 100644 --- a/test/fixtures/win-ppid.js +++ b/test/fixtures/win-ppid.js @@ -1,11 +1,14 @@ #!/usr/bin/env node -var spawnSync = require('child_process').spawnSync || require('spawn-sync'); +'use strict'; + +const spawnSync = require('child_process').spawnSync; function ppidSync() { - var res = spawnSync('wmic', - ['process', 'where', '(processid=' + process.pid + ')', 'get', 'parentprocessid']); - var lines = res.stdout.toString().split(/\r?\n/); + const res = spawnSync('wmic', + ['process', 'where', `(processid=${process.pid})`, 'get', 'parentprocessid']); + const lines = res.stdout.toString().split(/\r?\n/); + return parseInt(lines[1].trim(), 10); } diff --git a/test/prepare.js b/test/prepare.js index ba83714..06905af 100644 --- a/test/prepare.js +++ b/test/prepare.js @@ -1,17 +1,15 @@ 'use strict'; -var glob = require('glob'); -var fs = require('fs'); -var path = require('path'); -var buffered = require('./util/buffered'); +const fs = require('fs'); +const path = require('path'); +const glob = require('glob'); +// Prepare fixtures +const fixturesDir = __dirname + '/fixtures'; -// Preare fixtures -var fixturesDir = __dirname + '/fixtures'; - -glob.sync('prepare_*', { cwd: __dirname + '/fixtures' }).forEach(function (file) { - var contents = fs.readFileSync(fixturesDir + '/' + file); - var finalFile = file.replace(/^prepare_/, '').replace(/\.sh$/, ''); +glob.sync('prepare_*', { cwd: __dirname + '/fixtures' }).forEach((file) => { + const contents = fs.readFileSync(fixturesDir + '/' + file); + const finalFile = file.replace(/^prepare_/, '').replace(/\.sh$/, ''); fs.writeFileSync(fixturesDir + '/' + finalFile, contents); fs.chmodSync(fixturesDir + '/' + finalFile, parseInt('0777', 8)); @@ -19,24 +17,10 @@ glob.sync('prepare_*', { cwd: __dirname + '/fixtures' }).forEach(function (file) process.stdout.write('Copied "' + file + '" to "' + finalFile + '"\n'); }); -// Install spawn-sync for older node versions -if (/^v0\.10\./.test(process.version)) { - process.stdout.write('Installing spawn-sync..\n'); - buffered('spawn', 'npm', ['install', 'spawn-sync'], { stdio: 'inherit' }, function (err) { - if (err) { - throw err; - } - - process.exit(); - }); -} - // Fix AppVeyor tests because Git bin folder is in PATH and it has a "echo" program there if (process.env.APPVEYOR) { process.env.PATH = process.env.PATH .split(path.delimiter) - .filter(function (entry) { - return !/\\git\\bin$/i.test(path.normalize(entry)); - }) + .filter((entry) => !/\\git\\bin$/i.test(path.normalize(entry))) .join(path.delimiter); } diff --git a/test/test.js b/test/test.js index 7e506ee..ad2f2ae 100644 --- a/test/test.js +++ b/test/test.js @@ -1,24 +1,22 @@ -'use strict'; +/* eslint prefer-template:0*/ -var fs = require('fs'); -var path = require('path'); -var expect = require('expect.js'); -var rimraf = require('rimraf'); -var mkdirp = require('mkdirp'); -var hasEmptyArgumentBug = require('../lib/util/hasEmptyArgumentBug'); -var spawn = require('../'); -var buffered = require('./util/buffered'); +'use strict'; -var isWin = process.platform === 'win32'; +const fs = require('fs'); +const path = require('path'); +const expect = require('expect.js'); +const rimraf = require('rimraf'); +const mkdirp = require('mkdirp'); +const run = require('./util/run'); -describe('cross-spawn', function () { - var methods = ['spawn', 'sync']; +const isWin = process.platform === 'win32'; - methods.forEach(function (method) { - describe(method, function () { - var originalPath = process.env.PATH; +describe('cross-spawn', () => { + run.methods.forEach((method) => { + describe(method, () => { + const originalPath = process.env.PATH; - before(function () { + before(() => { mkdirp.sync(__dirname + '/tmp'); }); @@ -26,18 +24,18 @@ describe('cross-spawn', function () { // Give it some time, RIMRAF was giving problems on windows this.timeout(10000); - rimraf(__dirname + '/tmp', function () { + rimraf(__dirname + '/tmp', () => { // Ignore errors, RIMRAF was giving problems on windows next(null); }); }); - afterEach(function () { + afterEach(() => { process.env.PATH = originalPath; }); - it('should support shebang in executables with /usr/bin/env', function (next) { - buffered(method, __dirname + '/fixtures/shebang', function (err, data, code) { + it('should support shebang in executables with /usr/bin/env', (next) => { + run(method, __dirname + '/fixtures/shebang', (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data).to.equal('shebang works!'); @@ -45,7 +43,7 @@ describe('cross-spawn', function () { // Test if the actual shebang file is resolved against the PATH process.env.PATH = path.normalize(__dirname + '/fixtures/') + path.delimiter + process.env.PATH; - buffered(method, 'shebang', function (err, data, code) { + run(method, 'shebang', (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data).to.equal('shebang works!'); @@ -55,14 +53,14 @@ describe('cross-spawn', function () { }); }); - it('should support shebang in executables with relative path', function (next) { - var executable = './' + path.relative(process.cwd(), __dirname + '/fixtures/shebang'); + it('should support shebang in executables with relative path', (next) => { + const executable = './' + path.relative(process.cwd(), __dirname + '/fixtures/shebang'); fs.writeFileSync(__dirname + '/tmp/shebang', '#!/usr/bin/env node\n\nprocess.stdout.write(\'yeah\');', { mode: parseInt('0777', 8) }); process.env.PATH = path.normalize(__dirname + '/tmp/') + path.delimiter + process.env.PATH; - buffered(method, executable, function (err, data, code) { + run(method, executable, (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data).to.equal('shebang works!'); @@ -71,14 +69,15 @@ describe('cross-spawn', function () { }); }); - it('should support shebang in executables with relative path that starts with `..`', function (next) { - var executable = '../' + path.basename(process.cwd()) + '/' + path.relative(process.cwd(), __dirname + '/fixtures/shebang'); + it('should support shebang in executables with relative path that starts with `..`', (next) => { + const executable = '../' + path.basename(process.cwd()) + + '/' + path.relative(process.cwd(), __dirname + '/fixtures/shebang'); fs.writeFileSync(__dirname + '/tmp/shebang', '#!/usr/bin/env node\n\nprocess.stdout.write(\'yeah\');', { mode: parseInt('0777', 8) }); process.env.PATH = path.normalize(__dirname + '/tmp/') + path.delimiter + process.env.PATH; - buffered(method, executable, function (err, data, code) { + run(method, executable, (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data).to.equal('shebang works!'); @@ -87,12 +86,12 @@ describe('cross-spawn', function () { }); }); - it('should support shebang in executables with extensions', function (next) { + it('should support shebang in executables with extensions', (next) => { fs.writeFileSync(__dirname + '/tmp/shebang_' + method + '.js', '#!/usr/bin/env node\n\nprocess.stdout.write(\'shebang with \ extension\');', { mode: parseInt('0777', 8) }); process.env.PATH = path.normalize(__dirname + '/tmp/') + path.delimiter + process.env.PATH; - buffered(method, __dirname + '/tmp/shebang_' + method + '.js', function (err, data, code) { + run(method, __dirname + '/tmp/shebang_' + method + '.js', (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data).to.equal('shebang with extension'); @@ -100,7 +99,7 @@ extension\');', { mode: parseInt('0777', 8) }); // Test if the actual shebang file is resolved against the PATH process.env.PATH = path.normalize(__dirname + '/fixtures/') + path.delimiter + process.env.PATH; - buffered(method, 'shebang_' + method + '.js', function (err, data, code) { + run(method, 'shebang_' + method + '.js', (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data).to.equal('shebang with extension'); @@ -110,8 +109,8 @@ extension\');', { mode: parseInt('0777', 8) }); }); }); - it('should expand using PATHEXT properly', function (next) { - buffered(method, __dirname + '/fixtures/foo', function (err, data, code) { + it('should expand using PATHEXT properly', (next) => { + run(method, __dirname + '/fixtures/foo', (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data.trim()).to.equal('foo'); @@ -120,8 +119,8 @@ extension\');', { mode: parseInt('0777', 8) }); }); }); - it('should handle commands with spaces', function (next) { - buffered(method, __dirname + '/fixtures/bar space', function (err, data, code) { + it('should handle commands with spaces', (next) => { + run(method, __dirname + '/fixtures/bar space', ['foo bar'], (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data.trim()).to.equal('bar'); @@ -130,8 +129,8 @@ extension\');', { mode: parseInt('0777', 8) }); }); }); - it('should handle commands with special shell chars', function (next) { - buffered(method, __dirname + '/fixtures/()%!^&;, ', function (err, data, code) { + it('should handle commands with special shell chars', (next) => { + run(method, __dirname + '/fixtures/()%!^&;, ', (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data.trim()).to.equal('special'); @@ -140,12 +139,12 @@ extension\');', { mode: parseInt('0777', 8) }); }); }); - it('should handle arguments with quotes', function (next) { - buffered(method, 'node', [ + it('should handle arguments with quotes', (next) => { + run(method, 'node', [ __dirname + '/fixtures/echo', '"foo"', 'foo"bar"foo', - ], function (err, data, code) { + ], (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data).to.equal('"foo"\nfoo"bar"foo'); @@ -154,36 +153,26 @@ extension\');', { mode: parseInt('0777', 8) }); }); }); - it('should handle empty arguments', function (next) { - buffered(method, 'node', [ + it('should handle empty arguments', (next) => { + run(method, 'node', [ __dirname + '/fixtures/echo', 'foo', '', 'bar', - ], function (err, data, code) { + ], (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data).to.equal('foo\n\nbar'); - buffered(method, 'echo', [ - 'foo', - '', - 'bar', - ], function (err, data, code) { - expect(err).to.not.be.ok(); - expect(code).to.be(0); - expect(data.trim()).to.equal('foo bar'); - - next(); - }); + next(); }); }); - it('should handle non-string arguments', function (next) { - buffered(method, 'node', [ + it('should handle non-string arguments', (next) => { + run(method, 'node', [ __dirname + '/fixtures/echo', 1234, - ], function (err, data, code) { + ], (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data).to.equal('1234'); @@ -192,12 +181,12 @@ extension\');', { mode: parseInt('0777', 8) }); }); }); - it('should handle arguments with spaces', function (next) { - buffered(method, 'node', [ + it('should handle arguments with spaces', (next) => { + run(method, 'node', [ __dirname + '/fixtures/echo', 'I am', 'André Cruz', - ], function (err, data, code) { + ], (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data).to.equal('I am\nAndré Cruz'); @@ -206,13 +195,13 @@ extension\');', { mode: parseInt('0777', 8) }); }); }); - it('should handle arguments with \\"', function (next) { - buffered(method, 'node', [ + it('should handle arguments with \\"', (next) => { + run(method, 'node', [ __dirname + '/fixtures/echo', 'foo', '\\"', 'bar', - ], function (err, data, code) { + ], (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data).to.equal('foo\n\\"\nbar'); @@ -221,13 +210,13 @@ extension\');', { mode: parseInt('0777', 8) }); }); }); - it('should handle arguments that end with \\', function (next) { - buffered(method, 'node', [ + it('should handle arguments that end with \\', (next) => { + run(method, 'node', [ __dirname + '/fixtures/echo', 'foo', 'bar\\', 'baz', - ], function (err, data, code) { + ], (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data).to.equal('foo\nbar\\\nbaz'); @@ -236,12 +225,13 @@ extension\');', { mode: parseInt('0777', 8) }); }); }); - it('should handle arguments that contain shell special chars', function (next) { - buffered(method, 'node', [ - __dirname + '/fixtures/echo', + it('should handle arguments that contain shell special chars', (next) => { + const args = [ 'foo', '()', 'foo', + '[]', + 'foo', '%!', 'foo', '^<', @@ -252,59 +242,68 @@ extension\');', { mode: parseInt('0777', 8) }); 'foo', ', ', 'foo', - ], function (err, data, code) { + '!=', + 'foo', + '\\*', + 'foo', + '"f"', + 'foo', + '?.', + 'foo', + '=`', + 'foo', + '\'', + 'foo', + ]; + + run(method, 'node', [__dirname + '/fixtures/echo'].concat(args), (err, data, code) => { + expect(err).to.not.be.ok(); + expect(code).to.be(0); + expect(data).to.equal(args.join('\n')); + + next(); + }); + }); + + it('should handle escaping of quotes followed by special meta chars', (next) => { + const arg = '"(foo|bar>baz|foz)"'; + + run(method, 'node', [ + __dirname + '/fixtures/echo', + arg, + ], (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); - expect(data).to.equal('foo\n()\nfoo\n%!\nfoo\n^<\nfoo\n>&\nfoo\n|;\nfoo\n, \nfoo'); + expect(data).to.equal(arg); next(); }); }); - it('should handle special arguments when using echo', function (next) { - buffered(method, 'echo', ['foo\\"foo\\foo&bar"foo\'bar'], function (err, data, code) { + it('should handle commands with names of environment variables', (next) => { + run(method, __dirname + '/fixtures/%CD%', (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); - expect(data.trim()).to.equal('foo\\"foo\\foo&bar"foo\'bar'); - - buffered(method, 'echo', [ - 'foo', - '()', - 'foo', - '%!', - 'foo', - '^<', - 'foo', - '>&', - 'foo', - '|;', - 'foo', - ', ', - 'foo', - ], function (err, data, code) { - expect(err).to.not.be.ok(); - expect(code).to.be(0); - expect(data.trim()).to.equal('foo () foo %! foo ^< foo >& foo |; foo , foo'); + expect(data.trim()).to.equal('special'); - next(); - }); + next(); }); }); - it('should handle optional args correctly', function (next) { - buffered(method, __dirname + '/fixtures/foo', function (err, data, code) { + it('should handle optional args correctly', (next) => { + run(method, __dirname + '/fixtures/foo', (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); - buffered(method, __dirname + '/fixtures/foo', { + run(method, __dirname + '/fixtures/foo', { stdio: ['pipe', 'pipe', 'pipe'], - }, function (err, data, code) { + }, (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); - buffered(method, __dirname + '/fixtures/foo', null, { + run(method, __dirname + '/fixtures/foo', null, { stdio: ['pipe', 'pipe', 'pipe'], - }, function (err, data, code) { + }, (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); @@ -314,11 +313,11 @@ extension\');', { mode: parseInt('0777', 8) }); }); }); - it('should not mutate args nor options', function (next) { - var args = []; - var options = {}; + it('should not mutate args nor options', (next) => { + const args = []; + const options = {}; - buffered(method, __dirname + '/fixtures/foo', function (err, data, code) { + run(method, __dirname + '/fixtures/foo', options, (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); @@ -329,8 +328,8 @@ extension\');', { mode: parseInt('0777', 8) }); }); }); - it('should give correct exit code', function (next) { - buffered(method, 'node', [__dirname + '/fixtures/exit'], function (err, data, code) { + it('should give correct exit code', (next) => { + run(method, 'node', [__dirname + '/fixtures/exit'], (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(25); @@ -338,8 +337,8 @@ extension\');', { mode: parseInt('0777', 8) }); }); }); - it('should work with a relative command', function (next) { - buffered(method, path.relative(process.cwd(), __dirname + '/fixtures/foo'), function (err, data, code) { + it('should work with a relative command', (next) => { + run(method, path.relative(process.cwd(), __dirname + '/fixtures/foo'), (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data.trim()).to.equal('foo'); @@ -348,7 +347,7 @@ extension\');', { mode: parseInt('0777', 8) }); return next(); } - buffered(method, path.relative(process.cwd(), __dirname + '/fixtures/foo.bat'), function (err, data, code) { + run(method, path.relative(process.cwd(), __dirname + '/fixtures/foo.bat'), (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data.trim()).to.equal('foo'); @@ -359,13 +358,12 @@ extension\');', { mode: parseInt('0777', 8) }); }); it('should emit "error" and "close" if command does not exist', function (next) { - var errors; - var spawned = spawn[method]('somecommandthatwillneverexist'); + const spawned = run(method, 'somecommandthatwillneverexist'); this.timeout(5000); function assertError(err) { - var syscall = method === 'sync' ? 'spawnSync' : 'spawn'; + const syscall = run.isMethodSync(method) ? 'spawnSync' : 'spawn'; expect(err).to.be.an(Error); expect(err.message).to.contain(syscall); @@ -377,152 +375,125 @@ extension\');', { mode: parseInt('0777', 8) }); expect(err.syscall).to.not.contain('undefined'); } - if (method === 'spawn') { - errors = []; - - spawned - .on('error', function (err) { - errors.push(err); - }) - .on('exit', function () { - spawned.removeAllListeners(); - next(new Error('Should not emit exit')); - }) - .on('close', function (code, signal) { - expect(code).to.not.be(0); - expect(signal).to.be(null); - - setTimeout(function () { - expect(errors).to.have.length(1); - assertError(errors[0]); - - next(); - }, 1000); - }); - } else { + if (run.isMethodSync(method)) { assertError(spawned.error); - next(); + return next(); } + + const errors = []; + + spawned + .on('error', (err) => { + errors.push(err); + }) + .on('exit', () => { + spawned.removeAllListeners(); + next(new Error('Should not emit exit')); + }) + .on('close', (code, signal) => { + expect(code).to.not.be(0); + expect(signal).to.be(null); + + setTimeout(() => { + expect(errors).to.have.length(1); + assertError(errors[0]); + + next(); + }, 1000); + }); }); it('should NOT emit "error" if shebang command does not exist', function (next) { - var spawned = spawn[method](__dirname + '/fixtures/shebang_enoent'); - var exited; - var timeout; + const spawned = run(method, __dirname + '/fixtures/shebang_enoent'); + let exited; + let timeout; this.timeout(5000); - if (method === 'spawn') { - spawned - .on('error', function () { - spawned.removeAllListeners(); - clearTimeout(timeout); - next(new Error('Should not emit error')); - }) - .on('exit', function () { - exited = true; - }) - .on('close', function (code, signal) { - expect(code).to.not.be(0); - expect(signal).to.be(null); - expect(exited).to.be(true); - - timeout = setTimeout(next, 1000); - }); - } else { + if (run.isMethodSync(method)) { expect(spawned.error).to.not.be.ok(); - next(); + return next(); } + + spawned + .on('error', () => { + spawned.removeAllListeners(); + clearTimeout(timeout); + next(new Error('Should not emit error')); + }) + .on('exit', () => { + exited = true; + }) + .on('close', (code, signal) => { + expect(code).to.not.be(0); + expect(signal).to.be(null); + expect(exited).to.be(true); + + timeout = setTimeout(next, 1000); + }); }); it('should NOT emit "error" if the command actual exists but exited with 1', function (next) { - var spawned = spawn[method](__dirname + '/fixtures/exit1'); - var exited; - var timeout; + const spawned = run(method, __dirname + '/fixtures/exit1'); + let exited; + let timeout; this.timeout(5000); - if (method === 'spawn') { - spawned - .on('error', function () { - spawned.removeAllListeners(); - clearTimeout(timeout); - next(new Error('Should not emit error')); - }) - .on('exit', function () { - exited = true; - }) - .on('close', function (code, signal) { - expect(code).to.not.be(0); - expect(signal).to.be(null); - expect(exited).to.be(true); - - timeout = setTimeout(next, 1000); - }); - } else { + if (run.isMethodSync(method)) { expect(spawned.error).to.not.be.ok(); - next(); + return next(); } + + spawned + .on('error', () => { + spawned.removeAllListeners(); + clearTimeout(timeout); + next(new Error('Should not emit error')); + }) + .on('exit', () => { + exited = true; + }) + .on('close', (code, signal) => { + expect(code).to.not.be(0); + expect(signal).to.be(null); + expect(exited).to.be(true); + + timeout = setTimeout(next, 1000); + }); }); if (isWin) { - it('should use nodejs\' spawn when option.shell is specified', function (next) { - buffered(method, 'echo', ['%RANDOM%'], { shell: true }, function (err, data, code) { + it('should use nodejs\' spawn when options.shell is specified', (next) => { + run(method, 'echo', ['%RANDOM%'], { shell: true }, (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data.trim()).to.match(/\d+/); - buffered(method, 'echo', ['%RANDOM%'], { shell: false }, function (err, data) { - // In some windows versions, the echo exists outside the shell as echo.exe so we must account for that here - if (err) { - expect(err).to.be.an(Error); - expect(err.message).to.contain('ENOENT'); - } else { - expect(data.trim()).to.equal('%RANDOM%'); - } - - next(); - }); + next(); }); }); } else { - it('should use nodejs\' spawn when option.shell is specified', function (next) { - buffered(method, 'echo', ['hello &&', 'echo there'], { shell: true }, function (err, data, code) { + it('should use nodejs\' spawn when options.shell is specified', (next) => { + run(method, 'echo', ['hello &&', 'echo there'], { shell: true }, (err, data, code) => { expect(err).to.not.be.ok(); expect(code).to.be(0); expect(data.trim()).to.equal('hello\nthere'); - buffered(method, 'echo', ['hello &&', 'echo there'], { shell: false }, function (err, data) { - expect(err).to.not.be.ok(); - expect(code).to.be(0); - expect(data.trim()).to.equal('hello && echo there'); - - next(); - }); + next(); }); }); } - if (isWin) { - if (hasEmptyArgumentBug) { - it('should spawn a shell for a .exe on old Node', function (next) { - buffered(method, __dirname + '/fixtures/win-ppid.js', function (err, data, code) { - expect(err).to.not.be.ok(); - expect(code).to.be(0); - expect(data.trim()).to.not.equal('' + process.pid); - next(); - }); - }); - } else { - it('should NOT spawn a shell for a .exe', function (next) { - buffered(method, __dirname + '/fixtures/win-ppid.js', function (err, data, code) { - expect(err).to.not.be.ok(); - expect(code).to.be(0); - expect(data.trim()).to.equal('' + process.pid); - next(); - }); + if (isWin && !run.isForceShell(method)) { + it('should NOT spawn a shell for a .exe', (next) => { + run(method, __dirname + '/fixtures/win-ppid.js', (err, data, code) => { + expect(err).to.not.be.ok(); + expect(code).to.be(0); + expect(data.trim()).to.equal('' + process.pid); + next(); }); - } + }); } }); }); diff --git a/test/util/buffered.js b/test/util/buffered.js deleted file mode 100644 index 2c0ce59..0000000 --- a/test/util/buffered.js +++ /dev/null @@ -1,49 +0,0 @@ -'use strict'; - -var spawn = require('../../index'); -var once = require('once'); - -function buffered(method, command, args, options, callback) { - var cp; - var stdout; - var stderr; - var results; - - if (typeof options === 'function') { - callback = options; - options = null; - } - - if (typeof args === 'function') { - callback = args; - args = options = null; - } - - if (method === 'sync') { - results = spawn.sync(command, args, options); - callback(results.error, results.stdout ? results.stdout.toString() : null, results.status); - } else { - cp = spawn(command, args, options); - stdout = stderr = null; - callback = once(callback); - - cp.stdout && cp.stdout.on('data', function (buffer) { - stdout = stdout || ''; - stdout += buffer.toString(); - }); - - cp.stderr && cp.stderr.on('data', function (buffer) { - stderr = stderr || ''; - stderr += buffer.toString(); - }); - - cp.on('error', callback); - - cp.on('close', function (code) { - code !== 0 && stderr && console.warn(stderr); - callback(null, stdout, code); - }); - } -} - -module.exports = buffered; diff --git a/test/util/run.js b/test/util/run.js new file mode 100644 index 0000000..461ad7d --- /dev/null +++ b/test/util/run.js @@ -0,0 +1,77 @@ +'use strict'; + +const spawn = require('../../index'); +const once = require('once'); + +function isForceShell(method) { + return /-force-shell$/.test(method); +} + +function isMethodSync(method) { + return /^sync(-|$)/.test(method); +} + +function run(method, command, args, options, callback) { + if (typeof options === 'function') { + callback = options; + options = null; + } + + if (typeof args === 'function') { + callback = args; + args = options = null; + } + + callback = callback || (() => {}); + + // Are we forcing the shell? + if (isForceShell(method)) { + if (args && !Array.isArray(args)) { + options = args; + args = null; + } + + method = method.replace(/-force-shell$/, ''); + options = Object.assign({ forceShell: true }, options); + } + + // Run sync version + if (method === 'sync') { + const results = spawn.sync(command, args, options); + + callback(results.error, results.stdout ? results.stdout.toString() : null, results.status); + + return results; + } + + // Run normal version + const cp = spawn(command, args, options); + let stdout = null; + let stderr = null; + + callback = once(callback); + + cp.stdout && cp.stdout.on('data', (buffer) => { + stdout = stdout || ''; + stdout += buffer.toString(); + }); + + cp.stderr && cp.stderr.on('data', (buffer) => { + stderr = stderr || ''; + stderr += buffer.toString(); + }); + + cp.on('error', callback); + + cp.on('close', (code) => { + code !== 0 && stderr && console.warn(stderr); + callback(null, stdout, code); + }); + + return cp; +} + +module.exports = run; +module.exports.methods = ['spawn-force-shell', 'spawn', 'sync-force-shell', 'sync']; +module.exports.isMethodSync = isMethodSync; +module.exports.isForceShell = isForceShell;