diff --git a/src/which.js b/src/which.js index 0e5433c0..cc497384 100644 --- a/src/which.js +++ b/src/which.js @@ -13,13 +13,33 @@ common.register('which', _which, { // set on Windows. var XP_DEFAULT_PATHEXT = '.com;.exe;.bat;.cmd;.vbs;.vbe;.js;.jse;.wsf;.wsh'; +// For earlier versions of NodeJS that doesn't have a list of constants (< v6) +var FILE_EXECUTABLE_MODE = 1; + +function isWindowsPlatform() { + return process.platform === 'win32'; +} + // Cross-platform method for splitting environment `PATH` variables function splitPath(p) { return p ? p.split(path.delimiter) : []; } +// Tests are running all cases for this func but it stays uncovered by codecov due to unknown reason +/* istanbul ignore next */ +function isExecutable(pathName) { + try { + // TODO(node-support): replace with fs.constants.X_OK once remove support for node < v6 + fs.accessSync(pathName, FILE_EXECUTABLE_MODE); + } catch (err) { + return false; + } + return true; +} + function checkPath(pathName) { - return fs.existsSync(pathName) && !common.statFollowLinks(pathName).isDirectory(); + return fs.existsSync(pathName) && !common.statFollowLinks(pathName).isDirectory() + && (isWindowsPlatform() || isExecutable(pathName)); } //@ @@ -37,9 +57,8 @@ function checkPath(pathName) { function _which(options, cmd) { if (!cmd) common.error('must specify command'); - var isWindows = process.platform === 'win32'; - var pathEnv = process.env.path || process.env.Path || process.env.PATH; - var pathArray = splitPath(pathEnv); + var isWindows = isWindowsPlatform(); + var pathArray = splitPath(process.env.PATH); var queryMatches = []; diff --git a/test/resources/which/node b/test/resources/which/node new file mode 100644 index 00000000..0b917ba3 --- /dev/null +++ b/test/resources/which/node @@ -0,0 +1 @@ +text file, not an executable diff --git a/test/which.js b/test/which.js index 530fcd2c..f81d9e0e 100644 --- a/test/which.js +++ b/test/which.js @@ -1,4 +1,5 @@ import fs from 'fs'; +import path from 'path'; import test from 'ava'; @@ -69,5 +70,27 @@ test('Searching with -a flag returns an array with first item equals to the regu t.falsy(shell.error()); t.truthy(resultForWhich); t.truthy(resultForWhichA); - t.is(resultForWhich.toString(), resultForWhichA[0].toString()); + t.is(resultForWhich.toString(), resultForWhichA[0]); +}); + +test('None executable files does not appear in the result list', t => { + const commandName = 'node'; // Should be an existing command + const extraPath = path.resolve(__dirname, 'resources', 'which'); + const matchingFile = path.resolve(extraPath, commandName); + const pathEnv = process.env.PATH; + + // make sure that file is exists (will throw error otherwise) + t.truthy(fs.existsSync(matchingFile)); + + process.env.PATH = extraPath + path.delimiter + process.env.PATH; + const resultForWhich = shell.which(commandName); + const resultForWhichA = shell.which('-a', commandName); + t.falsy(shell.error()); + t.truthy(resultForWhich); + t.truthy(resultForWhichA); + t.truthy(resultForWhichA.length); + t.not(resultForWhich.toString(), matchingFile); + t.is(resultForWhichA.indexOf(matchingFile), -1); + + process.env.PATH = pathEnv; });