fix: explicitly promisify fs methods

`promisifyAll` will throw if a method with the suffix `Async` already
exists.

Fixes: https://github.com/trs/ftp-srv/pull/159

package.json

@@ -27,7 +27,7 @@
   "scripts": {
     "pre-release": "npm run verify",
     "semantic-release": "semantic-release",
-    "test": "mocha **/*.spec.js --ui bdd --bail",
+    "test": "mocha test/**/*.spec.js test/*.spec.js --ui bdd",
     "verify": "eslint src/**/*.js test/**/*.js bin/**/*.js"
   },
   "release": {
@@ -66,14 +66,14 @@
     "bluebird": "^3.5.1",
     "bunyan": "^1.8.12",
     "ip": "^1.1.5",
-    "lodash": "^4.17.10",
+    "lodash": "^4.17.15",
     "moment": "^2.22.1",
     "uuid": "^3.2.1",
     "yargs": "^11.0.0"
   },
   "devDependencies": {
-    "@commitlint/cli": "^7.5.2",
-    "@commitlint/config-conventional": "^7.5.0",
+    "@commitlint/cli": "^8.1.0",
+    "@commitlint/config-conventional": "^8.1.0",
     "@icetee/ftp": "^1.0.2",
     "chai": "^4.2.0",
     "condition-circle": "^2.0.2",
@@ -82,7 +82,7 @@
     "lint-staged": "^8.1.4",
     "mocha": "^5.2.0",
     "rimraf": "^2.6.1",
-    "semantic-release": "^15.10.6",
+    "semantic-release": "^15.13.24",
     "sinon": "^2.3.5"
   },
   "engines": {

src/commands/registration/list.js

@@ -36,6 +36,7 @@ module.exports = {
     .tap(() => this.reply(150))
     .then((fileList) => {
       if (fileList.length) return this.reply({}, ...fileList);
+      return this.reply({socket: this.connector.socket, useEmptyMessage: true});
     })
     .tap(() => this.reply(226))
     .catch(Promise.TimeoutError, (err) => {

src/commands/registration/opts.js

@@ -33,7 +33,6 @@ function utf8([setting] = []) {
   if (!encoding) return this.reply(501, 'Unknown setting for option');
 
   this.encoding = encoding;
-  if (this.transferType !== 'binary') this.transferType = this.encoding;
 
   return this.reply(200, `UTF8 encoding ${_.toLower(setting)}`);
 }

src/commands/registration/pasv.js

@@ -1,4 +1,5 @@
 const PassiveConnector = require('../../connector/passive');
+const {isLocalIP} = require('../../helpers/is-local');
 
 module.exports = {
   directive: 'PASV',
@@ -10,7 +11,11 @@ module.exports = {
     this.connector = new PassiveConnector(this);
     return this.connector.setupServer()
     .then((server) => {
-      const address = this.server.options.pasv_url;
+      let address = this.server.options.pasv_url;
+      // Allow connecting from local
+      if (isLocalIP(this.ip)) {
+        address = this.ip;
+      }
       const {port} = server.address();
       const host = address.replace(/\./g, ',');
       const portByte1 = port / 256 | 0;

src/commands/registration/retr.js

@@ -28,7 +28,7 @@ module.exports = {
         stream.on('data', (data) => {
           if (stream) stream.pause();
           if (this.connector.socket) {
-            this.connector.socket.write(data, this.transferType, () => stream && stream.resume());
+            this.connector.socket.write(data, () => stream && stream.resume());
           }
         });
         stream.once('end', () => resolve());

src/commands/registration/stor.js

@@ -21,11 +21,14 @@ module.exports = {
       const serverPath = stream.path || fileName;
 
       const destroyConnection = (connection, reject) => (err) => {
-        if (connection) {
-          if (connection.writable) connection.end();
-          connection.destroy(err);
+        try {
+          if (connection) {
+            if (connection.writable) connection.end();
+            connection.destroy(err);
+          }
+        } finally {
+          reject(err);
         }
-        reject(err);
       };
 
       const streamPromise = new Promise((resolve, reject) => {
@@ -37,7 +40,7 @@ module.exports = {
         this.connector.socket.on('data', (data) => {
           if (this.connector.socket) this.connector.socket.pause();
           if (stream && stream.writable) {
-            stream.write(data, this.transferType, () => this.connector.socket && this.connector.socket.resume());
+            stream.write(data, () => this.connector.socket && this.connector.socket.resume());
           }
         });
         this.connector.socket.once('end', () => {

src/connection.js

@@ -104,15 +104,21 @@ class FtpConnection extends EventEmitter {
           else if (typeof letter === 'string') letter = {message: letter}; // allow passing in message as first param
 
           if (!letter.socket) letter.socket = options.socket ? options.socket : this.commandSocket;
-          if (!letter.message) letter.message = DEFAULT_MESSAGE[options.code] || 'No information';
-          if (!letter.encoding) letter.encoding = this.encoding;
+          if (!options.useEmptyMessage) {
+            if (!letter.message) letter.message = DEFAULT_MESSAGE[options.code] || 'No information';
+            if (!letter.encoding) letter.encoding = this.encoding;
+          }
           return Promise.resolve(letter.message) // allow passing in a promise as a message
           .then((message) => {
-            const seperator = !options.hasOwnProperty('eol') ?
-              letters.length - 1 === index ? ' ' : '-' :
-              options.eol ? ' ' : '-';
-            message = !letter.raw ? _.compact([letter.code || options.code, message]).join(seperator) : message;
-            letter.message = message;
+            if (!options.useEmptyMessage) {
+              const seperator = !options.hasOwnProperty('eol') ?
+                letters.length - 1 === index ? ' ' : '-' :
+                options.eol ? ' ' : '-';
+              message = !letter.raw ? _.compact([letter.code || options.code, message]).join(seperator) : message;
+              letter.message = message;
+            } else {
+              letter.message = '';
+            }
             return letter;
           });
         });

src/connector/active.js

@@ -28,7 +28,6 @@ class Active extends Connector {
     return closeExistingServer()
     .then(() => {
       this.dataSocket = new Socket();
-      this.dataSocket.setEncoding(this.connection.transferType);
       this.dataSocket.on('error', (err) => this.server && this.server.emit('client-error', {connection: this.connection, context: 'dataSocket', error: err}));
       this.dataSocket.connect({host, port, family}, () => {
         this.dataSocket.pause();

src/connector/passive.js

@@ -6,6 +6,8 @@ const Promise = require('bluebird');
 const Connector = require('./base');
 const errors = require('../errors');
 
+const CONNECT_TIMEOUT = 30 * 1000;
+
 class Passive extends Connector {
   constructor(connection) {
     super(connection);
@@ -30,6 +32,9 @@ class Passive extends Connector {
     this.closeServer();
     return this.server.getNextPasvPort()
     .then((port) => {
+      this.dataSocket = null;
+      let idleServerTimeout;
+
       const connectionHandler = (socket) => {
         if (!ip.isEqual(this.connection.commandSocket.remoteAddress, socket.remoteAddress)) {
           this.log.error({
@@ -41,19 +46,19 @@ class Passive extends Connector {
           return this.connection.reply(550, 'Remote addresses do not match')
           .finally(() => this.connection.close());
         }
+        clearTimeout(idleServerTimeout);
+
         this.log.trace({port, remoteAddress: socket.remoteAddress}, 'Passive connection fulfilled.');
 
         this.dataSocket = socket;
-        this.dataSocket.setEncoding(this.connection.transferType);
         this.dataSocket.on('error', (err) => this.server && this.server.emit('client-error', {connection: this.connection, context: 'dataSocket', error: err}));
+        this.dataSocket.once('close', () => this.closeServer());
 
         if (!this.connection.secure) {
           this.dataSocket.connected = true;
         }
       };
 
-      this.dataSocket = null;
-
       const serverOptions = Object.assign({}, this.connection.secure ? this.server.options.tls : {}, {pauseOnConnect: true});
       this.dataServer = (this.connection.secure ? tls : net).createServer(serverOptions, connectionHandler);
       this.dataServer.maxConnections = 1;
@@ -74,6 +79,8 @@ class Passive extends Connector {
         this.dataServer.listen(port, this.server.url.hostname, (err) => {
           if (err) reject(err);
           else {
+            idleServerTimeout = setTimeout(() => this.closeServer(), CONNECT_TIMEOUT);
+
             this.log.debug({port}, 'Passive connection listening');
             resolve(this.dataServer);
           }

src/fs.js

@@ -2,13 +2,14 @@ const _ = require('lodash');
 const nodePath = require('path');
 const uuid = require('uuid');
 const Promise = require('bluebird');
-const fs = Promise.promisifyAll(require('fs'));
+const {createReadStream, createWriteStream, constants} = require('fs');
+const fsAsync = require('./helpers/fs-async');
 const errors = require('./errors');
 
 class FileSystem {
   constructor(connection, {root, cwd} = {}) {
     this.connection = connection;
-    this.cwd = cwd ? nodePath.join(nodePath.sep, cwd) : nodePath.sep;
+    this.cwd = nodePath.normalize(cwd ? nodePath.join(nodePath.sep, cwd) : nodePath.sep);
     this._root = nodePath.resolve(root || process.cwd());
   }
 
@@ -27,8 +28,8 @@ class FileSystem {
     })();
 
     const fsPath = (() => {
-      const resolvedPath = nodePath.resolve(this.root, `.${nodePath.sep}${clientPath}`);
-      return nodePath.join(resolvedPath);
+      const resolvedPath = nodePath.join(this.root, clientPath);
+      return nodePath.resolve(nodePath.normalize(nodePath.join(resolvedPath)));
     })();
 
     return {
@@ -43,19 +44,19 @@ class FileSystem {
 
   get(fileName) {
     const {fsPath} = this._resolvePath(fileName);
-    return fs.statAsync(fsPath)
+    return fsAsync.stat(fsPath)
     .then((stat) => _.set(stat, 'name', fileName));
   }
 
   list(path = '.') {
     const {fsPath} = this._resolvePath(path);
-    return fs.readdirAsync(fsPath)
+    return fsAsync.readdir(fsPath)
     .then((fileNames) => {
       return Promise.map(fileNames, (fileName) => {
         const filePath = nodePath.join(fsPath, fileName);
-        return fs.accessAsync(filePath, fs.constants.F_OK)
+        return fsAsync.access(filePath, constants.F_OK)
         .then(() => {
-          return fs.statAsync(filePath)
+          return fsAsync.stat(filePath)
           .then((stat) => _.set(stat, 'name', fileName));
         })
         .catch(() => null);
@@ -66,7 +67,7 @@ class FileSystem {
 
   chdir(path = '.') {
     const {fsPath, clientPath} = this._resolvePath(path);
-    return fs.statAsync(fsPath)
+    return fsAsync.stat(fsPath)
     .tap((stat) => {
       if (!stat.isDirectory()) throw new errors.FileSystemError('Not a valid directory');
     })
@@ -78,8 +79,8 @@ class FileSystem {
 
   write(fileName, {append = false, start = undefined} = {}) {
     const {fsPath, clientPath} = this._resolvePath(fileName);
-    const stream = fs.createWriteStream(fsPath, {flags: !append ? 'w+' : 'a+', start});
-    stream.once('error', () => fs.unlinkAsync(fsPath));
+    const stream = createWriteStream(fsPath, {flags: !append ? 'w+' : 'a+', start});
+    stream.once('error', () => fsAsync.unlink(fsPath));
     stream.once('close', () => stream.end());
     return {
       stream,
@@ -89,12 +90,12 @@ class FileSystem {
 
   read(fileName, {start = undefined} = {}) {
     const {fsPath, clientPath} = this._resolvePath(fileName);
-    return fs.statAsync(fsPath)
+    return fsAsync.stat(fsPath)
     .tap((stat) => {
       if (stat.isDirectory()) throw new errors.FileSystemError('Cannot read a directory');
     })
     .then(() => {
-      const stream = fs.createReadStream(fsPath, {flags: 'r', start});
+      const stream = createReadStream(fsPath, {flags: 'r', start});
       return {
         stream,
         clientPath
@@ -104,28 +105,28 @@ class FileSystem {
 
   delete(path) {
     const {fsPath} = this._resolvePath(path);
-    return fs.statAsync(fsPath)
+    return fsAsync.stat(fsPath)
     .then((stat) => {
-      if (stat.isDirectory()) return fs.rmdirAsync(fsPath);
-      else return fs.unlinkAsync(fsPath);
+      if (stat.isDirectory()) return fsAsync.rmdir(fsPath);
+      else return fsAsync.unlink(fsPath);
     });
   }
 
   mkdir(path) {
     const {fsPath} = this._resolvePath(path);
-    return fs.mkdirAsync(fsPath)
+    return fsAsync.mkdir(fsPath)
     .then(() => fsPath);
   }
 
   rename(from, to) {
     const {fsPath: fromPath} = this._resolvePath(from);
     const {fsPath: toPath} = this._resolvePath(to);
-    return fs.renameAsync(fromPath, toPath);
+    return fsAsync.rename(fromPath, toPath);
   }
 
   chmod(path, mode) {
     const {fsPath} = this._resolvePath(path);
-    return fs.chmodAsync(fsPath, mode);
+    return fsAsync.chmod(fsPath, mode);
   }
 
   getUniqueName() {

src/helpers/find-port.js

@@ -16,10 +16,11 @@ function* portNumberGenerator(min, max = MAX_PORT) {
 
 function getNextPortFactory(host, portMin, portMax, maxAttempts = MAX_PORT_CHECK_ATTEMPT) {
   const nextPortNumber = portNumberGenerator(portMin, portMax);
-  const portCheckServer = net.createServer();
-  portCheckServer.maxConnections = 0;
 
   return () => new Promise((resolve, reject) => {
+    const portCheckServer = net.createServer();
+    portCheckServer.maxConnections = 0;
+
     let attemptCount = 0;
     const tryGetPort = () => {
       attemptCount++;
@@ -39,6 +40,7 @@ function getNextPortFactory(host, portMin, portMax, maxAttempts = MAX_PORT_CHECK
         }
       });
       portCheckServer.once('listening', () => {
+        portCheckServer.removeAllListeners();
         portCheckServer.close(() => resolve(port));
       });
 

src/helpers/fs-async.js

@@ -0,0 +1,18 @@
+const fs = require('fs');
+const {promisify} = require('bluebird');
+
+const methods = [
+  'stat',
+  'readdir',
+  'access',
+  'unlink',
+  'rmdir',
+  'mkdir',
+  'rename',
+  'chmod'
+];
+
+module.exports = methods.reduce((obj, method) => {
+  obj[method] = promisify(fs[method]);
+  return obj;
+}, {});

src/helpers/is-local.js

@@ -0,0 +1,3 @@
+module.exports.isLocalIP = function(ip) {
+  return ip === '127.0.0.1' || ip == '::1';
+}

test/helpers/find-port.spec.js

@@ -2,9 +2,22 @@
 const {expect} = require('chai');
 const net = require('net');
 
-const {getNextPortFactory} = require('../../src/helpers/find-port');
+const {getNextPortFactory, portNumberGenerator} = require('../../src/helpers/find-port');
+
+describe('getNextPortFactory', function () {
+  describe('portNumberGenerator', () => {
+    it('loops through given set of numbers', () => {
+      const nextNumber = portNumberGenerator(1, 5);
+      expect(nextNumber.next().value).to.equal(1);
+      expect(nextNumber.next().value).to.equal(2);
+      expect(nextNumber.next().value).to.equal(3);
+      expect(nextNumber.next().value).to.equal(4);
+      expect(nextNumber.next().value).to.equal(5);
+      expect(nextNumber.next().value).to.equal(1);
+      expect(nextNumber.next().value).to.equal(2);
+    });
+  });
 
-describe('helpers // find-port', function () {
   describe('keeps trying new ports', () => {
     let getNextPort;
     let serverAlreadyRunning;
@@ -26,4 +39,17 @@ describe('helpers // find-port', function () {
       });
     });
   });
+
+  it('finds ports concurrently', () => {
+    const portStart = 10000;
+    const getCount = 100;
+
+    const getNextPort = getNextPortFactory('::', portStart);
+    const portFinders = new Array(getCount).fill().map(() => getNextPort());
+    return Promise.all(portFinders)
+    .then((ports) => {
+      expect(ports.length).to.equal(getCount);
+      expect(ports).to.eql(new Array(getCount).fill().map((v, i) => i + portStart));
+    });
+  });
 });

test/index.spec.js

@@ -187,9 +187,9 @@ describe('Integration', function () {
       });
 
       client.put(buffer, 'fail.txt', (err) => {
+        expect(err).to.exist;
         setImmediate(() => {
           const fileExists = fs.existsSync(fsPath);
-          expect(err).to.exist;
           expect(fileExists).to.equal(false);
           done();
         });
@@ -217,6 +217,24 @@ describe('Integration', function () {
       });
     });
 
+    it('STOR logo.png', (done) => {
+      const logo = `${__dirname}/../logo.png`;
+      const fsPath = `${clientDirectory}/${name}/logo.png`;
+
+      client.put(logo, 'logo.png', (err) => {
+        expect(err).to.not.exist;
+        setImmediate(() => {
+          expect(fs.existsSync(fsPath)).to.equal(true);
+
+          const logoContents = fs.readFileSync(logo);
+          const transferedContects = fs.readFileSync(fsPath);
+
+          expect(logoContents.equals(transferedContects));
+          done();
+        });
+      });
+    });
+
     it('APPE tést.txt', (done) => {
       const buffer = Buffer.from(', awesome!');
       const fsPath = `${clientDirectory}/${name}/tést.txt`;

test/start.js

@@ -5,7 +5,7 @@ const FtpServer = require('../src');
 const server = new FtpServer({
   log: bunyan.createLogger({name: 'test', level: 'trace'}),
   url: 'ftp://127.0.0.1:8880',
-  pasv_url: '127.0.0.1',
+  pasv_url: '192.168.1.1',
   pasv_min: 8881,
   greeting: ['Welcome', 'to', 'the', 'jungle!'],
   tls: {