fix: correctly destroy socket on close (#208)

* chore: update owner references

* fix: correctly destroy socket on close

This fixes an issue where the client would never close, hanging the server and preventing shutdown

* fix: only set timeout if greater than 0

* fix: move dependency to top

* fix: notify of command that caused error

* fix: emit disconnect event on client close

README.md

@@ -1,5 +1,5 @@
 <p align="center">
-  <a href="https://github.com/trs/ftp-srv">
+  <a href="https://github.com/autovance/ftp-srv">
     <img alt="ftp-srv" src="logo.png" width="600px"  />
   </a>
 </p>
@@ -14,8 +14,8 @@
     <img alt="npm" src="https://img.shields.io/npm/dm/ftp-srv.svg?style=for-the-badge" />
   </a>
 
-  <a href="https://circleci.com/gh/trs/workflows/ftp-srv/tree/master">
-    <img alt="circleci" src="https://img.shields.io/circleci/project/github/trs/ftp-srv/master.svg?style=for-the-badge" />
+  <a href="https://circleci.com/gh/autovance/workflows/ftp-srv/tree/master">
+    <img alt="circleci" src="https://img.shields.io/circleci/project/github/autovance/ftp-srv/master.svg?style=for-the-badge" />
   </a>
 </p>
 

package.json

@@ -22,7 +22,7 @@
   "types": "./ftp-srv.d.ts",
   "repository": {
     "type": "git",
-    "url": "https://github.com/trs/ftp-srv"
+    "url": "https://github.com/autovance/ftp-srv"
   },
   "scripts": {
     "pre-release": "npm run verify",

src/commands/index.js

@@ -45,25 +45,25 @@ class FtpCommands {
     log.trace({command: logCommand}, 'Handle command');
 
     if (!REGISTRY.hasOwnProperty(command.directive)) {
-      return this.connection.reply(502, 'Command not allowed');
+      return this.connection.reply(502, `Command not allowed: ${command.directive}`);
     }
 
     if (_.includes(this.blacklist, command.directive)) {
-      return this.connection.reply(502, 'Command blacklisted');
+      return this.connection.reply(502, `Command blacklisted: ${command.directive}`);
     }
 
     if (this.whitelist.length > 0 && !_.includes(this.whitelist, command.directive)) {
-      return this.connection.reply(502, 'Command not whitelisted');
+      return this.connection.reply(502, `Command not whitelisted: ${command.directive}`);
     }
 
     const commandRegister = REGISTRY[command.directive];
     const commandFlags = _.get(commandRegister, 'flags', {});
     if (!commandFlags.no_auth && !this.connection.authenticated) {
-      return this.connection.reply(530, 'Command requires authentication');
+      return this.connection.reply(530, `Command requires authentication: ${command.directive}`);
     }
 
     if (!commandRegister.handler) {
-      return this.connection.reply(502, 'Handler not set on command');
+      return this.connection.reply(502, `Handler not set on command: ${command.directive}`);
     }
 
     const handler = commandRegister.handler.bind(this.connection);

src/commands/registration/feat.js

@@ -1,9 +1,9 @@
 const _ = require('lodash');
+const registry = require('../registry');
 
 module.exports = {
   directive: 'FEAT',
   handler: function () {
-    const registry = require('../registry');
     const features = Object.keys(registry)
       .reduce((feats, cmd) => {
         const feat = _.get(registry[cmd], 'flags.feat', null);

src/connection.js

@@ -32,7 +32,7 @@ class FtpConnection extends EventEmitter {
     this.commandSocket.on('data', this._handleData.bind(this));
     this.commandSocket.on('timeout', () => {
       this.log.trace('Client timeout');
-      this.close().catch((e) => this.log.trace(e, 'Client close error'));
+      this.close();
     });
     this.commandSocket.on('close', () => {
       if (this.connector) this.connector.end();
@@ -72,7 +72,7 @@ class FtpConnection extends EventEmitter {
   close(code = 421, message = 'Closing connection') {
     return Promise.resolve(code)
       .then((_code) => _code && this.reply(_code, message))
-      .then(() => this.commandSocket && this.commandSocket.end());
+      .finally(() => this.commandSocket && this.commandSocket.destroy());
   }
 
   login(username, password) {
@@ -136,7 +136,10 @@ class FtpConnection extends EventEmitter {
             }
             resolve();
           });
-        } else reject(new errors.SocketError('Socket not writable'));
+        } else {
+          this.log.trace({message: letter.message}, 'Could not write message');
+          reject(new errors.SocketError('Socket not writable'));
+        }
       });
     };
 

src/index.js

@@ -44,11 +44,12 @@ class FtpServer extends EventEmitter {
     this.options.timeout = isNaN(timeout) ? 0 : Number(timeout);
 
     const serverConnectionHandler = (socket) => {
-      socket.setTimeout(this.options.timeout);
+      this.options.timeout > 0 && socket.setTimeout(this.options.timeout);
       let connection = new Connection(this, {log: this.log, socket});
       this.connections[connection.id] = connection;
 
       socket.on('close', () => this.disconnectClient(connection.id));
+      socket.once('close', () => this.emit('disconnect', {connection, id: connection.id}));
 
       const greeting = this._greeting || [];
       const features = this._features || 'Ready';
@@ -119,7 +120,6 @@ class FtpServer extends EventEmitter {
     return new Promise((resolve) => {
       const client = this.connections[id];
       if (!client) return resolve();
-      this.emit('disconnect', {connection: client, id});
       delete this.connections[id];
       try {
         client.close(0);