Skip to content

Commit

Permalink
Fix: Avoid shell mangling during eslint --init (#8936)
Browse files Browse the repository at this point in the history
If you ran ‘eslint --init’ and selected the Google style guide, it
would end up calling execSync("npm i --save-dev eslint@>=4.1.1").
Because execSync spawns the child through a shell, this had the effect
of running ‘npm i --save-dev eslint@’ with its output redirected to a
new file named ‘=4.1.1’, leaving the wrong version in package.json.

Fix this by spawning processes using the cross-spawn package, which
avoids spawning a shell at all on Unix and quotes the arguments
appropriately on Windows.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
  • Loading branch information
andersk authored and not-an-aardvark committed Jul 16, 2017
1 parent 10c3d78 commit 55bc35d
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 14 deletions.
15 changes: 8 additions & 7 deletions lib/util/npm-util.js
Expand Up @@ -10,7 +10,7 @@
//------------------------------------------------------------------------------

const fs = require("fs"),
childProcess = require("child_process"),
spawn = require("cross-spawn"),
path = require("path"),
log = require("../logging");

Expand Down Expand Up @@ -50,10 +50,10 @@ function findPackageJson(startDir) {
* @returns {void}
*/
function installSyncSaveDev(packages) {
if (Array.isArray(packages)) {
packages = packages.join(" ");
if (!Array.isArray(packages)) {
packages = [packages];
}
childProcess.execSync(`npm i --save-dev ${packages}`, { stdio: "inherit", encoding: "utf8" });
spawn.sync("npm", ["i", "--save-dev"].concat(packages), { stdio: "inherit" });
}

/**
Expand All @@ -62,10 +62,11 @@ function installSyncSaveDev(packages) {
* @returns {string[]} Gotten peerDependencies.
*/
function fetchPeerDependencies(packageName) {
const fetchedText = childProcess.execSync(
`npm show --json ${packageName} peerDependencies`,
const fetchedText = spawn.sync(
"npm",
["show", "--json", packageName, "peerDependencies"],
{ encoding: "utf8" }
).trim();
).stdout.trim();

return JSON.parse(fetchedText || "{}");
}
Expand Down
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -39,6 +39,7 @@
"babel-code-frame": "^6.22.0",
"chalk": "^1.1.3",
"concat-stream": "^1.6.0",
"cross-spawn": "^5.1.0",
"debug": "^2.6.8",
"doctrine": "^2.0.0",
"eslint-scope": "^3.7.1",
Expand Down
17 changes: 10 additions & 7 deletions tests/lib/util/npm-util.js
Expand Up @@ -9,7 +9,7 @@
//------------------------------------------------------------------------------

const assert = require("chai").assert,
childProcess = require("child_process"),
spawn = require("cross-spawn"),
sinon = require("sinon"),
npmUtil = require("../../../lib/util/npm-util"),
log = require("../../../lib/logging"),
Expand Down Expand Up @@ -170,31 +170,34 @@ describe("npmUtil", () => {

describe("installSyncSaveDev()", () => {
it("should invoke npm to install a single desired package", () => {
const stub = sandbox.stub(childProcess, "execSync");
const stub = sandbox.stub(spawn, "sync");

npmUtil.installSyncSaveDev("desired-package");
assert(stub.calledOnce);
assert.equal(stub.firstCall.args[0], "npm i --save-dev desired-package");
assert.equal(stub.firstCall.args[0], "npm");
assert.deepEqual(stub.firstCall.args[1], ["i", "--save-dev", "desired-package"]);
stub.restore();
});

it("should accept an array of packages to install", () => {
const stub = sandbox.stub(childProcess, "execSync");
const stub = sandbox.stub(spawn, "sync");

npmUtil.installSyncSaveDev(["first-package", "second-package"]);
assert(stub.calledOnce);
assert.equal(stub.firstCall.args[0], "npm i --save-dev first-package second-package");
assert.equal(stub.firstCall.args[0], "npm");
assert.deepEqual(stub.firstCall.args[1], ["i", "--save-dev", "first-package", "second-package"]);
stub.restore();
});
});

describe("fetchPeerDependencies()", () => {
it("should execute 'npm show --json <packageName> peerDependencies' command", () => {
const stub = sandbox.stub(childProcess, "execSync").returns("");
const stub = sandbox.stub(spawn, "sync").returns({ stdout: "" });

npmUtil.fetchPeerDependencies("desired-package");
assert(stub.calledOnce);
assert.equal(stub.firstCall.args[0], "npm show --json desired-package peerDependencies");
assert.equal(stub.firstCall.args[0], "npm");
assert.deepEqual(stub.firstCall.args[1], ["show", "--json", "desired-package", "peerDependencies"]);
stub.restore();
});
});
Expand Down

0 comments on commit 55bc35d

Please sign in to comment.