Skip to content

Commit

Permalink
path: fix posix.relative returns different results
Browse files Browse the repository at this point in the history 
Throw an error `ERR_UNSUPPORTED_PLATFOMR` when direct use
`path.posix.resolve` on Windows or direct use `path.win32.resolve`
on *nix.

Update docs, list win32 functions do not support direct use on
*nix and posix functions do not support direct use on Windows.

Update tests, only run current platform type test.

Fixes: nodejs#13683
Refs: nodejs#13714
  • Loading branch information
@DuanPengfei
DuanPengfei committed Jun 22, 2017
1 parent 2a46e57 commit 466faeb
Show file tree
Hide file tree
Showing 4 changed files with 163 additions and 90 deletions.
8 changes: 8 additions & 0 deletions doc/api/path.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,10 @@ added: v0.11.15
The `path.posix` property provides access to POSIX specific implementations
of the `path` methods.

The following functions do not support direct use on Windows.
* path.posix.resolve
* path.posix.relative

## path.relative(from, to)
<!-- YAML
added: v0.5.0
Expand Down Expand Up @@ -551,6 +555,10 @@ added: v0.11.15
The `path.win32` property provides access to Windows-specific implementations
of the `path` methods.

The following functions do not support direct use on *nix.
* path.win32.resolve
* path.win32.relative

[`TypeError`]: errors.html#errors_class_typeerror
[`path.parse()`]: #path_path_parse_path
[`path.posix`]: #path_path_posix
Expand Down
1 change: 1 addition & 0 deletions lib/internal/errors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ E('ERR_UNKNOWN_BUILTIN_MODULE', (id) => `No such built-in module: ${id}`);
E('ERR_UNKNOWN_SIGNAL', (signal) => `Unknown signal: ${signal}`);
E('ERR_UNKNOWN_STDIN_TYPE', 'Unknown stdin file type');
E('ERR_UNKNOWN_STREAM_TYPE', 'Unknown stream file type');
E('ERR_UNSUPPORTED_PLATFORM', 'Unsupported platform');
E('ERR_SOCKET_ALREADY_BOUND', 'Socket is already bound');
E('ERR_SOCKET_BAD_TYPE',
'Bad socket type specified. Valid types are: udp4, udp6');
Expand Down
14 changes: 14 additions & 0 deletions lib/path.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,18 @@ function assertPath(path) {
}
}

function assertWindowsPlatform(platform) {
// throw an error if direct use the function on *nix platform
if (process.platform !== 'win32')
throw new errors.Error('ERR_UNSUPPORTED_PLATFORM');
}

function assertPosixPlatform(platform) {
// throw an error if direct use the function on windows platform
if (process.process === 'win32')
throw new errors.Error('ERR_UNSUPPORTED_PLATFORM');
}

