Skip to content

Commit

Permalink
path: fallback to process cwd when resolving drive cwd
Browse files Browse the repository at this point in the history
The `path.resolve()` function when given just a drive letter such as
"C:" tries to get a drive-specific CWD, but that isn't available in
cases when the process is not launched via cmd.exe and the process
CWD has not been explicitly set on that drive.

This change adds a fallback to the process CWD, if the process CWD
happens to be on the resolved drive letter. If the process CWD is on
another drive, then a drive-specific CWD cannot be resolved and
defaults to the drive's root as before.

Based on experimentation, the fixed behavior matches that of other
similar path resolution implementations on Windows I checked: .NET's
`System.IO.Path.GetFullPath()` and Python's `os.path.abspath()`.

In the automated path test cases the issue doesn't occur when the
tests are run normally from cmd.exe. But it did cause an assertion
when running the tests from PowerShell, that is fixed by this change.

PR-URL: #8541
Fixes: #7215
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
jasongin authored and jasnell committed Sep 29, 2016
1 parent eb972b0 commit 9dc9074
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 3 deletions.
8 changes: 5 additions & 3 deletions lib/path.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,12 @@ const win32 = {
} else {
// Windows has the concept of drive-specific current working
// directories. If we've resolved a drive letter but not yet an
// absolute path, get cwd for that drive. We're sure the device is not
// absolute path, get cwd for that drive, or the process cwd if
// the drive cwd is not available. We're sure the device is not
// a UNC path at this points, because UNC paths are always absolute.
path = process.env['=' + resolvedDevice];
// Verify that a drive-local cwd was found and that it actually points
path = process.env['=' + resolvedDevice] || process.cwd();

// Verify that a cwd was found and that it actually points
// to our drive. If not, default to the drive's root.
if (path === undefined ||
path.slice(0, 3).toLowerCase() !==
Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/path-resolve.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Tests resolving a path in the context of a spawned process.
// See https://github.com/nodejs/node/issues/7215
var path = require('path');
console.log(path.resolve(process.argv[2]));
12 changes: 12 additions & 0 deletions test/parallel/test-path.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const child = require('child_process');
const path = require('path');

const f = __filename;
Expand Down Expand Up @@ -444,6 +445,17 @@ resolveTests.forEach(function(test) {
});
assert.strictEqual(failures.length, 0, failures.join(''));

if (common.isWindows) {
// Test resolving the current Windows drive letter from a spawned process.
// See https://github.com/nodejs/node/issues/7215
const currentDriveLetter = path.parse(process.cwd()).root.substring(0, 2);
const resolveFixture = path.join(common.fixturesDir, 'path-resolve.js');
var spawnResult = child.spawnSync(
process.argv[0], [resolveFixture, currentDriveLetter]);
var resolvedPath = spawnResult.stdout.toString().trim();
assert.equal(resolvedPath.toLowerCase(), process.cwd().toLowerCase());
}


// path.isAbsolute tests
assert.strictEqual(path.win32.isAbsolute('/'), true);
Expand Down

0 comments on commit 9dc9074

Please sign in to comment.