From 06ee3c696dc258c010e0f88a924011fddbd45f60 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Cruz?= <andremiguelcruz@msn.com>
Date: Wed, 31 Jan 2018 04:06:43 +0000
Subject: [PATCH] fix: fix paths being incorrectly normalized on unix

This actually fixes another bug with `options.cwd` on Windows.

Closes #90.
---
 lib/parse.js               |  6 +++-
 lib/util/resolveCommand.js | 35 ++++++++++++++++++----
 test/index.test.js         | 61 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 95 insertions(+), 7 deletions(-)

diff --git a/lib/parse.js b/lib/parse.js
index a8f0bf0..6f9c76d 100644
--- a/lib/parse.js
+++ b/lib/parse.js
@@ -49,6 +49,10 @@ function parseNonShell(parsed) {
         // we need to double escape them
         const needsDoubleEscapeMetaChars = isCmdShimRegExp.test(commandFile);
 
+        // Normalize posix paths into OS compatible paths (e.g.: foo/bar -> foo\bar)
+        // This is necessary otherwise it will always fail with ENOENT in those cases
+        parsed.command = path.normalize(parsed.command);
+
         // Escape command & arguments
         parsed.command = escape.command(parsed.command);
         parsed.args = parsed.args.map((arg) => escape.argument(arg, needsDoubleEscapeMetaChars));
@@ -104,7 +108,7 @@ function parse(command, args, options) {
 
     // Build our parsed object
     const parsed = {
-        command: path.normalize(command),
+        command,
         args,
         options,
         file: undefined,
diff --git a/lib/util/resolveCommand.js b/lib/util/resolveCommand.js
index 0d32711..2fd5ad2 100644
--- a/lib/util/resolveCommand.js
+++ b/lib/util/resolveCommand.js
@@ -4,15 +4,40 @@ const path = require('path');
 const which = require('which');
 const pathKey = require('path-key')();
 
-function resolveCommandAttempt(parsed, withPathExt) {
-    withPathExt = !!withPathExt;
+function resolveCommandAttempt(parsed, withoutPathExt) {
+    const cwd = process.cwd();
+    const hasCustomCwd = parsed.options.cwd != null;
+
+    // If a custom `cwd` was specified, we need to change the process cwd
+    // because `which` will do stat calls but does not support a custom cwd
+    if (hasCustomCwd) {
+        try {
+            process.chdir(parsed.options.cwd);
+        } catch (err) {
+            /* Empty */
+        }
+    }
+
+    let resolved;
 
     try {
-        return which.sync(parsed.command, {
+        resolved = which.sync(parsed.command, {
             path: (parsed.options.env || process.env)[pathKey],
-            pathExt: withPathExt && process.env.PATHEXT ? path.delimiter + process.env.PATHEXT : undefined,
+            pathExt: withoutPathExt ? path.delimiter : undefined,
         });
-    } catch (e) { /* Empty */ }
+    } catch (e) {
+        /* Empty */
+    } finally {
+        process.chdir(cwd);
+    }
+
+    // If we successfully resolved, ensure that an absolute path is returned
+    // Note that when a custom `cwd` was used, we need to resolve to an absolute path based on it
+    if (resolved) {
+        resolved = path.resolve(hasCustomCwd ? parsed.options.cwd : '', resolved);
+    }
+
+    return resolved;
 }
 
 function resolveCommand(parsed) {
diff --git a/test/index.test.js b/test/index.test.js
index fe5e020..2217d9c 100644
--- a/test/index.test.js
+++ b/test/index.test.js
@@ -32,7 +32,7 @@ run.methods.forEach((method) => {
             expect(stdout.trim()).toBe('foo');
         });
 
-        it('should support shebang in executables with /usr/bin/env', async () => {
+        it('should support shebang in executables with `/usr/bin/env`', async () => {
             const { stdout: stdout1 } = await run(method, `${__dirname}/fixtures/shebang`);
 
             expect(stdout1).toBe('shebang works!');
@@ -219,6 +219,26 @@ run.methods.forEach((method) => {
             expect(stdout3.trim()).toBe('foo');
         });
 
+        it('should work with a relative posix path to a command with a custom `cwd`', async () => {
+            const relativeTestPath = path.relative(process.cwd(), __dirname).replace(/\\/, '/');
+
+            const { stdout: stdout1 } = await run(method, 'fixtures/say-foo', { cwd: relativeTestPath });
+
+            expect(stdout1.trim()).toBe('foo');
+
+            const { stdout: stdout2 } = await run(method, './fixtures/say-foo', { cwd: `./${relativeTestPath}` });
+
+            expect(stdout2.trim()).toBe('foo');
+
+            if (!isWin) {
+                return;
+            }
+
+            const { stdout: stdout3 } = await run(method, './fixtures/say-foo.bat', { cwd: `./${relativeTestPath}` });
+
+            expect(stdout3.trim()).toBe('foo');
+        });
+
         {
             const assertError = (err) => {
                 const syscall = run.isMethodSync(method) ? 'spawnSync' : 'spawn';
@@ -354,6 +374,45 @@ run.methods.forEach((method) => {
             });
         }
 
+        if (run.isMethodSync(method)) {
+            it('should fail with ENOENT a non-existing `cwd` was specified', () => {
+                expect.assertions(1);
+
+                try {
+                    run(method, 'fixtures/say-foo', { cwd: 'somedirthatwillneverexist' });
+                } catch (err) {
+                    expect(err.code).toBe('ENOENT');
+                }
+            });
+        } else {
+            it('should emit `error` and `close` if a non-existing `cwd` was specified', async () => {
+                expect.assertions(3);
+
+                await new Promise((resolve, reject) => {
+                    const promise = run(method, 'somecommandthatwillneverexist', ['foo']);
+                    const { cp } = promise;
+
+                    promise.catch(() => {});
+
+                    let timeout;
+
+                    cp
+                    .on('error', (err) => expect(err.code).toBe('ENOENT'))
+                    .on('exit', () => {
+                        cp.removeAllListeners();
+                        clearTimeout(timeout);
+                        reject(new Error('Should not emit exit'));
+                    })
+                    .on('close', (code, signal) => {
+                        expect(code).not.toBe(0);
+                        expect(signal).toBe(null);
+
+                        timeout = setTimeout(resolve, 1000);
+                    });
+                });
+            });
+        }
+
         if (isWin) {
             it('should use nodejs\' spawn when options.shell is specified (windows)', async () => {
                 const { stdout } = await run(method, 'echo', ['%RANDOM%'], { shell: true });