Support file:// URLs and Buffers for path args, throw when invalid

README.md

@@ -232,7 +232,7 @@ The node.js [path module](http://nodejs.org/api/path.html) is available via the
 identical to the node.js (see [https://github.com/browserify/path-browserify](https://github.com/browserify/path-browserify)) version with the following differences:
 
 * The CWD always defaults to `/`
-* No support for Windows style paths
+* No support for Windows style paths (assume you are on a POSIX system)
 * Additional utility methods (see below)
 
 ```javascript
@@ -267,6 +267,21 @@ Filer.Path also includes the following extra methods:
 * `addTrailing(p)` returns the path `p` with a single trailing slash added
 * `removeTrailing(p)` returns the path `p` with trailing slash(es) removed
 
+[As with node.js](https://nodejs.org/api/fs.html#fs_file_paths), all methods below that
+accept a `path` argument as a `String` can also take a [`file://` URL](https://nodejs.org/api/fs.html#fs_url_object_support)
+or a `Buffer`. For example, all of the following cases will work the same way with Filer:
+
+```js
+// 1. path as a String
+fs.writeFile('/dir/file.txt', 'data', function(err) {...});
+
+// 2. path as a URL
+fs.writeFile(new URL('file:///dir/file.txt'), 'data', function(err) {...});
+
+// 3. path as a Buffer
+fs.writeFile(Buffer.from('/dir/file.txt'), 'data', function(err) {...});
+```
+
 #### Filer.Errors<a name="Errors"></a>
 
 The error objects used internally by Filer are also exposed via the `Filer.Errors` object. As much as possible

src/filesystem/implementation.js

@@ -3,7 +3,6 @@ var normalize = Path.normalize;
 var dirname = Path.dirname;
 var basename = Path.basename;
 var isAbsolutePath = Path.isAbsolute;
-var isNullPath = Path.isNull;
 var shared = require('../shared.js');
 
 var Constants = require('../constants.js');
@@ -1627,30 +1626,6 @@ function validate_file_options(options, enc, fileMode){
   return options;
 }
 
-function pathCheck(path, allowRelative, callback) {
-  var err;
-
-  if(typeof allowRelative === 'function') {
-    callback = allowRelative;
-    allowRelative = false;
-  }
-
-  if(!path) {
-    err = new Errors.EINVAL('Path must be a string', path);
-  } else if(isNullPath(path)) {
-    err = new Errors.EINVAL('Path must be a string without null bytes.', path);
-  } else if(!allowRelative && !isAbsolutePath(path)) {
-    err = new Errors.EINVAL('Path must be absolute.', path);
-  }
-
-  if(err) {
-    callback(err);
-    return false;
-  }
-  return true;
-}
-
-
 function open(context, path, flags, mode, callback) {
   if (arguments.length < 5 ){
     callback = arguments[arguments.length - 1];
@@ -1659,8 +1634,6 @@ function open(context, path, flags, mode, callback) {
   else {
     mode = validateAndMaskMode(mode, FULL_READ_WRITE_EXEC_PERMISSIONS, callback);
   }
-    
-  if(!pathCheck(path, callback)) return;
 
   function check_result(error, fileNode) {
     if(error) {
@@ -1696,7 +1669,6 @@ function close(context, fd, callback) {
 }
 
 function mknod(context, path, type, callback) {
-  if(!pathCheck(path, callback)) return;
   make_node(context, path, type, callback);
 }
 
@@ -1709,7 +1681,6 @@ function mkdir(context, path, mode, callback) {
     if(!mode) return;
   }
  
-  if(!pathCheck(path, callback)) return;
   make_directory(context, path, callback);
 }
 
@@ -1719,7 +1690,6 @@ function access(context, path, mode, callback) {
     mode = Constants.fsConstants.F_OK;
   }
 
-  if (!pathCheck(path, callback)) return;
   mode = mode | Constants.fsConstants.F_OK;
   access_file(context, path, mode, callback);
 }
@@ -1733,20 +1703,16 @@ function mkdtemp(context, prefix, options, callback) {
   let random = shared.randomChars(6);
   var path = prefix + '-' + random; 
 
-  if(!pathCheck(path, callback)) return;
   make_directory(context, path, function(error) {
     callback(error, path);
   });  
 }
 
 function rmdir(context, path, callback) {
-  if(!pathCheck(path, callback)) return;
   remove_directory(context, path, callback);
 }
 
 function stat(context, path, callback) {
-  if(!pathCheck(path, callback)) return;
-
   function check_result(error, result) {
     if(error) {
       callback(error);
@@ -1778,13 +1744,10 @@ function fstat(context, fd, callback) {
 }
 
 function link(context, oldpath, newpath, callback) {
-  if(!pathCheck(oldpath, callback)) return;
-  if(!pathCheck(newpath, callback)) return;
   link_node(context, oldpath, newpath, callback);
 }
 
 function unlink(context, path, callback) {
-  if(!pathCheck(path, callback)) return;
   unlink_node(context, path, callback);
 }
 
@@ -1823,8 +1786,6 @@ function readFile(context, path, options, callback) {
   callback = arguments[arguments.length - 1];
   options = validate_file_options(options, null, 'r');
 
-  if(!pathCheck(path, callback)) return;
-
   var flags = validate_flags(options.flag || 'r');
   if(!flags) {
     return callback(new Errors.EINVAL('flags is not valid', path));
@@ -1897,8 +1858,6 @@ function writeFile(context, path, data, options, callback) {
   callback = arguments[arguments.length - 1];
   options = validate_file_options(options, 'utf8', 'w');
 
-  if(!pathCheck(path, callback)) return;
-
   var flags = validate_flags(options.flag || 'w');
   if(!flags) {
     return callback(new Errors.EINVAL('flags is not valid', path));
@@ -1934,8 +1893,6 @@ function appendFile(context, path, data, options, callback) {
   callback = arguments[arguments.length - 1];
   options = validate_file_options(options, 'utf8', 'a');
 
-  if(!pathCheck(path, callback)) return;
-
   var flags = validate_flags(options.flag || 'a');
   if(!flags) {
     return callback(new Errors.EINVAL('flags is not valid', path));
@@ -2098,7 +2055,6 @@ function fchown_file(context, ofd, uid, gid, callback) {
 }
 
 function getxattr(context, path, name, callback) {
-  if (!pathCheck(path, callback)) return;
   getxattr_file(context, path, name, callback);
 }
 
@@ -2118,7 +2074,6 @@ function setxattr(context, path, name, value, flag, callback) {
     flag = null;
   }
 
-  if (!pathCheck(path, callback)) return;
   setxattr_file(context, path, name, value, flag, callback);
 }
 
@@ -2141,7 +2096,6 @@ function fsetxattr(context, fd, name, value, flag, callback) {
 }
 
 function removexattr(context, path, name, callback) {
-  if (!pathCheck(path, callback)) return;
   removexattr_file(context, path, name, callback);
 }
 
@@ -2199,7 +2153,6 @@ function lseek(context, fd, offset, whence, callback) {
 }
 
 function readdir(context, path, callback) {
-  if(!pathCheck(path, callback)) return;
   read_directory(context, path, callback);
 }
 
@@ -2213,8 +2166,6 @@ function toUnixTimestamp(time) {
 }
 
 function utimes(context, path, atime, mtime, callback) {
-  if(!pathCheck(path, callback)) return;
-
   var currentTime = Date.now();
   atime = (atime) ? toUnixTimestamp(atime) : toUnixTimestamp(currentTime);
   mtime = (mtime) ? toUnixTimestamp(mtime) : toUnixTimestamp(currentTime);
@@ -2238,7 +2189,6 @@ function futimes(context, fd, atime, mtime, callback) {
 }
 
 function chmod(context, path, mode, callback) {
-  if(!pathCheck(path, callback)) return;
   mode = validateAndMaskMode(mode, callback);
   if(!mode) return;
 
@@ -2260,7 +2210,6 @@ function fchmod(context, fd, mode, callback) {
 }
 
 function chown(context, path, uid, gid, callback) {
-  if(!pathCheck(path, callback)) return;
   if(!isUint32(uid)) {
     return callback(new Errors.EINVAL('uid must be a valid integer', uid));
   }
@@ -2290,9 +2239,6 @@ function fchown(context, fd, uid, gid, callback) {
 }
 
 function rename(context, oldpath, newpath, callback) {
-  if(!pathCheck(oldpath, callback)) return;
-  if(!pathCheck(newpath, callback)) return;
-
   oldpath = normalize(oldpath);
   newpath = normalize(newpath);
 
@@ -2407,23 +2353,14 @@ function rename(context, oldpath, newpath, callback) {
 function symlink(context, srcpath, dstpath, type, callback) {
   // NOTE: we support passing the `type` arg, but ignore it.
   callback = arguments[arguments.length - 1];
-
-  // Special Case: allow srcpath to be relative, which we normally don't permit.
-  // If the srcpath is relative, we assume it's relative to the dirpath of dstpath.
-  if(!pathCheck(srcpath, true, callback)) return;
-  if(!pathCheck(dstpath, callback)) return;
-
   make_symbolic_link(context, srcpath, dstpath, callback);
 }
 
 function readlink(context, path, callback) {
-  if(!pathCheck(path, callback)) return;
   read_link(context, path, callback);
 }
 
 function lstat(context, path, callback) {
-  if(!pathCheck(path, callback)) return;
-
   function check_result(error, result) {
     if(error) {
       callback(error);
@@ -2441,7 +2378,6 @@ function truncate(context, path, length, callback) {
   callback = arguments[arguments.length - 1];
   length = length || 0;
 
-  if(!pathCheck(path, callback)) return;
   if(validateInteger(length, callback) !== length) return;
 
   truncate_file(context, path, length, callback);

src/filesystem/interface.js

@@ -1,6 +1,6 @@
 var { promisify } = require('es6-promisify');
 
-var isNullPath = require('../path.js').isNull;
+var Path = require('../path.js');
 var nop = require('../shared.js').nop;
 
 var Constants = require('../constants.js');
@@ -45,6 +45,61 @@ function defaultCallback(err) {
     console.error('Filer error: ', err);
   }
 }
+// Get a path (String) from a file:// URL. Support URL() like objects
+// https://github.com/nodejs/node/blob/968e901aff38a343b1de4addebf79fd8fa991c59/lib/internal/url.js#L1381
+function toPathIfFileURL(fileURLOrPath) {
+  if(!(fileURLOrPath &&
+       fileURLOrPath.protocol &&
+       fileURLOrPath.pathname)) {
+    return fileURLOrPath;
+  }
+
+  if(fileURLOrPath.protocol !== 'file:') {
+    throw new Errors.EINVAL('only file: URLs are supported for paths', fileURLOrPath);
+  }
+
+  var pathname = fileURLOrPath.pathname;
+  for (var n = 0; n < pathname.length; n++) {
+    if (pathname[n] === '%') {
+      var third = pathname.codePointAt(n + 2) | 0x20;
+      if (pathname[n + 1] === '2' && third === 102) {
+        throw new Errors.EINVAL('file: URLs must not include encoded / characters', fileURLOrPath);
+      }
+    }
+  }
+
+  return decodeURIComponent(pathname);
+}
+
+// Allow Buffers for paths. Assumes we want UTF8.
+function toPathIfBuffer(bufferOrPath) {
+  return Buffer.isBuffer(bufferOrPath) ? bufferOrPath.toString() : bufferOrPath;
+}
+
+function validatePath(path, allowRelative) {
+  if(!path) {
+    return new Errors.EINVAL('Path must be a string', path);
+  } else if(Path.isNull(path)) {
+    return new Errors.EINVAL('Path must be a string without null bytes.', path);
+  } else if(!allowRelative && !Path.isAbsolute(path)) {
+    return new Errors.EINVAL('Path must be absolute.', path);
+  }
+}
+
+function processPathArg(args, idx, allowRelative) {
+  var path = args[idx];
+  path = toPathIfFileURL(path);
+  path = toPathIfBuffer(path);
+
+  // Some methods specifically allow for rel paths (eg symlink with srcPath)
+  var err = validatePath(path, allowRelative);
+  if(err) {
+    throw err;
+  }
+
+  // Overwrite path arg with converted and validated path
+  args[idx] = path;
+}
 
 /**
  * FileSystem
@@ -124,7 +179,7 @@ function FileSystem(options, callback) {
 
   // We support the optional `options` arg from node, but ignore it
   this.watch = function(filename, options, listener) {
-    if(isNullPath(filename)) {
+    if(Path.isNull(filename)) {
       throw new Error('Path must be a string without null bytes.');
     }
     if(typeof options === 'function') {
@@ -247,50 +302,53 @@ function FileSystem(options, callback) {
     }
   });
   FileSystem.prototype.promises = {};
+
   /**
-   * Public API for FileSystem. All node.js methods that are
+   * Public API for FileSystem. All node.js methods that are exposed on fs.promises
-   * exposed on fs.promises include `promise: true`.  We also
+   * include `promise: true`.  We also include our own extra methods, but skip the
-   * include our own extra methods, but skip the fd versions
+   * fd versions to match node.js, which puts these on a `FileHandle` object.
-   * to match node.js, which puts these on a FileHandle object.
+   * Any method that deals with path argument(s) also includes the position of
+   * those args in one of `absPathArgs: [...]` or `relPathArgs: [...]`, so they
+   * can be processed and validated before being passed on to the method.
    */
   [
-    { name: 'open', promises: true },
+    { name: 'open', promises: true, absPathArgs: [ 0 ] },
-    { name: 'access', promises: true },
+    { name: 'access', promises: true, absPathArgs: [ 0 ] },
-    { name: 'chmod', promises: true },
+    { name: 'chmod', promises: true, absPathArgs: [ 0 ] },
     { name: 'fchmod' },
-    { name: 'chown', promises: true },
+    { name: 'chown', promises: true, absPathArgs: [ 0 ] },
     { name: 'fchown' },
     { name: 'close' },
-    { name: 'mknod', promises: true },
+    { name: 'mknod', promises: true, absPathArgs: [ 0 ] },
-    { name: 'mkdir', promises: true },
+    { name: 'mkdir', promises: true, absPathArgs: [ 0 ] },
     { name: 'mkdtemp', promises: true },
-    { name: 'rmdir', promises: true },
+    { name: 'rmdir', promises: true, absPathArgs: [ 0 ] },
-    { name: 'stat', promises: true },
+    { name: 'stat', promises: true, absPathArgs: [ 0 ] },
     { name: 'fstat' },
     { name: 'fsync' },
-    { name: 'link', promises: true },
+    { name: 'link', promises: true, absPathArgs: [0, 1] },
-    { name: 'unlink', promises: true },
+    { name: 'unlink', promises: true, absPathArgs: [ 0 ] },
     { name: 'read' },
-    { name: 'readFile', promises: true },
+    { name: 'readFile', promises: true, absPathArgs: [ 0 ] },
     { name: 'write' },
-    { name: 'writeFile', promises: true },
+    { name: 'writeFile', promises: true, absPathArgs: [ 0 ] },
-    { name: 'appendFile', promises: true },
+    { name: 'appendFile', promises: true, absPathArgs: [ 0 ] },
-    { name: 'exists' },
+    { name: 'exists', absPathArgs: [ 0 ] },
     { name: 'lseek' },
-    { name: 'readdir', promises: true },
+    { name: 'readdir', promises: true, absPathArgs: [ 0 ] },
-    { name: 'rename', promises: true },
+    { name: 'rename', promises: true, absPathArgs: [0, 1] },
-    { name: 'readlink', promises: true },
+    { name: 'readlink', promises: true, absPathArgs: [ 0 ] },
-    { name: 'symlink', promises: true },
+    { name: 'symlink', promises: true, relPathPargs: [ 0 ], absPathArgs: [ 1 ] },
     { name: 'lstat', promises: true },
-    { name: 'truncate', promises: true },
+    { name: 'truncate', promises: true, absPathArgs: [ 0 ] },
     { name: 'ftruncate' },
-    { name: 'utimes', promises: true },
+    { name: 'utimes', promises: true, absPathArgs: [ 0 ] },
     { name: 'futimes' },
-    { name: 'setxattr', promises: true },
+    { name: 'setxattr', promises: true, absPathArgs: [ 0 ] },
-    { name: 'getxattr', promises: true },
+    { name: 'getxattr', promises: true, absPathArgs: [ 0 ] },
     { name: 'fsetxattr' },
     { name: 'fgetxattr' },
-    { name: 'removexattr', promises: true },
+    { name: 'removexattr', promises: true, absPathArgs: [ 0 ] },
     { name: 'fremovexattr' }
   ].forEach(function(method) {
     var methodName = method.name;
@@ -306,6 +364,14 @@ function FileSystem(options, callback) {
       var missingCallback = typeof args[lastArgIndex] !== 'function';
       var callback = maybeCallback(args[lastArgIndex]);
 
+      // Deal with path arguments, validating and normalizing Buffer and file:// URLs
+      if(method.absPathArgs) {
+        method.absPathArgs.forEach(pathArg => processPathArg(args, pathArg, false));
+      }
+      if(method.relPathPargs) {
+        method.relPathPargs.forEach(pathArg => processPathArg(args, pathArg, true));
+      }
+
       var error = fs.queueOrRun(function() {
         var context = fs.provider.openReadWriteContext();
 

tests/bugs/issue270.js

@@ -5,23 +5,15 @@ describe('undefined and relative paths, issue270', function() {
   beforeEach(util.setup);
   afterEach(util.cleanup);
 
-  it('should fail with EINVAL when called on an undefined path', function(done) {
+  it('should fail with EINVAL when called on an undefined path', function() {
     var fs = util.fs();
-
+    var fn = () => fs.writeFile(undefined, 'data');
-    fs.writeFile(undefined, 'data', function(err) {
+    expect(fn).to.throw();
-      expect(err).to.exist;
-      expect(err.code).to.equal('EINVAL');
-      done();
-    });
   });
 
-  it('should fail with EINVAL when called on a relative path', function(done) {
+  it('should fail with EINVAL when called on a relative path', function() {
     var fs = util.fs();
-
+    var fn = () => fs.writeFile('relpath/file.txt', 'data');
-    fs.writeFile('relpath/file.txt', 'data', function(err) {
+    expect(fn).to.throw();
-      expect(err).to.exist;
-      expect(err.code).to.equal('EINVAL');
-      done();
-    });
   });
 });

tests/spec/fs.write.spec.js

@@ -10,14 +10,10 @@ describe('fs.write', function() {
     expect(fs.write).to.be.a('function');
   });
 
-  it('should error if file path is undefined',function(done) {
+  it('should error if file path is undefined', function() {
     var fs = util.fs();
-
+    var fn = () => fs.writeFile(undefined, 'data');
-    fs.writeFile(undefined, 'data', function(error) {
+    expect(fn).to.throw();
-      expect(error).to.exist;
-      expect(error.code).to.equal('EINVAL');
-      done();
-    });
   });
 
   it('should write data to a file', function(done) {

tests/spec/node-js/simple/test-fs-null-bytes.js

@@ -9,23 +9,18 @@ describe('node.js tests: https://github.com/joyent/node/blob/master/test/simple/
     var checks = [];
     var fnCount = 0;
     var fnTotal = 16;
-    var expected = 'Path must be a string without null bytes.';
     var fs = util.fs();
 
     // Make sure function fails with null path error in callback.
     function check(fn) {
       var args = Array.prototype.slice.call(arguments, 1);
-      args = args.concat(function(err) {
+      fn = () => fn.apply(fs, args);
-        checks.push(function(){
+      expect(fn).to.throw();
-          expect(err).to.exist;
+
-          expect(err.message).to.equal(expected);
+      fnCount++;
-        });
+      if(fnCount === fnTotal) {
-        fnCount++;
+        done();
-        if(fnCount === fnTotal) {
+      }
-          done();
-        }
-      });
-      fn.apply(fs, args);
     }
 
     check(fs.link,        '/foo\u0000bar', 'foobar');

tests/spec/path-resolution.spec.js

@@ -2,6 +2,9 @@ var Filer = require('../../src');
 var util = require('../lib/test-utils.js');
 var expect = require('chai').expect;
 
+// Support global URL and node's url module
+var URL = global.URL || require('url').URL;
+
 describe('path resolution', function() {
   beforeEach(util.setup);
   afterEach(util.cleanup);
@@ -245,4 +248,52 @@ describe('path resolution', function() {
     expect(Path.removeTrailing('/dir/')).to.equal('/dir');
     expect(Path.removeTrailing('/dir//')).to.equal('/dir');
   });
+
+  it('should allow using Buffer for paths', function(done) {
+    var fs = util.fs();
+    var filePath = '/file';
+    var bufferPath = Buffer.from(filePath);
+    var data = 'data';
+
+    fs.writeFile(bufferPath, data, function(err) {
+      if(err) throw err;
+
+      fs.readFile(filePath, 'utf8', function(err, result) {
+        if(err) throw err;
+        expect(result).to.equal(data);
+        done();
+      });
+    });
+  });
+
+  it('should allow using file: URLs for paths', function(done) {
+    var fs = util.fs();
+    var filePath = '/file';
+    var fileUrl = new URL(`file://${filePath}`);
+    var data = 'data';
+
+    fs.writeFile(fileUrl, data, function(err) {
+      if(err) throw err;
+
+      fs.readFile(filePath, 'utf8', function(err, result) {
+        if(err) throw err;
+        expect(result).to.equal(data);
+        done();
+      });
+    });
+  });
+
+  it('should error for non file: URLs for paths', function() {
+    var fs = util.fs();
+    var fileUrl = new URL('http://file');
+    var fn = () => fs.writeFile(fileUrl, 1);
+    expect(fn).to.throw();
+  });
+
+  it('should error if file: URLs include escaped / characters', function() {
+    var fs = util.fs();
+    var fileUrl = new URL('file:///p/a/t/h/%2F');
+    var fn = () => fs.writeFile(fileUrl, 1);
+    expect(fn).to.throw();
+  });
 });