Merge pull request #293 from tschaub/recursive-rmdir

fix: support path input from Buffer on many fs APIs

lib/binding.js

@@ -164,6 +164,10 @@ function notImplemented() {
   throw new Error('Method not implemented');
 }
 
+function deBuffer(p) {
+  return Buffer.isBuffer(p) ? p.toString() : p;
+}
+
 /**
  * Create a new stats object.
  * @param {Object} config Stats properties.
@@ -342,9 +346,7 @@ Binding.prototype.realpath = function(filepath, encoding, callback, ctx) {
 
   return maybeCallback(normalizeCallback(callback), ctx, this, function() {
     let realPath;
-    if (Buffer.isBuffer(filepath)) {
-      filepath = filepath.toString();
-    }
+    filepath = deBuffer(filepath);
     const resolved = path.resolve(filepath);
     const parts = getPathParts(resolved);
     let item = _system.getRoot();
@@ -450,6 +452,7 @@ Binding.prototype.stat = function(filepath, options, callback, ctx) {
   markSyscall(ctx, 'stat');
 
   return maybeCallback(wrapStatsCallback(callback), ctx, this, function() {
+    filepath = deBuffer(filepath);
     let item = _system.getItem(filepath);
     if (item instanceof SymbolicLink) {
       item = _system.getItem(
@@ -539,6 +542,7 @@ Binding.prototype.open = function(pathname, flags, mode, callback, ctx) {
   markSyscall(ctx, 'open');
 
   return maybeCallback(normalizeCallback(callback), ctx, this, function() {
+    pathname = deBuffer(pathname);
     const descriptor = new FileDescriptor(flags);
     let item = _system.getItem(pathname);
     while (item instanceof SymbolicLink) {
@@ -677,6 +681,8 @@ Binding.prototype.copyFile = function(src, dest, flags, callback, ctx) {
   markSyscall(ctx, 'copyfile');
 
   return maybeCallback(normalizeCallback(callback), ctx, this, function() {
+    src = deBuffer(src);
+    dest = deBuffer(dest);
     const srcFd = this.open(src, constants.O_RDONLY);
 
     try {
@@ -876,6 +882,8 @@ Binding.prototype.rename = function(oldPath, newPath, callback, ctx) {
   markSyscall(ctx, 'rename');
 
   return maybeCallback(normalizeCallback(callback), ctx, this, function() {
+    oldPath = deBuffer(oldPath);
+    newPath = deBuffer(newPath);
     const oldItem = _system.getItem(oldPath);
     if (!oldItem) {
       throw new FSError('ENOENT', oldPath);
@@ -941,6 +949,7 @@ Binding.prototype.readdir = function(
   markSyscall(ctx, 'scandir');
 
   return maybeCallback(normalizeCallback(callback), ctx, this, function() {
+    dirpath = deBuffer(dirpath);
     let dpath = dirpath;
     let dir = _system.getItem(dirpath);
     while (dir instanceof SymbolicLink) {
@@ -993,6 +1002,7 @@ Binding.prototype.mkdir = function(pathname, mode, recursive, callback, ctx) {
   markSyscall(ctx, 'mkdir');
 
   return maybeCallback(normalizeCallback(callback), ctx, this, function() {
+    pathname = deBuffer(pathname);
     const item = _system.getItem(pathname);
     if (item) {
       if (recursive && item instanceof Directory) {
@@ -1033,6 +1043,7 @@ Binding.prototype.rmdir = function(pathname, callback, ctx) {
   markSyscall(ctx, 'rmdir');
 
   return maybeCallback(normalizeCallback(callback), ctx, this, function() {
+    pathname = deBuffer(pathname);
     const item = _system.getItem(pathname);
     if (!item) {
       throw new FSError('ENOENT', pathname);
@@ -1161,6 +1172,7 @@ Binding.prototype.chown = function(pathname, uid, gid, callback, ctx) {
   markSyscall(ctx, 'chown');
 
   return maybeCallback(normalizeCallback(callback), ctx, this, function() {
+    pathname = deBuffer(pathname);
     const item = _system.getItem(pathname);
     if (!item) {
       throw new FSError('ENOENT', pathname);
@@ -1200,6 +1212,7 @@ Binding.prototype.chmod = function(pathname, mode, callback, ctx) {
   markSyscall(ctx, 'chmod');
 
   return maybeCallback(normalizeCallback(callback), ctx, this, function() {
+    pathname = deBuffer(pathname);
     const item = _system.getItem(pathname);
     if (!item) {
       throw new FSError('ENOENT', pathname);
@@ -1235,6 +1248,7 @@ Binding.prototype.unlink = function(pathname, callback, ctx) {
   markSyscall(ctx, 'unlink');
 
   return maybeCallback(normalizeCallback(callback), ctx, this, function() {
+    pathname = deBuffer(pathname);
     const item = _system.getItem(pathname);
     if (!item) {
       throw new FSError('ENOENT', pathname);
@@ -1259,6 +1273,7 @@ Binding.prototype.utimes = function(pathname, atime, mtime, callback, ctx) {
   markSyscall(ctx, 'utimes');
 
   return maybeCallback(normalizeCallback(callback), ctx, this, function() {
+    pathname = deBuffer(pathname);
     const item = _system.getItem(pathname);
     if (!item) {
       throw new FSError('ENOENT', pathname);
@@ -1326,6 +1341,8 @@ Binding.prototype.link = function(srcPath, destPath, callback, ctx) {
   markSyscall(ctx, 'link');
 
   return maybeCallback(normalizeCallback(callback), ctx, this, function() {
+    srcPath = deBuffer(srcPath);
+    destPath = deBuffer(destPath);
     const item = _system.getItem(srcPath);
     if (!item) {
       throw new FSError('ENOENT', srcPath);
@@ -1359,6 +1376,8 @@ Binding.prototype.symlink = function(srcPath, destPath, type, callback, ctx) {
   markSyscall(ctx, 'symlink');
 
   return maybeCallback(normalizeCallback(callback), ctx, this, function() {
+    srcPath = deBuffer(srcPath);
+    destPath = deBuffer(destPath);
     if (_system.getItem(destPath)) {
       throw new FSError('EEXIST', destPath);
     }
@@ -1393,6 +1412,7 @@ Binding.prototype.readlink = function(pathname, encoding, callback, ctx) {
   markSyscall(ctx, 'readlink');
 
   return maybeCallback(normalizeCallback(callback), ctx, this, function() {
+    pathname = deBuffer(pathname);
     const link = _system.getItem(pathname);
     if (!link) {
       throw new FSError('ENOENT', pathname);
@@ -1426,6 +1446,7 @@ Binding.prototype.lstat = function(filepath, options, callback, ctx) {
   markSyscall(ctx, 'lstat');
 
   return maybeCallback(wrapStatsCallback(callback), ctx, this, function() {
+    filepath = deBuffer(filepath);
     const item = _system.getItem(filepath);
     if (!item) {
       throw new FSError('ENOENT', filepath);
@@ -1458,6 +1479,7 @@ Binding.prototype.access = function(filepath, mode, callback, ctx) {
   markSyscall(ctx, 'access');
 
   return maybeCallback(normalizeCallback(callback), ctx, this, function() {
+    filepath = deBuffer(filepath);
     let item = _system.getItem(filepath);
     let links = 0;
     while (item instanceof SymbolicLink) {

test/helper.js

@@ -15,8 +15,27 @@ chai.config.includeStack = true;
  */
 exports.assert = chai.assert;
 
