From a465840313f548b913eb2bd8ea3d26c2ab5dcebb Mon Sep 17 00:00:00 2001 From: cjihrig Date: Fri, 13 Mar 2015 22:21:47 -0400 Subject: [PATCH] path: add type checking for path inputs 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. --- lib/path.js | 68 ++++++++++++++++++------- test/parallel/test-path-parse-format.js | 10 ++-- test/parallel/test-path.js | 32 +++++++++--- 3 files changed, 80 insertions(+), 30 deletions(-) diff --git a/lib/path.js b/lib/path.js index 6259c5fd1d8a4a..3c48c7f9359e67 100644 --- a/lib/path.js +++ b/lib/path.js @@ -1,7 +1,14 @@ '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 @@ -84,10 +91,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; } @@ -137,6 +144,8 @@ win32.resolve = function() { win32.normalize = function(path) { + assertPath(path); + var result = splitDeviceRe.exec(path), device = result[1] || '', isUnc = device && device.charAt(1) !== ':', @@ -165,6 +174,8 @@ win32.normalize = function(path) { win32.isAbsolute = function(path) { + assertPath(path); + var result = splitDeviceRe.exec(path), device = result[1] || '', isUnc = !!device && device.charAt(1) !== ':'; @@ -210,6 +221,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); @@ -287,6 +301,8 @@ win32._makeLong = function(path) { win32.dirname = function(path) { + assertPath(path); + var result = win32SplitPath(path), root = result[0], dir = result[1]; @@ -306,6 +322,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) { @@ -316,6 +337,7 @@ win32.basename = function(path, ext) { win32.extname = function(path) { + assertPath(path); return win32SplitPath(path)[3]; }; @@ -351,11 +373,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 + "'"); @@ -395,10 +414,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; } @@ -419,6 +438,8 @@ posix.resolve = function() { // path.normalize(path) // posix version posix.normalize = function(path) { + assertPath(path); + var isAbsolute = posix.isAbsolute(path), trailingSlash = path.substr(-1) === '/'; @@ -437,6 +458,7 @@ posix.normalize = function(path) { // posix version posix.isAbsolute = function(path) { + assertPath(path); return path.charAt(0) === '/'; }; @@ -463,6 +485,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); @@ -510,6 +535,8 @@ posix._makeLong = function(path) { posix.dirname = function(path) { + assertPath(path); + var result = posixSplitPath(path), root = result[0], dir = result[1]; @@ -529,8 +556,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); } @@ -539,6 +571,7 @@ posix.basename = function(path, ext) { posix.extname = function(path) { + assertPath(path); return posixSplitPath(path)[3]; }; @@ -566,11 +599,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 + "'"); diff --git a/test/parallel/test-path-parse-format.js b/test/parallel/test-path-parse-format.js index a3120891fe7016..d31dc995720030 100644 --- a/test/parallel/test-path-parse-format.js +++ b/test/parallel/test-path-parse-format.js @@ -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/}, diff --git a/test/parallel/test-path.js b/test/parallel/test-path.js index 6227369e7532f7..e1bbe8ae7215ed 100644 --- a/test/parallel/test-path.js +++ b/test/parallel/test-path.js @@ -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); + } });