fix: walk PATH past Windows drive-letter entries

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

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')
+  })
+})