-const TEST = {it: it, xit: xit, describe: describe, xdescribe: xdescribe};
-const NO_TEST = {it: xit, xit: xit, describe: xdescribe, xdescribe: xdescribe};
+function run(func) {
+  func();
+}
+
+function noRun() {}
+
+const TEST = {
+  it: it,
+  xit: xit,
+  describe: describe,
+  xdescribe: xdescribe,
+  run: run
+};
+
+const NO_TEST = {
+  it: xit,
+  xit: xit,
+  describe: xdescribe,
+  xdescribe: xdescribe,
+  run: noRun
+};
 
 exports.inVersion = function(range) {
   if (semver.satisfies(process.version, range)) {

test/lib/fs.access.spec.js

@@ -63,6 +63,10 @@ if (fs.access && fs.accessSync && process.getuid && process.getgid) {
       fs.access('path/to/accessible/file', done);
     });
 
+    it('supports Buffer input', function(done) {
+      fs.access(Buffer.from('path/to/accessible/file'), done);
+    });
+
     withPromise.it('promise works for an accessible file', function(done) {
       fs.promises.access('path/to/accessible/file').then(done, done);
     });

test/lib/fs.chmod-fchmod.spec.js

@@ -26,6 +26,17 @@ describe('fs.chmod(path, mode, callback)', function() {
     });
   });
 
