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.

Fixes: #1139
PR-URL: #1153
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Brendan Ashworth <[email protected]>

Author: cjihrig

lib/path.js

@@ -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
@@ -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;
     }
 
@@ -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) !== ':',
@@ -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) !== ':';
@@ -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);
 
@@ -287,6 +302,8 @@ win32._makeLong = function(path) {
 
 
 win32.dirname = function(path) {
+  assertPath(path);
+
   var result = win32SplitPath(path),
       root = result[0],
       dir = result[1];
@@ -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) {
@@ -316,6 +338,7 @@ win32.basename = function(path, ext) {
 
 
 win32.extname = function(path) {
+  assertPath(path);
   return win32SplitPath(path)[3];
 };
 
@@ -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 + "'");
@@ -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;
     }
 
@@ -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) === '/';
 
@@ -437,6 +459,7 @@ posix.normalize = function(path) {
 
 // posix version
 posix.isAbsolute = function(path) {
+  assertPath(path);
   return path.charAt(0) === '/';
 };
 
@@ -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);
 
@@ -510,6 +536,8 @@ posix._makeLong = function(path) {
 
 
 posix.dirname = function(path) {
+  assertPath(path);
+
   var result = posixSplitPath(path),
       root = result[0],
       dir = result[1];
@@ -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);
   }
@@ -539,6 +572,7 @@ posix.basename = function(path, ext) {
 
 
 posix.extname = function(path) {
+  assertPath(path);
   return posixSplitPath(path)[3];
 };
 
@@ -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 + "'");

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/},

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);
+  }
 });