From c8d693d90a4d5b271d9e38dad566c7cf8f6261fe Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 20 Sep 2026 15:07:07 +0000 Subject: [PATCH] fix: walk PATH past Windows drive-letter entries Naive splitting on ':' treats the colon in D:\... as a delimiter, so whichSync never sees later PATH directories. Preserve drive prefixes and keep scanning when a drive entry throws. Fixes npm/node-which#56 Co-authored-by: David --- lib/index.js | 63 ++++++++++++++++++++++++++++++++++---- test/index.js | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 140 insertions(+), 6 deletions(-) diff --git a/lib/index.js b/lib/index.js index 2fd358b..b41d2c1 100644 --- a/lib/index.js +++ b/lib/index.js @@ -1,5 +1,5 @@ const { isexe, sync: isexeSync } = require('isexe') -const { join, delimiter, sep, posix } = require('path') +const { join, delimiter, sep, posix, win32 } = require('path') const isWindows = process.platform === 'win32' @@ -11,10 +11,45 @@ const isWindows = process.platform === 'win32' /* istanbul ignore next */ const rSlash = new RegExp(`[${posix.sep}${sep === posix.sep ? '' : sep}]`.replace(/(\\)/g, '\\$1')) const rRel = new RegExp(`^\\.${rSlash.source}`) +const rDrive = /^[A-Za-z]:/ const getNotFoundError = (cmd) => Object.assign(new Error(`not found: ${cmd}`), { code: 'ENOENT' }) +// Split a ':' PATH without treating the colon in "D:\..." / "C:/" / trailing +// "D:" as a delimiter. Naive String#split(':') truncates those entries and +// never walks later directories (https://github.com/npm/node-which/issues/56). +const splitColonPathEnv = (pathStr) => { + const parts = [] + let start = 0 + for (let i = 0; i < pathStr.length; i++) { + if (pathStr[i] !== ':') { + continue + } + const segment = pathStr.slice(start, i) + const next = pathStr[i + 1] + if (/^[A-Za-z]$/.test(segment) && (next === '\\' || next === '/' || next === undefined)) { + continue + } + parts.push(pathStr.slice(start, i)) + start = i + 1 + } + parts.push(pathStr.slice(start)) + return parts +} + +const splitPathEnv = (pathStr, delim) => { + // Windows PATH is ';'-separated. If a colon split was requested but the + // string is clearly a Windows PATH, prefer ';' so "D:\..." stays intact. + if (delim !== ';' && /;[A-Za-z]:[\\/]/.test(pathStr)) { + return pathStr.split(';') + } + if (delim === ':') { + return splitColonPathEnv(pathStr) + } + return pathStr.split(delim) +} + const getPathInfo = (cmd, { path: optPath = process.env.PATH, pathExt: optPathExt = process.env.PATHEXT, @@ -25,7 +60,7 @@ const getPathInfo = (cmd, { const pathEnv = cmd.match(rSlash) ? [''] : [ // windows always checks the cwd first ...(isWindows ? [process.cwd()] : []), - ...(optPath || /* istanbul ignore next: very unusual */ '').split(optDelimiter), + ...splitPathEnv(optPath || /* istanbul ignore next: very unusual */ '', optDelimiter), ] if (isWindows) { @@ -44,7 +79,25 @@ const getPathInfo = (cmd, { const getPathPart = (raw, cmd) => { const pathPart = /^".*"$/.test(raw) ? raw.slice(1, -1) : raw const prefix = !pathPart && rRel.test(cmd) ? cmd.slice(0, 2) : '' - return prefix + join(pathPart, cmd) + const joined = rDrive.test(pathPart) ? win32.join(pathPart, cmd) : join(pathPart, cmd) + return prefix + joined +} + +const checkExe = async (file, pathExtExe) => { + try { + return await isexe(file, { pathExt: pathExtExe, ignoreErrors: true }) + } catch { + // A not-ready drive (D:\) or similar must not abort the rest of PATH. + return false + } +} + +const checkExeSync = (file, pathExtExe) => { + try { + return isexeSync(file, { pathExt: pathExtExe, ignoreErrors: true }) + } catch { + return false + } } const which = async (cmd, opt = {}) => { @@ -56,7 +109,7 @@ const which = async (cmd, opt = {}) => { for (const ext of pathExt) { const withExt = p + ext - const is = await isexe(withExt, { pathExt: pathExtExe, ignoreErrors: true }) + const is = await checkExe(withExt, pathExtExe) if (is) { if (!opt.all) { return withExt @@ -86,7 +139,7 @@ const whichSync = (cmd, opt = {}) => { for (const ext of pathExt) { const withExt = p + ext - const is = isexeSync(withExt, { pathExt: pathExtExe, ignoreErrors: true }) + const is = checkExeSync(withExt, pathExtExe) if (is) { if (!opt.all) { return withExt diff --git a/test/index.js b/test/index.js index c77fed5..fc56ac9 100644 --- a/test/index.js +++ b/test/index.js @@ -1,7 +1,7 @@ const t = require('tap') const fs = require('fs') -const { basename, join, relative, sep, delimiter } = require('path') +const { basename, join, relative, sep, delimiter, win32 } = require('path') const isWindows = process.platform === 'win32' const ENV_VARS = { PATH: process.env.PATH, PATHEXT: process.env.PATHEXT } @@ -173,3 +173,84 @@ t.test('pathExt', async (t) => { }) }) }) + +// https://github.com/npm/node-which/issues/56 +t.test('walks PATH past Windows drive-letter entries', async t => { + t.test('finds executable after a D:\\\\ entry', async t => { + const fixture = t.testdir({ 'foo.sh': 'echo foo\n' }) + const foo = join(fixture, 'foo.sh') + fs.chmodSync(foo, '0755') + await runTest(t, basename(foo), foo, { + path: ['D:\\definitely-not-here', fixture].join(';'), + delimiter: ';', + pathExt: '.sh', + }) + }) + + t.test('does not split D:\\\\ or C:\\\\ colons when delimiter is :', async t => { + const cmd = 'git' + const laterDir = 'C:\\Program Files\\Git\\cmd' + const later = win32.join(laterDir, cmd) + '.EXE' + // Include D:/, D:\, and a trailing D: so every drive-colon form is walked. + const pathEnv = `D:/offline:D:\\Tools\\Git\\cmd:${laterDir}:D:` + + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { ...platform, value: 'win32' }) + t.teardown(() => Object.defineProperty(process, 'platform', platform)) + + const isexe = async (p) => p === later + isexe.sync = (p) => p === later + const which = t.mock('..', { isexe: { isexe, sync: isexe.sync } }) + const opt = { path: pathEnv, delimiter: ':', pathExt: '.EXE' } + + t.equal(await which(cmd, opt), later, 'async') + t.equal(which.sync(cmd, opt), later, 'sync') + }) + + t.test('splits a Windows PATH on ; even if delimiter is :', async t => { + const cmd = 'git' + const laterDir = 'C:\\Program Files\\Git\\cmd' + const later = win32.join(laterDir, cmd) + '.EXE' + const pathEnv = `D:\\NoGit;${laterDir}` + + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { ...platform, value: 'win32' }) + t.teardown(() => Object.defineProperty(process, 'platform', platform)) + + const isexe = async (p) => p === later + isexe.sync = (p) => p === later + const which = t.mock('..', { isexe: { isexe, sync: isexe.sync } }) + const opt = { path: pathEnv, delimiter: ':', pathExt: '.EXE' } + + t.equal(await which(cmd, opt), later, 'async') + t.equal(which.sync(cmd, opt), later, 'sync') + }) + + t.test('continues after isexe throws on a drive entry', async t => { + const fixture = t.testdir({ 'foo.sh': 'echo foo\n' }) + const foo = join(fixture, 'foo.sh') + fs.chmodSync(foo, '0755') + + const { isexe, sync } = require('isexe') + const throwThen = (fn) => { + let thrown = false + return (p, opt) => { + if (!thrown) { + thrown = true + throw Object.assign(new Error('not ready'), { code: 'UNKNOWN' }) + } + return fn(p, opt) + } + } + + const which = t.mock('..', { + isexe: { isexe: throwThen(isexe), sync: throwThen(sync) }, + }) + const opt = { + path: `D:\\offline\\bin${delimiter}${fixture}`, + pathExt: '.sh', + } + t.equal(await which(basename(foo), opt), foo, 'async') + t.equal(which.sync(basename(foo), opt), foo, 'sync') + }) +})