+  it('supports Buffer input', function(done) {
+    fs.chmod(Buffer.from('file.txt'), parseInt('0664', 8), function(err) {
+      if (err) {
+        return done(err);
+      }
+      const stats = fs.statSync(Buffer.from('file.txt'));
+      assert.equal(stats.mode & parseInt('0777', 8), parseInt('0664', 8));
+      done();
+    });
+  });
+
   withPromise.it('promise changes permissions of a file', function(done) {
     fs.promises.chmod('file.txt', parseInt('0664', 8)).then(function() {
       const stats = fs.statSync('file.txt');

test/lib/fs.chown-fchown.spec.js

@@ -20,6 +20,10 @@ describe('fs.chown(path, uid, gid, callback)', function() {
     fs.chown('file.txt', 42, 43, done);
   });
 
+  it('supports Buffer input', function(done) {
+    fs.chown(Buffer.from('file.txt'), 42, 43, done);
+  });
+
   withPromise.it('promise changes ownership of a file', function(done) {
     fs.promises.chown('file.txt', 42, 43).then(done, done);
   });

test/lib/fs.copyFile.spec.js

@@ -27,6 +27,22 @@ if (fs.copyFile && fs.copyFileSync) {
       });
     });
 
+    it('supports Buffer input', function(done) {
+      fs.copyFile(
+        Buffer.from('path/to/src.txt'),
+        Buffer.from('empty/dest.txt'),
+        function(err) {
+          assert.isTrue(!err);
+          assert.isTrue(fs.existsSync('empty/dest.txt'));
+          assert.equal(
+            String(fs.readFileSync('empty/dest.txt')),
+            'file content'
+          );
+          done();
+        }
+      );
+    });
+
     withPromise.it('promise copies a file to an empty directory', function(
       done
     ) {

test/lib/fs.link-symlink.spec.js

@@ -31,6 +31,21 @@ describe('fs.link(srcpath, dstpath, callback)', function() {
     });
   });
 
+  it('supports Buffer input', function(done) {
+    assert.equal(fs.statSync('file.txt').nlink, 1);
+
+    fs.link(Buffer.from('file.txt'), Buffer.from('link.txt'), function(err) {
+      if (err) {
+        return done(err);
+      }
+      assert.isTrue(fs.statSync('link.txt').isFile());
+      assert.equal(fs.statSync('link.txt').nlink, 2);
+      assert.equal(fs.statSync('file.txt').nlink, 2);
+      assert.equal(String(fs.readFileSync('link.txt')), 'content');
+      done();
+    });
+  });
+
   withPromise.it('promise creates a link to a file', function(done) {
     assert.equal(fs.statSync('file.txt').nlink, 1);
 
@@ -173,6 +188,21 @@ describe('fs.symlink(srcpath, dstpath, [type], callback)', function() {
     });
   });
 
+  it('supports Buffer input', function(done) {
+    fs.symlink(
+      Buffer.from('../file.txt'),
+      Buffer.from('dir/link.txt'),
+      function(err) {
+        if (err) {
+          return done(err);
+        }
+        assert.isTrue(fs.statSync('dir/link.txt').isFile());
+        assert.equal(String(fs.readFileSync('dir/link.txt')), 'content');
+        done();
+      }
+    );
+  });
+
   withPromise.it('promise creates a symbolic link to a file', function(done) {
     fs.promises.symlink('../file.txt', 'dir/link.txt').then(function() {
       assert.isTrue(fs.statSync('dir/link.txt').isFile());

test/lib/fs.lstat.spec.js

@@ -34,6 +34,18 @@ describe('fs.lstat(path, callback)', function() {
     });
   });
 
+  it('suports Buffer input', function(done) {
+    fs.lstat(Buffer.from('link'), function(err, stats) {
+      if (err) {
+        return done(err);
+      }
+      assert.isTrue(stats.isSymbolicLink());
+      assert.isFalse(stats.isFile());
+      assert.equal(stats.mtime.getTime(), 2);
+      done();
+    });
+  });
+
   withPromise.it('promise stats a symbolic link', function(done) {
     fs.promises.lstat('link').then(function(stats) {
       assert.isTrue(stats.isSymbolicLink());

test/lib/fs.mkdir.spec.js

@@ -35,6 +35,17 @@ describe('fs.mkdir(path, [mode], callback)', function() {
     });
   });
 
+  it('supports Buffer input', function(done) {
+    fs.mkdir(Buffer.from('parent/dir'), function(err) {
+      if (err) {
+        return done(err);
+      }
+      const stats = fs.statSync('parent/dir');
+      assert.isTrue(stats.isDirectory());
+      done();
+    });
+  });
+
   withPromise.it('promise creates a new directory', function(done) {
     fs.promises.mkdir('parent/dir').then(function() {
       const stats = fs.statSync('parent/dir');

test/lib/fs.open-close.spec.js

@@ -34,6 +34,16 @@ describe('fs.open(path, flags, [mode], callback)', function() {
     });
   });
 
+  it('supports Buffer input', function(done) {
+    fs.open(Buffer.from('nested/sub/dir/one.txt'), 'r', function(err, fd) {
+      if (err) {
+        return done(err);
+      }
+      assert.isNumber(fd);
+      done();
+    });
+  });
+
   withPromise.it('promise opens an existing file for reading (r)', function(
     done
   ) {

test/lib/fs.readdir.spec.js

@@ -35,6 +35,15 @@ describe('fs.readdir(path, callback)', function() {
     });
   });
 
+  it('supports Buffer input', function(done) {
+    fs.readdir(Buffer.from(path.join('path', 'to')), function(err, items) {
+      assert.isNull(err);
+      assert.isArray(items);
+      assert.deepEqual(items, ['file.txt']);
+      done();
+    });
+  });
+
   withPromise.it('promise lists directory contents', function(done) {
     fs.promises.readdir(path.join('path', 'to')).then(function(items) {
       assert.isArray(items);

test/lib/fs.readlink.spec.js

@@ -26,6 +26,16 @@ describe('fs.readlink(path, callback)', function() {
     });
   });
 
+  it('supports Buffer input', function(done) {
+    fs.readlink(Buffer.from('link'), function(err, srcPath) {
+      if (err) {
+        return done(err);
+      }
+      assert.equal(srcPath, './file.txt');
+      done();
+    });
+  });
+
   withPromise.it('promise reads a symbolic link', function(done) {
     fs.promises.readlink('link').then(function(srcPath) {
       assert.equal(srcPath, './file.txt');

test/lib/fs.rename.spec.js

@@ -32,6 +32,19 @@ describe('fs.rename(oldPath, newPath, callback)', function() {
     });
   });
 
+  it('supports Buffer input', function(done) {
+    fs.rename(
+      Buffer.from('path/to/a.bin'),
+      Buffer.from('path/to/b.bin'),
+      function(err) {
+        assert.isTrue(!err);
+        assert.isFalse(fs.existsSync('path/to/a.bin'));
+        assert.isTrue(fs.existsSync('path/to/b.bin'));
+        done();
+      }
+    );
+  });
+
   withPromise.it('promise allows files to be renamed', function(done) {
     fs.promises.rename('path/to/a.bin', 'path/to/b.bin').then(function() {
       assert.isFalse(fs.existsSync('path/to/a.bin'));