Skip to content

Commit

Permalink
path: fix input type checking regression
Browse files Browse the repository at this point in the history 
Before b212be0, input types were not checked in some path functions
and the inputs were passed directly to `regexp.exec()` which
implicitly converts its argument to a string.

This commit both removes the type checking added in b212be0 and
adds string coercion for those functions.

PR-URL: #5244
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
  • Loading branch information
@mscdex @rvagg
mscdex authored and rvagg committed Feb 18, 2016
1 parent 7684b0f commit 1c6f927
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 7 deletions.
20 changes: 13 additions & 7 deletions lib/path.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,8 @@ const win32 = {


dirname: function dirname(path) {
assertPath(path);
if (typeof path !== 'string')
path = '' + path;
const len = path.length;
if (len === 0)
return '.';
Expand Down Expand Up @@ -791,7 +792,7 @@ const win32 = {

if (end === -1) {
if (rootEnd === -1)
end = len;
return '.';
else
end = rootEnd;
}
Expand All @@ -800,9 +801,10 @@ const win32 = {


basename: function basename(path, ext) {
assertPath(path);
if (ext !== undefined && typeof ext !== 'string')
throw new TypeError('ext must be a string');
if (typeof path !== 'string')
path = '' + path;
var start = 0;
var end = -1;
var matchedSlash = true;
Expand Down Expand Up @@ -888,7 +890,8 @@ const win32 = {


extname: function extname(path) {
assertPath(path);
if (typeof path !== 'string')
path = '' + path;
var startDot = -1;
var startPart = 0;
var end = -1;
Expand Down Expand Up @@ -1320,7 +1323,8 @@ const posix = {


dirname: function dirname(path) {
assertPath(path);
if (typeof path !== 'string')
path = '' + path;
if (path.length === 0)
return '.';
var code = path.charCodeAt(0);
Expand Down Expand Up @@ -1349,9 +1353,10 @@ const posix = {


basename: function basename(path, ext) {
assertPath(path);
if (ext !== undefined && typeof ext !== 'string')
throw new TypeError('ext must be a string');
if (typeof path !== 'string')
path = '' + path;

var start = 0;
var end = -1;
Expand Down Expand Up @@ -1425,7 +1430,8 @@ const posix = {


extname: function extname(path) {
assertPath(path);
if (typeof path !== 'string')
path = '' + path;
var startDot = -1;
var startPart = 0;
var end = -1;
Expand Down
45 changes: 45 additions & 0 deletions test/parallel/test-path.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,25 @@ 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('basename.ext\\\\'), 'basename.ext');
assert.equal(path.win32.basename('foo'), 'foo');
assert.equal(path.win32.basename(null), 'null');
assert.equal(path.win32.basename(true), 'true');
assert.equal(path.win32.basename(1), '1');
assert.equal(path.win32.basename(), 'undefined');
assert.equal(path.win32.basename({}), '[object Object]');

// On unix a backslash is just treated as any other character.
assert.equal(path.posix.basename('\\dir\\basename.ext'), '\\dir\\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('basename.ext\\'), 'basename.ext\\');
assert.equal(path.posix.basename('basename.ext\\\\'), 'basename.ext\\\\');
assert.equal(path.posix.basename('foo'), 'foo');
assert.equal(path.posix.basename(null), 'null');
assert.equal(path.posix.basename(true), 'true');
assert.equal(path.posix.basename(1), '1');
assert.equal(path.posix.basename(), 'undefined');
assert.equal(path.posix.basename({}), '[object Object]');

// POSIX filenames may include control characters
// c.f. http://www.dwheeler.com/essays/fixing-unix-linux-filenames.html
Expand All @@ -47,6 +59,12 @@ assert.equal(path.posix.dirname('/a'), '/');
assert.equal(path.posix.dirname(''), '.');
assert.equal(path.posix.dirname('/'), '/');
assert.equal(path.posix.dirname('////'), '/');
assert.equal(path.posix.dirname('foo'), '.');
assert.equal(path.posix.dirname(null), '.');
assert.equal(path.posix.dirname(true), '.');
assert.equal(path.posix.dirname(1), '.');
assert.equal(path.posix.dirname(), '.');
assert.equal(path.posix.dirname({}), '.');

assert.equal(path.win32.dirname('c:\\'), 'c:\\');
assert.equal(path.win32.dirname('c:\\foo'), 'c:\\');
Expand Down Expand Up @@ -81,6 +99,12 @@ assert.equal(path.win32.dirname('/a'), '/');
assert.equal(path.win32.dirname(''), '.');
assert.equal(path.win32.dirname('/'), '/');
assert.equal(path.win32.dirname('////'), '/');
assert.equal(path.win32.dirname('foo'), '.');
assert.equal(path.win32.dirname(null), '.');
assert.equal(path.win32.dirname(true), '.');
assert.equal(path.win32.dirname(1), '.');
assert.equal(path.win32.dirname(), '.');
assert.equal(path.win32.dirname({}), '.');


// path.extname tests
Expand Down Expand Up @@ -156,6 +180,11 @@ 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.equal(path.win32.extname(null), '');
assert.equal(path.win32.extname(true), '');
assert.equal(path.win32.extname(1), '');
assert.equal(path.win32.extname(), '');
assert.equal(path.win32.extname({}), '');

// On *nix, backslash is a valid name component like any other character.
assert.equal(path.posix.extname('.\\'), '');
Expand All @@ -166,6 +195,11 @@ 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.equal(path.posix.extname(null), '');
assert.equal(path.posix.extname(true), '');
assert.equal(path.posix.extname(1), '');
assert.equal(path.posix.extname(), '');
assert.equal(path.posix.extname({}), '');


// path.join tests
Expand Down Expand Up @@ -486,8 +520,14 @@ assert.equal(path.posix.delimiter, ':');


// path._makeLong tests
const emptyObj = {};
assert.equal(path.posix._makeLong('/foo/bar'), '/foo/bar');
assert.equal(path.posix._makeLong('foo/bar'), 'foo/bar');
assert.equal(path.posix._makeLong(null), null);
assert.equal(path.posix._makeLong(true), true);
assert.equal(path.posix._makeLong(1), 1);
assert.equal(path.posix._makeLong(), undefined);
assert.equal(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)
Expand All @@ -505,6 +545,11 @@ assert.equal(path.win32._makeLong('C:/foo'), '\\\\?\\C:\\foo');
assert.equal(path.win32._makeLong('\\\\foo\\bar'), '\\\\?\\UNC\\foo\\bar\\');
assert.equal(path.win32._makeLong('//foo//bar'), '\\\\?\\UNC\\foo\\bar\\');
assert.equal(path.win32._makeLong('\\\\?\\foo'), '\\\\?\\foo');
assert.equal(path.win32._makeLong(null), null);
assert.equal(path.win32._makeLong(true), true);
assert.equal(path.win32._makeLong(1), 1);
assert.equal(path.win32._makeLong(), undefined);
assert.equal(path.win32._makeLong(emptyObj), emptyObj);


if (common.isWindows)
Expand Down

0 comments on commit 1c6f927

Please sign in to comment.