// Resolves . and .. elements in a path with directory names
function normalizeStringWin32(path, allowAboveRoot) {
var res = '';
Expand Down Expand Up @@ -205,6 +217,7 @@ const win32 = {
}

assertPath(path);
assertWindowsPlatform(process.platform);

// Skip empty entries
if (path.length === 0) {
Expand Down Expand Up @@ -1164,6 +1177,7 @@ const posix = {
}

assertPath(path);
assertPosixPlatform(process.platform);

// Skip empty entries
if (path.length === 0) {
Expand Down
230 changes: 140 additions & 90 deletions test/parallel/test-path.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,38 +418,58 @@ assert.strictEqual(path.posix.normalize('///..//./foo/.//bar'), '/foo/bar');


// path.resolve tests
const resolveTests = [
[ path.win32.resolve,
const resolveTest = {
win32: {
resolve: path.win32.resolve,
tests:
// arguments result
[[['c:/blah\\blah', 'd:/games', 'c:../a'], 'c:\\blah\\a'],
[['c:/ignore', 'd:\\a/b\\c/d', '\\e.exe'], 'd:\\e.exe'],
[['c:/ignore', 'c:/some/file'], 'c:\\some\\file'],
[['d:/ignore', 'd:some/dir//'], 'd:\\ignore\\some\\dir'],
[['.'], process.cwd()],
[['//server/share', '..', 'relative\\'], '\\\\server\\share\\relative'],
[['c:/', '//'], 'c:\\'],
[['c:/', '//dir'], 'c:\\dir'],
[['c:/', '//server/share'], '\\\\server\\share\\'],
[['c:/', '//server//share'], '\\\\server\\share\\'],
[['c:/', '///some//dir'], 'c:\\some\\dir'],
[['c:/ignore', 'd:\\a/b\\c/d', '\\e.exe'], 'd:\\e.exe'],
[['c:/ignore', 'c:/some/file'], 'c:\\some\\file'],
[['d:/ignore', 'd:some/dir//'], 'd:\\ignore\\some\\dir'],
[['.'], process.cwd()],
[['//server/share', '..', 'relative\\'], '\\\\server\\share\\relative'],
[['c:/', '//'], 'c:\\'],
[['c:/', '//dir'], 'c:\\dir'],
[['c:/', '//server/share'], '\\\\server\\share\\'],
[['c:/', '//server//share'], '\\\\server\\share\\'],
[['c:/', '///some//dir'], 'c:\\some\\dir'],
[['C:\\foo\\tmp.3\\', '..\\tmp.3\\cycles\\root.js'],
'C:\\foo\\tmp.3\\cycles\\root.js']
]
],
[ path.posix.resolve,
},
posix: {
resolve: path.posix.resolve,
tests:
// arguments result
[[['/var/lib', '../', 'file/'], '/var/file'],
[['/var/lib', '/../', 'file/'], '/file'],
[['a/b/c/', '../../..'], process.cwd()],
[['.'], process.cwd()],
[['/some/dir', '.', '/absolute/'], '/absolute'],
[['/foo/tmp.3/', '../tmp.3/cycles/root.js'], '/foo/tmp.3/cycles/root.js']
[['/var/lib', '/../', 'file/'], '/file'],
[['a/b/c/', '../../..'], process.cwd()],
[['.'], process.cwd()],
[['/some/dir', '.', '/absolute/'], '/absolute'],
[['/foo/tmp.3/', '../tmp.3/cycles/root.js'], '/foo/tmp.3/cycles/root.js']
]
]
];
resolveTests.forEach((test) => {
const resolve = test[0];
test[1].forEach((test) => {
}
};
{
let resolve;
let tests;
if (common.isWindows) {
resolve = resolveTest.win32.resolve;
tests = resolveTest.win32.tests;
// direct use path.posix.resolve on windows will throw error
assert.throws(() => {
path.posix.resolve('foo/bar');
}, common.expectsError({code: 'ERR_UNSUPPORTED_PLATFORM', type: Error}));
} else {
resolve = resolveTest.posix.resolve;
tests = resolveTest.posix.tests;
// direct use path.win32.resolve on *nix will throw error
assert.throws(() => {
path.win32.resolve('foo\\bar');
}, common.expectsError({code: 'ERR_UNSUPPORTED_PLATFORM', type: Error}));
}
tests.forEach((test) => {
const actual = resolve.apply(null, test[0]);
let actualAlt;
const os = resolve === path.win32.resolve ? 'win32' : 'posix';
Expand All @@ -465,7 +485,7 @@ resolveTests.forEach((test) => {
if (actual !== expected && actualAlt !== expected)
failures.push(`\n${message}`);
});
});
}
assert.strictEqual(failures.length, 0, failures.join(''));

if (common.isWindows) {
Expand Down Expand Up @@ -507,55 +527,78 @@ assert.strictEqual(path.posix.isAbsolute('./baz'), false);


// path.relative tests
const relativeTests = [
[ path.win32.relative,
const relativeTest = {
win32: {
relative: path.win32.relative,
tests:
// arguments result
[['c:/blah\\blah', 'd:/games', 'd:\\games'],
['c:/aaaa/bbbb', 'c:/aaaa', '..'],
['c:/aaaa/bbbb', 'c:/cccc', '..\\..\\cccc'],
['c:/aaaa/bbbb', 'c:/aaaa/bbbb', ''],
['c:/aaaa/bbbb', 'c:/aaaa/cccc', '..\\cccc'],
['c:/aaaa/', 'c:/aaaa/cccc', 'cccc'],
['c:/', 'c:\\aaaa\\bbbb', 'aaaa\\bbbb'],
['c:/aaaa/bbbb', 'd:\\', 'd:\\'],
['c:/AaAa/bbbb', 'c:/aaaa/bbbb', ''],
['c:/aaaaa/', 'c:/aaaa/cccc', '..\\aaaa\\cccc'],
['C:\\foo\\bar\\baz\\quux', 'C:\\', '..\\..\\..\\..'],
['C:\\foo\\test', 'C:\\foo\\test\\bar\\package.json', 'bar\\package.json'],
['C:\\foo\\bar\\baz-quux', 'C:\\foo\\bar\\baz', '..\\baz'],
['C:\\foo\\bar\\baz', 'C:\\foo\\bar\\baz-quux', '..\\baz-quux'],
['\\\\foo\\bar', '\\\\foo\\bar\\baz', 'baz'],
['\\\\foo\\bar\\baz', '\\\\foo\\bar', '..'],
['\\\\foo\\bar\\baz-quux', '\\\\foo\\bar\\baz', '..\\baz'],
['\\\\foo\\bar\\baz', '\\\\foo\\bar\\baz-quux', '..\\baz-quux'],
['C:\\baz-quux', 'C:\\baz', '..\\baz'],
['C:\\baz', 'C:\\baz-quux', '..\\baz-quux'],
['\\\\foo\\baz-quux', '\\\\foo\\baz', '..\\baz'],
['\\\\foo\\baz', '\\\\foo\\baz-quux', '..\\baz-quux'],
['C:\\baz', '\\\\foo\\bar\\baz', '\\\\foo\\bar\\baz'],
['\\\\foo\\bar\\baz', 'C:\\baz', 'C:\\baz']
['c:/aaaa/bbbb', 'c:/aaaa', '..'],
['c:/aaaa/bbbb', 'c:/cccc', '..\\..\\cccc'],
['c:/aaaa/bbbb', 'c:/aaaa/bbbb', ''],
['c:/aaaa/bbbb', 'c:/aaaa/cccc', '..\\cccc'],
['c:/aaaa/', 'c:/aaaa/cccc', 'cccc'],
['c:/', 'c:\\aaaa\\bbbb', 'aaaa\\bbbb'],
['c:/aaaa/bbbb', 'd:\\', 'd:\\'],
['c:/AaAa/bbbb', 'c:/aaaa/bbbb', ''],
['c:/aaaaa/', 'c:/aaaa/cccc', '..\\aaaa\\cccc'],
['C:\\foo\\bar\\baz\\quux', 'C:\\', '..\\..\\..\\..'],
[
'C:\\foo\\test', 'C:\\foo\\test\\bar\\package.json',
'bar\\package.json'
],
['C:\\foo\\bar\\baz-quux', 'C:\\foo\\bar\\baz', '..\\baz'],
['C:\\foo\\bar\\baz', 'C:\\foo\\bar\\baz-quux', '..\\baz-quux'],
['\\\\foo\\bar', '\\\\foo\\bar\\baz', 'baz'],
['\\\\foo\\bar\\baz', '\\\\foo\\bar', '..'],
['\\\\foo\\bar\\baz-quux', '\\\\foo\\bar\\baz', '..\\baz'],
['\\\\foo\\bar\\baz', '\\\\foo\\bar\\baz-quux', '..\\baz-quux'],
['C:\\baz-quux', 'C:\\baz', '..\\baz'],
['C:\\baz', 'C:\\baz-quux', '..\\baz-quux'],
['\\\\foo\\baz-quux', '\\\\foo\\baz', '..\\baz'],
['\\\\foo\\baz', '\\\\foo\\baz-quux', '..\\baz-quux'],
['C:\\baz', '\\\\foo\\bar\\baz', '\\\\foo\\bar\\baz'],
['\\\\foo\\bar\\baz', 'C:\\baz', 'C:\\baz']
]
],
[ path.posix.relative,
},
posix: {
relative: path.posix.relative,
tests:
// arguments result
[['/var/lib', '/var', '..'],
['/var/lib', '/bin', '../../bin'],
['/var/lib', '/var/lib', ''],
['/var/lib', '/var/apache', '../apache'],
['/var/', '/var/lib', 'lib'],
['/', '/var/lib', 'var/lib'],
['/foo/test', '/foo/test/bar/package.json', 'bar/package.json'],
['/Users/a/web/b/test/mails', '/Users/a/web/b', '../..'],
['/foo/bar/baz-quux', '/foo/bar/baz', '../baz'],
['/foo/bar/baz', '/foo/bar/baz-quux', '../baz-quux'],
['/baz-quux', '/baz', '../baz'],
['/baz', '/baz-quux', '../baz-quux']
['/var/lib', '/bin', '../../bin'],
['/var/lib', '/var/lib', ''],
['/var/lib', '/var/apache', '../apache'],
['/var/', '/var/lib', 'lib'],
['/', '/var/lib', 'var/lib'],
['/foo/test', '/foo/test/bar/package.json', 'bar/package.json'],
['/Users/a/web/b/test/mails', '/Users/a/web/b', '../..'],
['/foo/bar/baz-quux', '/foo/bar/baz', '../baz'],
['/foo/bar/baz', '/foo/bar/baz-quux', '../baz-quux'],
['/baz-quux', '/baz', '../baz'],
['/baz', '/baz-quux', '../baz-quux']
]
]
];
relativeTests.forEach((test) => {
const relative = test[0];
test[1].forEach((test) => {
}
};
{
let relative;
let tests;
if (common.isWindows) {
relative = relativeTest.win32.relative;
tests = relativeTest.win32.tests;
// direct use path.posix.relative on windows will throw error
assert.throws(() => {
path.posix.relative('foo/bar', 'foo');
}, common.expectsError({code: 'ERR_UNSUPPORTED_PLATFORM', type: Error}));
} else {
relative = relativeTest.posix.relative;
tests = relativeTest.posix.tests;
// direct use path.win32.relative on *nix will throw error
assert.throws(() => {
path.win32.relative('foo\\bar', 'foo');
}, common.expectsError({code: 'ERR_UNSUPPORTED_PLATFORM', type: Error}));
}
tests.forEach((test) => {
const actual = relative(test[0], test[1]);
const expected = test[2];
const os = relative === path.win32.relative ? 'win32' : 'posix';
Expand All @@ -565,7 +608,7 @@ relativeTests.forEach((test) => {
if (actual !== expected)
failures.push(`\n${message}`);
});
});
}
assert.strictEqual(failures.length, 0, failures.join(''));


Expand All @@ -584,16 +627,7 @@ assert.strictEqual(path.posix.delimiter, ':');

// path._makeLong tests
const emptyObj = {};
assert.strictEqual(path.posix._makeLong('/foo/bar'), '/foo/bar');
assert.strictEqual(path.posix._makeLong('foo/bar'), 'foo/bar');
assert.strictEqual(path.posix._makeLong(null), null);
assert.strictEqual(path.posix._makeLong(true), true);
assert.strictEqual(path.posix._makeLong(1), 1);
assert.strictEqual(path.posix._makeLong(), undefined);
assert.strictEqual(path.posix._makeLong(emptyObj), emptyObj);
if (common.isWindows) {
// These tests cause resolve() to insert the cwd, so we cannot test them from
// non-Windows platforms (easily)
assert.strictEqual(path.win32._makeLong('foo\\bar').toLowerCase(),
`\\\\?\\${process.cwd().toLowerCase()}\\foo\\bar`);
assert.strictEqual(path.win32._makeLong('foo/bar').toLowerCase(),
Expand All @@ -603,19 +637,35 @@ if (common.isWindows) {
`\\\\?\\${process.cwd().toLowerCase()}`);
assert.strictEqual(path.win32._makeLong('C').toLowerCase(),
`\\\\?\\${process.cwd().toLowerCase()}\\c`);
assert.strictEqual(path.win32._makeLong('C:\\foo'), '\\\\?\\C:\\foo');
assert.strictEqual(path.win32._makeLong('C:/foo'), '\\\\?\\C:\\foo');
assert.strictEqual(path.win32._makeLong('\\\\foo\\bar'),
'\\\\?\\UNC\\foo\\bar\\');
assert.strictEqual(path.win32._makeLong('//foo//bar'),
'\\\\?\\UNC\\foo\\bar\\');
assert.strictEqual(path.win32._makeLong('\\\\?\\foo'), '\\\\?\\foo');
assert.strictEqual(path.win32._makeLong(null), null);
assert.strictEqual(path.win32._makeLong(true), true);
assert.strictEqual(path.win32._makeLong(1), 1);
assert.strictEqual(path.win32._makeLong(), undefined);
assert.strictEqual(path.win32._makeLong(emptyObj), emptyObj);
// direct use path.posix._makeLong on windows will throw error
assert.throws(() => {
path.posix._makeLong('/foo/bar');
}, common.expectsError({code: 'ERR_UNSUPPORTED_PLATFORM', type: Error}));
} else {
assert.strictEqual(path.posix._makeLong('/foo/bar'), '/foo/bar');
assert.strictEqual(path.posix._makeLong('foo/bar'), 'foo/bar');
assert.strictEqual(path.posix._makeLong(null), null);
assert.strictEqual(path.posix._makeLong(true), true);
assert.strictEqual(path.posix._makeLong(1), 1);
assert.strictEqual(path.posix._makeLong(), undefined);
assert.strictEqual(path.posix._makeLong(emptyObj), emptyObj);
// direct use path.win32._makeLong on *nix will throw error
assert.throws(() => {
path.win32._makeLong('\\\\?\\foo');
}, common.expectsError({code: 'ERR_UNSUPPORTED_PLATFORM', type: Error}));
}
assert.strictEqual(path.win32._makeLong('C:\\foo'), '\\\\?\\C:\\foo');
assert.strictEqual(path.win32._makeLong('C:/foo'), '\\\\?\\C:\\foo');
assert.strictEqual(path.win32._makeLong('\\\\foo\\bar'),
'\\\\?\\UNC\\foo\\bar\\');
assert.strictEqual(path.win32._makeLong('//foo//bar'),
'\\\\?\\UNC\\foo\\bar\\');
assert.strictEqual(path.win32._makeLong('\\\\?\\foo'), '\\\\?\\foo');
assert.strictEqual(path.win32._makeLong(null), null);
assert.strictEqual(path.win32._makeLong(true), true);
assert.strictEqual(path.win32._makeLong(1), 1);
assert.strictEqual(path.win32._makeLong(), undefined);
assert.strictEqual(path.win32._makeLong(emptyObj), emptyObj);


if (common.isWindows)
Expand Down

0 comments on commit 466faeb

Please sign in to comment.