Skip to content

Commit

Permalink
path: add type checking for path inputs
Browse files Browse the repository at this point in the history
This commit adds type checking of path inputs to exported methods
in the path module. The exception is _makeLong(), which seems to
explicitly support any data type.

Fixes: #1139
PR-URL: #1153
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Brendan Ashworth <brendan.ashworth@me.com>
  • Loading branch information
cjihrig committed Mar 16, 2015
1 parent a28945b commit eb995d6
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 30 deletions.
69 changes: 50 additions & 19 deletions lib/path.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
'use strict';

const util = require('util');
const isWindows = process.platform === 'win32';

function assertPath(path) {
if (typeof path !== 'string') {
throw new TypeError('Path must be a string. Received ' +
util.inspect(path));
}
}

// resolves . and .. elements in a path array with directory names there
// must be no slashes or device names (c:\) in the array
// (so also no leading and trailing slashes - it does not distinguish
Expand Down Expand Up @@ -84,10 +92,10 @@ win32.resolve = function() {
}
}

// Skip empty and invalid entries
if (typeof path !== 'string') {
throw new TypeError('Arguments to path.resolve must be strings');
} else if (!path) {
assertPath(path);

// Skip empty entries
if (path === '') {
continue;
}

Expand Down Expand Up @@ -137,6 +145,8 @@ win32.resolve = function() {


win32.normalize = function(path) {
assertPath(path);

var result = splitDeviceRe.exec(path),
device = result[1] || '',
isUnc = device && device.charAt(1) !== ':',
Expand Down Expand Up @@ -165,6 +175,8 @@ win32.normalize = function(path) {


win32.isAbsolute = function(path) {
assertPath(path);

var result = splitDeviceRe.exec(path),
device = result[1] || '',
isUnc = !!device && device.charAt(1) !== ':';
Expand Down Expand Up @@ -210,6 +222,9 @@ win32.join = function() {
// to = 'C:\\orandea\\impl\\bbb'
// The output of the function should be: '..\\..\\impl\\bbb'
win32.relative = function(from, to) {
assertPath(from);
assertPath(to);

from = win32.resolve(from);
to = win32.resolve(to);

Expand Down Expand Up @@ -287,6 +302,8 @@ win32._makeLong = function(path) {


win32.dirname = function(path) {
assertPath(path);

var result = win32SplitPath(path),
root = result[0],
dir = result[1];
Expand All @@ -306,6 +323,11 @@ win32.dirname = function(path) {


win32.basename = function(path, ext) {
assertPath(path);

if (ext !== undefined && typeof ext !== 'string')
throw new TypeError('ext must be a string');

var f = win32SplitPath(path)[2];
// TODO: make this comparison case-insensitive on windows?
if (ext && f.substr(-1 * ext.length) === ext) {
Expand All @@ -316,6 +338,7 @@ win32.basename = function(path, ext) {


win32.extname = function(path) {
assertPath(path);
return win32SplitPath(path)[3];
};

Expand Down Expand Up @@ -351,11 +374,8 @@ win32.format = function(pathObject) {


win32.parse = function(pathString) {
if (typeof pathString !== 'string') {
throw new TypeError(
"Parameter 'pathString' must be a string, not " + typeof pathString
);
}
assertPath(pathString);

var allParts = win32SplitPath(pathString);
if (!allParts || allParts.length !== 4) {
throw new TypeError("Invalid path '" + pathString + "'");
Expand Down Expand Up @@ -395,10 +415,10 @@ posix.resolve = function() {
for (var i = arguments.length - 1; i >= -1 && !resolvedAbsolute; i--) {
var path = (i >= 0) ? arguments[i] : process.cwd();

// Skip empty and invalid entries
if (typeof path !== 'string') {
throw new TypeError('Arguments to path.resolve must be strings');
} else if (!path) {
assertPath(path);

// Skip empty entries
if (path === '') {
continue;
}

Expand All @@ -419,6 +439,8 @@ posix.resolve = function() {
// path.normalize(path)
// posix version
posix.normalize = function(path) {
assertPath(path);

var isAbsolute = posix.isAbsolute(path),
trailingSlash = path.substr(-1) === '/';

Expand All @@ -437,6 +459,7 @@ posix.normalize = function(path) {

// posix version
posix.isAbsolute = function(path) {
assertPath(path);
return path.charAt(0) === '/';
};

Expand All @@ -463,6 +486,9 @@ posix.join = function() {
// path.relative(from, to)
// posix version
posix.relative = function(from, to) {
assertPath(from);
assertPath(to);

from = posix.resolve(from).substr(1);
to = posix.resolve(to).substr(1);

Expand Down Expand Up @@ -510,6 +536,8 @@ posix._makeLong = function(path) {


posix.dirname = function(path) {
assertPath(path);

var result = posixSplitPath(path),
root = result[0],
dir = result[1];
Expand All @@ -529,8 +557,13 @@ posix.dirname = function(path) {


posix.basename = function(path, ext) {
assertPath(path);

if (ext !== undefined && typeof ext !== 'string')
throw new TypeError('ext must be a string');

var f = posixSplitPath(path)[2];
// TODO: make this comparison case-insensitive on windows?

if (ext && f.substr(-1 * ext.length) === ext) {
f = f.substr(0, f.length - ext.length);
}
Expand All @@ -539,6 +572,7 @@ posix.basename = function(path, ext) {


posix.extname = function(path) {
assertPath(path);
return posixSplitPath(path)[3];
};

Expand Down Expand Up @@ -566,11 +600,8 @@ posix.format = function(pathObject) {


posix.parse = function(pathString) {
if (typeof pathString !== 'string') {
throw new TypeError(
"Parameter 'pathString' must be a string, not " + typeof pathString
);
}
assertPath(pathString);

var allParts = posixSplitPath(pathString);
if (!allParts || allParts.length !== 4) {
throw new TypeError("Invalid path '" + pathString + "'");
Expand Down
10 changes: 5 additions & 5 deletions test/parallel/test-path-parse-format.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ var unixPaths = [
];

var errors = [
{method: 'parse', input: [null], message: /Parameter 'pathString' must be a string, not/},
{method: 'parse', input: [{}], message: /Parameter 'pathString' must be a string, not object/},
{method: 'parse', input: [true], message: /Parameter 'pathString' must be a string, not boolean/},
{method: 'parse', input: [1], message: /Parameter 'pathString' must be a string, not number/},
{method: 'parse', input: [], message: /Parameter 'pathString' must be a string, not undefined/},
{method: 'parse', input: [null], message: /Path must be a string. Received null/},
{method: 'parse', input: [{}], message: /Path must be a string. Received {}/},
{method: 'parse', input: [true], message: /Path must be a string. Received true/},
{method: 'parse', input: [1], message: /Path must be a string. Received 1/},
{method: 'parse', input: [], message: /Path must be a string. Received undefined/},
// {method: 'parse', input: [''], message: /Invalid path/}, // omitted because it's hard to trigger!
{method: 'format', input: [null], message: /Parameter 'pathObject' must be an object, not/},
{method: 'format', input: [''], message: /Parameter 'pathObject' must be an object, not string/},
Expand Down
32 changes: 26 additions & 6 deletions test/parallel/test-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,14 +253,34 @@ joinTests.forEach(function(test) {
// assert.equal(actual, expected, message);
});
assert.equal(failures.length, 0, failures.join(''));
var joinThrowTests = [true, false, 7, null, {}, undefined, [], NaN];
joinThrowTests.forEach(function(test) {
assert.throws(function() {
path.join(test);
}, TypeError);

// Test thrown TypeErrors
var typeErrorTests = [true, false, 7, null, {}, undefined, [], NaN];

function fail(fn) {
var args = Array.prototype.slice.call(arguments, 1);

assert.throws(function() {
path.resolve(test);
fn.apply(null, args);
}, TypeError);
}

typeErrorTests.forEach(function(test) {
fail(path.join, test);
fail(path.resolve, test);
fail(path.normalize, test);
fail(path.isAbsolute, test);
fail(path.dirname, test);
fail(path.relative, test, 'foo');
fail(path.relative, 'foo', test);
fail(path.basename, test);
fail(path.extname, test);
fail(path.parse, test);

// undefined is a valid value as the second argument to basename
if (test !== undefined) {
fail(path.basename, 'foo', test);
}
});


Expand Down

0 comments on commit eb995d6

Please sign in to comment.