Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix: fix certain arguments not being correctly escaped or causing bat…
…ch syntax error

More specifically:
- 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
- Fix a bug were a command containing `%x%` would be replaced with the contents of the `x` environment variable, fixes #51

This was resolved by using `^` to escape all meta chars.
Additionally, double escaping was necessary for cmd-shim files located in `node_modules./bin/`.

Also, this commit was a major overhaul:
- Upgrade tooling
- Upgrate project to es6 (node v4)
- Fix commands as posix unix relatixe paths not working correctly
- Fix `options` argument being mutated
- Improve compliance with node's ENOENT errors
- Improve detection of node's shell option support
- Migrate project to moxystudio

BREAKING CHANGE: remove support for older nodejs versions, only node >= 4 is supported

Fixes #82, #51
  • Loading branch information
satazor committed Jan 22, 2018
1 parent a00d9e2 commit 900cf10
Show file tree
Hide file tree
Showing 38 changed files with 8,543 additions and 922 deletions.
1 change: 1 addition & 0 deletions .eslintignore
@@ -0,0 +1 @@
coverage/
6 changes: 3 additions & 3 deletions .eslintrc
@@ -1,7 +1,7 @@
{
"root": true,
"extends": [
"@satazor/eslint-config/es5",
"@satazor/eslint-config/addons/node"
"eslint-config-moxy/es6",
"eslint-config-moxy/addons/node"
]
}
}
4 changes: 2 additions & 2 deletions .gitignore
@@ -1,5 +1,5 @@
node_modules/
npm-debug.*
coverage/
test/fixtures/(*
test/fixtures/shebang_noenv
test/tmp
test/tmp/
13 changes: 9 additions & 4 deletions .travis.yml
@@ -1,7 +1,12 @@
language: node_js
node_js:
- '0.10'
- '0.12'
- '4'
- '4.7'
- '4.8'
- '5.6'
- '5.7'
- '6'
- '7'
- 'node'
- 'lts/*'
after_success:
- "npm i codecov"
- "node_modules/.bin/codecov"
19 changes: 18 additions & 1 deletion CHANGELOG.md
@@ -1,4 +1,21 @@
## 5.0.0 - 2016-10-30
# Change Log

All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines.

<a name="5.1.0"></a>
## [5.1.0](https://github.com/moxystudio/node-cross-spawn/compare/5.0.1...5.1.0) (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)


<a name="5.0.1"></a>
## [5.0.1](https://github.com/moxystudio/node-cross-spawn/compare/5.0.0...5.0.1) (2016-11-04)

- Fix `options.shell` support for NodeJS v7


<a name="5.0.0"></a>
# [5.0.0](https://github.com/moxystudio/node-cross-spawn/compare/4.0.2...5.0.0) (2016-10-30)

- Add support for `options.shell`
- Improve parsing of shebangs by using [`shebang-command`](https://github.com/kevva/shebang-command) module
Expand Down
36 changes: 20 additions & 16 deletions README.md
Expand Up @@ -5,15 +5,15 @@
[npm-url]:https://npmjs.org/package/cross-spawn
[downloads-image]:http://img.shields.io/npm/dm/cross-spawn.svg
[npm-image]:http://img.shields.io/npm/v/cross-spawn.svg
[travis-url]:https://travis-ci.org/IndigoUnited/node-cross-spawn
[travis-image]:http://img.shields.io/travis/IndigoUnited/node-cross-spawn/master.svg
[travis-url]:https://travis-ci.org/moxystudio/node-cross-spawn
[travis-image]:http://img.shields.io/travis/moxystudio/node-cross-spawn/master.svg
[appveyor-url]:https://ci.appveyor.com/project/satazor/node-cross-spawn
[appveyor-image]:https://img.shields.io/appveyor/ci/satazor/node-cross-spawn/master.svg
[david-dm-url]:https://david-dm.org/IndigoUnited/node-cross-spawn
[david-dm-image]:https://img.shields.io/david/IndigoUnited/node-cross-spawn.svg
[david-dm-dev-url]:https://david-dm.org/IndigoUnited/node-cross-spawn?type=dev
[david-dm-dev-image]:https://img.shields.io/david/dev/IndigoUnited/node-cross-spawn.svg
[greenkeeper-image]:https://badges.greenkeeper.io/IndigoUnited/node-cross-spawn.svg
[david-dm-url]:https://david-dm.org/moxystudio/node-cross-spawn
[david-dm-image]:https://img.shields.io/david/moxystudio/node-cross-spawn.svg
[david-dm-dev-url]:https://david-dm.org/moxystudio/node-cross-spawn?type=dev
[david-dm-dev-image]:https://img.shields.io/david/dev/moxystudio/node-cross-spawn.svg
[greenkeeper-image]:https://badges.greenkeeper.io/moxystudio/node-cross-spawn.svg
[greenkeeper-url]:https://greenkeeper.io/

A cross platform solution to node's spawn and spawnSync.
Expand All @@ -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

Expand All @@ -35,7 +31,9 @@ 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 `<v4.8`
- It does not allow you to run `del` or `dir`
- Has problems running commands with [spaces](https://github.com/nodejs/node/issues/7367)
- Has problems running commands with posix relative paths (e.g.: `my-folder/my-executable`)
- Circuvents an [issue](https://github.com/moxystudio/node-cross-spawn/issues/82) around command shims (files in node_modules/.bin/), where arguments with quotes and parenthesis would result in an [invalid syntax error](ADD_LINK_TO_TESTS)

All these issues are handled correctly by `cross-spawn`.
There are some known modules, such as [win-spawn](https://github.com/ForbesLindesay/win-spawn), that try to solve this but they are either broken or provide faulty escaping of shell arguments.
Expand All @@ -59,20 +57,26 @@ var results = spawn.sync('npm', ['list', '-g', '-depth', '0'], { stdio: 'inherit

## Caveats

#### `options.shell` as an alternative to `cross-spawn`
### Using `options.shell` as an alternative to `cross-spawn`

Starting from node `v4.8`, `spawn` has a `shell` option that allows you run commands from within a shell. This new option solves most of the problems that `cross-spawn` attempts to solve, but:

- It's not supported in node `<v4.8`
- It has no support for shebangs on Windows
- You must manually escape the command and arguments which is very error prone, specially when passing user input
- It just solves the [PATHEXT](https://github.com/joyent/node/issues/2318) issue from the [Why](#why) section

If you are using the `shell` option to spawn a command in a cross platform way, consider using `cross-spawn` instead. You have been warned.

### `options.shell` support

While `cross-spawn` adds support for `options.shell` in node `<v4.8`, all of its enhancements are disabled.

This mimics the Node.js behavior. More specifically, the command and its arguments will not be automatically escaped nor shebang support will be offered. This is by design because if you are using `options.shell` you are probably targeting a specific platform anyway and you don't want things to get into your way.

#### Shebangs
### Shebangs support

While `cross-spawn` handles shebangs on Windows, its support is limited: e.g.: it doesn't handle arguments after the path, e.g.: `#!/bin/bash -e`.
While `cross-spawn` handles shebangs on Windows, its support is limited. More specifically, it just supports `#!/usr/bin/env <program>` where `<program>` must not contain any arguments.
If you would like to have the shebang support improved, feel free to contribute via a pull-request.

Remember to always test your code on Windows!

Expand Down
33 changes: 22 additions & 11 deletions appveyor.yml
@@ -1,25 +1,32 @@
# appveyor file
# http://www.appveyor.com/docs/appveyor-yml

# build version format
version: "{build}"

# fix lineendings in Windows
# Fix line endings in Windows
init:
- git config --global core.autocrlf input

# what combinations to test
# If we are running on Node <6, we must install npm v3 otherwise
# there will be intermitent errors when running `npm install`
environment:
matrix:
- nodejs_version: 0.10
- nodejs_version: 0.12
- nodejs_version: 4
- nodejs_version: 4.7
npm_version: ^3.0.0
- nodejs_version: 4.8
npm_version: ^3.0.0
- nodejs_version: 5.6
npm_version: ^3.0.0
- nodejs_version: 5.7
npm_version: ^3.0.0
- nodejs_version: 6
- nodejs_version: 7
- nodejs_version: 8
- nodejs_version: 9

# get the latest stable version of Node 0.STABLE.latest
install:
- ps: Install-Product node $env:nodejs_version
- ps: |
if ($env:npm_version)
{
npm install -g npm@$env:npm_version
}
- npm install

build: off
Expand All @@ -28,3 +35,7 @@ test_script:
- node --version
- npm --version
- cmd: npm test --no-color

after_test:
- "npm i codecov"
- "node_modules/.bin/codecov"
36 changes: 8 additions & 28 deletions index.js
@@ -1,20 +1,15 @@
'use strict';

var cp = require('child_process');
var parse = require('./lib/parse');
var enoent = require('./lib/enoent');

var cpSpawnSync = cp.spawnSync;
const cp = require('child_process');
const parse = require('./lib/parse');
const enoent = require('./lib/enoent');

function spawn(command, args, options) {
var parsed;
var spawned;

// Parse the arguments
parsed = parse(command, args, options);
const parsed = parse(command, args, options);

// Spawn the child process
spawned = cp.spawn(parsed.command, parsed.args, parsed.options);
const spawned = cp.spawn(parsed.command, parsed.args, parsed.options);

// Hook into child process "exit" event to emit an error if the command
// does not exists, see: https://github.com/IndigoUnited/node-cross-spawn/issues/16
Expand All @@ -24,28 +19,13 @@ function spawn(command, args, options) {
}

function spawnSync(command, args, options) {
var parsed;
var result;

if (!cpSpawnSync) {
try {
cpSpawnSync = require('spawn-sync'); // eslint-disable-line global-require
} catch (ex) {
throw new Error(
'In order to use spawnSync on node 0.10 or older, you must ' +
'install spawn-sync:\n\n' +
' npm install spawn-sync --save'
);
}
}

// Parse the arguments
parsed = parse(command, args, options);
const parsed = parse(command, args, options);

// Spawn the child process
result = cpSpawnSync(parsed.command, parsed.args, parsed.options);
const result = cp.spawnSync(parsed.command, parsed.args, parsed.options);

// Analyze if the command does not exists, see: https://github.com/IndigoUnited/node-cross-spawn/issues/16
// Analyze if the command does not exist, see: https://github.com/IndigoUnited/node-cross-spawn/issues/16
result.error = result.error || enoent.verifyENOENTSync(result.status, parsed);

return result;
Expand Down
56 changes: 21 additions & 35 deletions lib/enoent.js
@@ -1,43 +1,37 @@
'use strict';

var isWin = process.platform === 'win32';
var resolveCommand = require('./util/resolveCommand');

var isNode10 = process.version.indexOf('v0.10.') === 0;

function notFoundError(command, syscall) {
var err;

err = new Error(syscall + ' ' + command + ' ENOENT');
err.code = err.errno = 'ENOENT';
err.syscall = syscall + ' ' + command;

return err;
const isWin = process.platform === 'win32';

function notFoundError(original, syscall) {
return Object.assign(new Error(`${syscall} ${original.command} ENOENT`), {
code: 'ENOENT',
errno: 'ENOENT',
syscall: `${syscall} ${original.command}`,
path: original.command,
spawnargs: original.args,
});
}

function hookChildProcess(cp, parsed) {
var originalEmit;

if (!isWin) {
return;
}

originalEmit = cp.emit;
cp.emit = function (name, arg1) {
var err;
const originalEmit = cp.emit;

cp.emit = function (name, arg1) {
// If emitting "exit" event and exit code is 1, we need to check if
// the command exists and emit an "error" instead
// See: https://github.com/IndigoUnited/node-cross-spawn/issues/16
// See https://github.com/IndigoUnited/node-cross-spawn/issues/16
if (name === 'exit') {
err = verifyENOENT(arg1, parsed, 'spawn');
const err = verifyENOENT(arg1, parsed, 'spawn');

if (err) {
return originalEmit.call(cp, 'error', err);
}
}

return originalEmit.apply(cp, arguments);
return originalEmit.apply(cp, arguments); // eslint-disable-line prefer-rest-params
};
}

Expand All @@ -54,20 +48,12 @@ function verifyENOENTSync(status, parsed) {
return notFoundError(parsed.original, 'spawnSync');
}

// If we are in node 10, then we are using spawn-sync; if it exited
// with -1 it probably means that the command does not exist
if (isNode10 && status === -1) {
parsed.file = isWin ? parsed.file : resolveCommand(parsed.original);

if (!parsed.file) {
return notFoundError(parsed.original, 'spawnSync');
}
}

return null;
}

module.exports.hookChildProcess = hookChildProcess;
module.exports.verifyENOENT = verifyENOENT;
module.exports.verifyENOENTSync = verifyENOENTSync;
module.exports.notFoundError = notFoundError;
module.exports = {
hookChildProcess,
verifyENOENT,
verifyENOENTSync,
notFoundError,
};

0 comments on commit 900cf10

Please sign in to comment.