From b41ae9847e15bf831ce5b67551a2340614acfce6 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Mon, 24 Jul 2017 08:39:02 +0800 Subject: [PATCH] path: fix win32 volume-relative paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `path.resolve()` and `path.join()` are left alone in this commit due to the lack of clear semantics. PR-URL: https://github.com/nodejs/node/pull/14440 Fixes: https://github.com/nodejs/node/issues/14405 Reviewed-By: Refael Ackermann Reviewed-By: Tobias Nießen --- lib/path.js | 45 ++++++++------ test/parallel/test-path-parse-format.js | 82 ++++++++++++++----------- test/parallel/test-path.js | 28 ++++++++- 3 files changed, 97 insertions(+), 58 deletions(-) diff --git a/lib/path.js b/lib/path.js index 4dc4b8398464d3..3fab7f79d746ec 100644 --- a/lib/path.js +++ b/lib/path.js @@ -908,6 +908,7 @@ const win32 = { extname: function extname(path) { assertPath(path); + var start = 0; var startDot = -1; var startPart = 0; var end = -1; @@ -915,7 +916,20 @@ const win32 = { // Track the state of characters (if any) we see before our first dot and // after any path separator we find var preDotState = 0; - for (var i = path.length - 1; i >= 0; --i) { + + // Check for a drive letter prefix so as not to mistake the following + // path separator as an extra separator at the end of the path that can be + // disregarded + if (path.length >= 2) { + const code = path.charCodeAt(0); + if (path.charCodeAt(1) === 58/*:*/ && + ((code >= 65/*A*/ && code <= 90/*Z*/) || + (code >= 97/*a*/ && code <= 122/*z*/))) { + start = startPart = 2; + } + } + + for (var i = path.length - 1; i >= start; --i) { const code = path.charCodeAt(i); if (code === 47/*/*/ || code === 92/*\*/) { // If we reached a path separator that was not part of a set of path @@ -979,15 +993,12 @@ const win32 = { var len = path.length; var rootEnd = 0; var code = path.charCodeAt(0); - var isAbsolute = false; // Try to match a root if (len > 1) { if (code === 47/*/*/ || code === 92/*\*/) { // Possible UNC root - isAbsolute = true; - code = path.charCodeAt(1); rootEnd = 1; if (code === 47/*/*/ || code === 92/*\*/) { @@ -1046,7 +1057,6 @@ const win32 = { ret.root = ret.dir = path; return ret; } - isAbsolute = true; rootEnd = 3; } } else { @@ -1068,7 +1078,7 @@ const win32 = { ret.root = path.slice(0, rootEnd); var startDot = -1; - var startPart = 0; + var startPart = rootEnd; var end = -1; var matchedSlash = true; var i = path.length - 1; @@ -1117,26 +1127,21 @@ const win32 = { startDot === end - 1 && startDot === startPart + 1)) { if (end !== -1) { - if (startPart === 0 && isAbsolute) - ret.base = ret.name = path.slice(rootEnd, end); - else - ret.base = ret.name = path.slice(startPart, end); + ret.base = ret.name = path.slice(startPart, end); } } else { - if (startPart === 0 && isAbsolute) { - ret.name = path.slice(rootEnd, startDot); - ret.base = path.slice(rootEnd, end); - } else { - ret.name = path.slice(startPart, startDot); - ret.base = path.slice(startPart, end); - } + ret.name = path.slice(startPart, startDot); + ret.base = path.slice(startPart, end); ret.ext = path.slice(startDot, end); } - if (startPart > 0) + // If the directory is the root, use the entire root as the `dir` including + // the trailing slash if any (`C:\abc` -> `C:\`). Otherwise, strip out the + // trailing slash (`C:\abc\def` -> `C:\abc`). + if (startPart > 0 && startPart !== rootEnd) ret.dir = path.slice(0, startPart - 1); - else if (isAbsolute) - ret.dir = path.slice(0, rootEnd); + else + ret.dir = ret.root; return ret; }, diff --git a/test/parallel/test-path-parse-format.js b/test/parallel/test-path-parse-format.js index c75c1a9135cd79..64e9429a2394f0 100644 --- a/test/parallel/test-path-parse-format.js +++ b/test/parallel/test-path-parse-format.js @@ -25,28 +25,33 @@ const assert = require('assert'); const path = require('path'); const winPaths = [ - 'C:\\path\\dir\\index.html', - 'C:\\another_path\\DIR\\1\\2\\33\\\\index', - 'another_path\\DIR with spaces\\1\\2\\33\\index', - '\\foo\\C:', - 'file', - '.\\file', - 'C:\\', - 'C:', - '\\', - '', + // [path, root] + ['C:\\path\\dir\\index.html', 'C:\\'], + ['C:\\another_path\\DIR\\1\\2\\33\\\\index', 'C:\\'], + ['another_path\\DIR with spaces\\1\\2\\33\\index', ''], + ['\\', '\\'], + ['\\foo\\C:', '\\'], + ['file', ''], + ['file:stream', ''], + ['.\\file', ''], + ['C:', 'C:'], + ['C:.', 'C:'], + ['C:..', 'C:'], + ['C:abc', 'C:'], + ['C:\\', 'C:\\'], + ['C:\\abc', 'C:\\' ], + ['', ''], // unc - '\\\\server\\share\\file_path', - '\\\\server two\\shared folder\\file path.zip', - '\\\\teela\\admin$\\system32', - '\\\\?\\UNC\\server\\share' + ['\\\\server\\share\\file_path', '\\\\server\\share\\'], + ['\\\\server two\\shared folder\\file path.zip', + '\\\\server two\\shared folder\\'], + ['\\\\teela\\admin$\\system32', '\\\\teela\\admin$\\'], + ['\\\\?\\UNC\\server\\share', '\\\\?\\UNC\\'] ]; const winSpecialCaseParseTests = [ ['/foo/bar', { root: '/' }], - ['C:', { root: 'C:', dir: 'C:', base: '' }], - ['C:\\', { root: 'C:\\', dir: 'C:\\', base: '' }] ]; const winSpecialCaseFormatTests = [ @@ -60,26 +65,27 @@ const winSpecialCaseFormatTests = [ ]; const unixPaths = [ - '/home/user/dir/file.txt', - '/home/user/a dir/another File.zip', - '/home/user/a dir//another&File.', - '/home/user/a$$$dir//another File.zip', - 'user/dir/another File.zip', - 'file', - '.\\file', - './file', - 'C:\\foo', - '/', - '', - '.', - '..', - '/foo', - '/foo.', - '/foo.bar', - '/.', - '/.foo', - '/.foo.bar', - '/foo/bar.baz', + // [path, root] + ['/home/user/dir/file.txt', '/'], + ['/home/user/a dir/another File.zip', '/'], + ['/home/user/a dir//another&File.', '/'], + ['/home/user/a$$$dir//another File.zip', '/'], + ['user/dir/another File.zip', ''], + ['file', ''], + ['.\\file', ''], + ['./file', ''], + ['C:\\foo', ''], + ['/', '/'], + ['', ''], + ['.', ''], + ['..', ''], + ['/foo', '/'], + ['/foo.', '/'], + ['/foo.bar', '/'], + ['/.', '/'], + ['/.foo', '/'], + ['/.foo.bar', '/'], + ['/foo/bar.baz', '/'] ]; const unixSpecialCaseFormatTests = [ @@ -190,7 +196,7 @@ function checkErrors(path) { } function checkParseFormat(path, paths) { - paths.forEach(function(element) { + paths.forEach(function([element, root]) { const output = path.parse(element); assert.strictEqual(typeof output.root, 'string'); assert.strictEqual(typeof output.dir, 'string'); @@ -198,6 +204,8 @@ function checkParseFormat(path, paths) { assert.strictEqual(typeof output.ext, 'string'); assert.strictEqual(typeof output.name, 'string'); assert.strictEqual(path.format(output), element); + assert.strictEqual(output.root, root); + assert(output.dir.startsWith(output.root)); assert.strictEqual(output.dir, output.dir ? path.dirname(element) : ''); assert.strictEqual(output.base, path.basename(element)); assert.strictEqual(output.ext, path.extname(element)); diff --git a/test/parallel/test-path.js b/test/parallel/test-path.js index ea5de95fb86620..3e4f2b43e4c21e 100644 --- a/test/parallel/test-path.js +++ b/test/parallel/test-path.js @@ -72,6 +72,16 @@ assert.strictEqual(path.win32.basename('aaa\\bbb', 'bbb'), 'bbb'); assert.strictEqual(path.win32.basename('aaa\\bbb\\\\\\\\', 'bbb'), 'bbb'); assert.strictEqual(path.win32.basename('aaa\\bbb', 'bb'), 'b'); assert.strictEqual(path.win32.basename('aaa\\bbb', 'b'), 'bb'); +assert.strictEqual(path.win32.basename('C:'), ''); +assert.strictEqual(path.win32.basename('C:.'), '.'); +assert.strictEqual(path.win32.basename('C:\\'), ''); +assert.strictEqual(path.win32.basename('C:\\dir\\base.ext'), 'base.ext'); +assert.strictEqual(path.win32.basename('C:\\basename.ext'), 'basename.ext'); +assert.strictEqual(path.win32.basename('C:basename.ext'), 'basename.ext'); +assert.strictEqual(path.win32.basename('C:basename.ext\\'), 'basename.ext'); +assert.strictEqual(path.win32.basename('C:basename.ext\\\\'), 'basename.ext'); +assert.strictEqual(path.win32.basename('C:foo'), 'foo'); +assert.strictEqual(path.win32.basename('file:stream'), 'file:stream'); // On unix a backslash is just treated as any other character. assert.strictEqual(path.posix.basename('\\dir\\basename.ext'), @@ -120,6 +130,8 @@ assert.strictEqual(path.win32.dirname('c:foo\\'), 'c:'); assert.strictEqual(path.win32.dirname('c:foo\\bar'), 'c:foo'); assert.strictEqual(path.win32.dirname('c:foo\\bar\\'), 'c:foo'); assert.strictEqual(path.win32.dirname('c:foo\\bar\\baz'), 'c:foo\\bar'); +assert.strictEqual(path.win32.dirname('file:stream'), '.'); +assert.strictEqual(path.win32.dirname('dir\\file:stream'), 'dir'); assert.strictEqual(path.win32.dirname('\\\\unc\\share'), '\\\\unc\\share'); assert.strictEqual(path.win32.dirname('\\\\unc\\share\\foo'), @@ -187,6 +199,7 @@ assert.strictEqual(path.win32.dirname('foo'), '.'); ['file./', '.'], ['file.//', '.'], ].forEach((test) => { + const expected = test[1]; [path.posix.extname, path.win32.extname].forEach((extname) => { let input = test[0]; let os; @@ -197,12 +210,19 @@ assert.strictEqual(path.win32.dirname('foo'), '.'); os = 'posix'; } const actual = extname(input); - const expected = test[1]; const message = `path.${os}.extname(${JSON.stringify(input)})\n expect=${ JSON.stringify(expected)}\n actual=${JSON.stringify(actual)}`; if (actual !== expected) failures.push(`\n${message}`); }); + { + const input = `C:${test[0].replace(slashRE, '\\')}`; + const actual = path.win32.extname(input); + const message = `path.win32.extname(${JSON.stringify(input)})\n expect=${ + JSON.stringify(expected)}\n actual=${JSON.stringify(actual)}`; + if (actual !== expected) + failures.push(`\n${message}`); + } }); assert.strictEqual(failures.length, 0, failures.join('')); @@ -406,6 +426,12 @@ assert.strictEqual(path.win32.normalize('a//b//.'), 'a\\b'); assert.strictEqual(path.win32.normalize('//server/share/dir/file.ext'), '\\\\server\\share\\dir\\file.ext'); assert.strictEqual(path.win32.normalize('/a/b/c/../../../x/y/z'), '\\x\\y\\z'); +assert.strictEqual(path.win32.normalize('C:'), 'C:.'); +assert.strictEqual(path.win32.normalize('C:..\\abc'), 'C:..\\abc'); +assert.strictEqual(path.win32.normalize('C:..\\..\\abc\\..\\def'), + 'C:..\\..\\def'); +assert.strictEqual(path.win32.normalize('C:\\.'), 'C:\\'); +assert.strictEqual(path.win32.normalize('file:stream'), 'file:stream'); assert.strictEqual(path.posix.normalize('./fixtures///b/../b/c.js'), 'fixtures/b/c.js');