From dfaa9c905530e30bd02d713c37546febb7446257 Mon Sep 17 00:00:00 2001 From: Brian White Date: Thu, 5 May 2016 01:27:06 -0400 Subject: [PATCH 1/3] path: fix basename() regressions This commit fixes a regression in basename() for the case when a supplied extension name completely matches the resulting basename. Fixes: https://github.com/nodejs/node/issues/6587 PR-URL: https://github.com/nodejs/node/pull/6590 Reviewed-By: James M Snell Reviewed-By: Evan Lucas --- lib/path.js | 12 ++++++++---- test/parallel/test-path.js | 17 ++++++++++++----- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/lib/path.js b/lib/path.js index 0fbc715ea8cc4f..5637b328c1da98 100644 --- a/lib/path.js +++ b/lib/path.js @@ -855,8 +855,10 @@ const win32 = { } } - if (end === -1) - return ''; + if (start === end) + end = firstNonSlashEnd; + else if (end === -1) + end = path.length; return path.slice(start, end); } else { for (i = path.length - 1; i >= start; --i) { @@ -1398,8 +1400,10 @@ const posix = { } } - if (end === -1) - return ''; + if (start === end) + end = firstNonSlashEnd; + else if (end === -1) + end = path.length; return path.slice(start, end); } else { for (i = path.length - 1; i >= 0; --i) { diff --git a/test/parallel/test-path.js b/test/parallel/test-path.js index b758b059a2d0a2..8b31046e66ffd0 100644 --- a/test/parallel/test-path.js +++ b/test/parallel/test-path.js @@ -15,6 +15,12 @@ assert.equal(path.basename('/basename.ext'), 'basename.ext'); assert.equal(path.basename('basename.ext'), 'basename.ext'); assert.equal(path.basename('basename.ext/'), 'basename.ext'); assert.equal(path.basename('basename.ext//'), 'basename.ext'); +assert.equal(path.basename('aaa/bbb', '/bbb'), 'bbb'); +assert.equal(path.basename('aaa/bbb', 'a/bbb'), 'bbb'); +assert.equal(path.basename('aaa/bbb', 'bbb'), 'bbb'); +assert.equal(path.basename('aaa/bbb//', 'bbb'), 'bbb'); +assert.equal(path.basename('aaa/bbb', 'bb'), 'b'); +assert.equal(path.basename('aaa/bbb', 'b'), 'bb'); // On Windows a backslash acts as a path separator. assert.equal(path.win32.basename('\\dir\\basename.ext'), 'basename.ext'); @@ -23,11 +29,12 @@ assert.equal(path.win32.basename('basename.ext'), 'basename.ext'); assert.equal(path.win32.basename('basename.ext\\'), 'basename.ext'); assert.equal(path.win32.basename('basename.ext\\\\'), 'basename.ext'); assert.equal(path.win32.basename('foo'), 'foo'); -assert.throws(path.win32.basename.bind(null, null), TypeError); -assert.throws(path.win32.basename.bind(null, true), TypeError); -assert.throws(path.win32.basename.bind(null, 1), TypeError); -assert.throws(path.win32.basename.bind(null), TypeError); -assert.throws(path.win32.basename.bind(null, {}), TypeError); +assert.equal(path.win32.basename('aaa\\bbb', '\\bbb'), 'bbb'); +assert.equal(path.win32.basename('aaa\\bbb', 'a\\bbb'), 'bbb'); +assert.equal(path.win32.basename('aaa\\bbb', 'bbb'), 'bbb'); +assert.equal(path.win32.basename('aaa\\bbb\\\\\\\\', 'bbb'), 'bbb'); +assert.equal(path.win32.basename('aaa\\bbb', 'bb'), 'b'); +assert.equal(path.win32.basename('aaa\\bbb', 'b'), 'bb'); // On unix a backslash is just treated as any other character. assert.equal(path.posix.basename('\\dir\\basename.ext'), '\\dir\\basename.ext'); From 7d404122d458dcbe886acc22c5a759b2e61b43fb Mon Sep 17 00:00:00 2001 From: Brian White Date: Thu, 5 May 2016 01:27:37 -0400 Subject: [PATCH 2/3] test: remove duplicate path tests The TypeError checks are already done later on in the test file for all path functions. PR-URL: https://github.com/nodejs/node/pull/6590 Reviewed-By: James M Snell Reviewed-By: Evan Lucas --- test/parallel/test-path.js | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/test/parallel/test-path.js b/test/parallel/test-path.js index 8b31046e66ffd0..503a1cbca68300 100644 --- a/test/parallel/test-path.js +++ b/test/parallel/test-path.js @@ -43,11 +43,6 @@ assert.equal(path.posix.basename('basename.ext'), 'basename.ext'); assert.equal(path.posix.basename('basename.ext\\'), 'basename.ext\\'); assert.equal(path.posix.basename('basename.ext\\\\'), 'basename.ext\\\\'); assert.equal(path.posix.basename('foo'), 'foo'); -assert.throws(path.posix.basename.bind(null, null), TypeError); -assert.throws(path.posix.basename.bind(null, true), TypeError); -assert.throws(path.posix.basename.bind(null, 1), TypeError); -assert.throws(path.posix.basename.bind(null), TypeError); -assert.throws(path.posix.basename.bind(null, {}), TypeError); // POSIX filenames may include control characters // c.f. http://www.dwheeler.com/essays/fixing-unix-linux-filenames.html @@ -67,11 +62,6 @@ assert.equal(path.posix.dirname(''), '.'); assert.equal(path.posix.dirname('/'), '/'); assert.equal(path.posix.dirname('////'), '/'); assert.equal(path.posix.dirname('foo'), '.'); -assert.throws(path.posix.dirname.bind(null, null), TypeError); -assert.throws(path.posix.dirname.bind(null, true), TypeError); -assert.throws(path.posix.dirname.bind(null, 1), TypeError); -assert.throws(path.posix.dirname.bind(null), TypeError); -assert.throws(path.posix.dirname.bind(null, {}), TypeError); assert.equal(path.win32.dirname('c:\\'), 'c:\\'); assert.equal(path.win32.dirname('c:\\foo'), 'c:\\'); @@ -107,11 +97,6 @@ assert.equal(path.win32.dirname(''), '.'); assert.equal(path.win32.dirname('/'), '/'); assert.equal(path.win32.dirname('////'), '/'); assert.equal(path.win32.dirname('foo'), '.'); -assert.throws(path.win32.dirname.bind(null, null), TypeError); -assert.throws(path.win32.dirname.bind(null, true), TypeError); -assert.throws(path.win32.dirname.bind(null, 1), TypeError); -assert.throws(path.win32.dirname.bind(null), TypeError); -assert.throws(path.win32.dirname.bind(null, {}), TypeError); // path.extname tests @@ -190,11 +175,6 @@ assert.equal(path.win32.extname('file\\'), ''); assert.equal(path.win32.extname('file\\\\'), ''); assert.equal(path.win32.extname('file.\\'), '.'); assert.equal(path.win32.extname('file.\\\\'), '.'); -assert.throws(path.win32.extname.bind(null, null), TypeError); -assert.throws(path.win32.extname.bind(null, true), TypeError); -assert.throws(path.win32.extname.bind(null, 1), TypeError); -assert.throws(path.win32.extname.bind(null), TypeError); -assert.throws(path.win32.extname.bind(null, {}), TypeError); // On *nix, backslash is a valid name component like any other character. assert.equal(path.posix.extname('.\\'), ''); @@ -205,11 +185,6 @@ assert.equal(path.posix.extname('file\\'), ''); assert.equal(path.posix.extname('file\\\\'), ''); assert.equal(path.posix.extname('file.\\'), '.\\'); assert.equal(path.posix.extname('file.\\\\'), '.\\\\'); -assert.throws(path.posix.extname.bind(null, null), TypeError); -assert.throws(path.posix.extname.bind(null, true), TypeError); -assert.throws(path.posix.extname.bind(null, 1), TypeError); -assert.throws(path.posix.extname.bind(null), TypeError); -assert.throws(path.posix.extname.bind(null, {}), TypeError); // path.join tests From 27549f64cea9b4822cc9e39e2d88b2481ca012a9 Mon Sep 17 00:00:00 2001 From: Brian White Date: Tue, 10 May 2016 10:26:50 -0400 Subject: [PATCH 3/3] test: add more path.basename() tests PR-URL: https://github.com/nodejs/node/pull/6590 Reviewed-By: James M Snell Reviewed-By: Evan Lucas --- test/parallel/test-path.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/parallel/test-path.js b/test/parallel/test-path.js index 503a1cbca68300..5aa60eb7c66b0d 100644 --- a/test/parallel/test-path.js +++ b/test/parallel/test-path.js @@ -21,6 +21,17 @@ assert.equal(path.basename('aaa/bbb', 'bbb'), 'bbb'); assert.equal(path.basename('aaa/bbb//', 'bbb'), 'bbb'); assert.equal(path.basename('aaa/bbb', 'bb'), 'b'); assert.equal(path.basename('aaa/bbb', 'b'), 'bb'); +assert.equal(path.basename('/aaa/bbb', '/bbb'), 'bbb'); +assert.equal(path.basename('/aaa/bbb', 'a/bbb'), 'bbb'); +assert.equal(path.basename('/aaa/bbb', 'bbb'), 'bbb'); +assert.equal(path.basename('/aaa/bbb//', 'bbb'), 'bbb'); +assert.equal(path.basename('/aaa/bbb', 'bb'), 'b'); +assert.equal(path.basename('/aaa/bbb', 'b'), 'bb'); +assert.equal(path.basename('/aaa/bbb'), 'bbb'); +assert.equal(path.basename('/aaa/'), 'aaa'); +assert.equal(path.basename('/aaa/b'), 'b'); +assert.equal(path.basename('/a/b'), 'b'); +assert.equal(path.basename('//a'), 'a'); // On Windows a backslash acts as a path separator. assert.equal(path.win32.basename('\\dir\\basename.ext'), 'basename.ext');