Handle error when attempting simultaneous PASV requests. Fixes #90.

Author: sergi

lib/jsftp.js

@@ -112,19 +112,15 @@ Ftp.prototype.reemit = function(event) {
 };
 
 Ftp.prototype._createSocket = function(port, host, firstAction) {
-  if (this.socket && this.socket.destroy) this.socket.destroy();
+  if (this.socket && this.socket.destroy) {
+    this.socket.destroy();
+  }
 
   this.authenticated = false;
   var socket = Net.createConnection(port, host, firstAction || NOOP);
   socket.on('connect', this.reemit('connect'));
   socket.on('timeout', this.reemit('timeout'));
 
-  this._createStreams(socket);
-
-  return socket;
-};
-
-Ftp.prototype._createStreams = function(socket) {
   this.pipeline = es.pipeline(socket, this.resParser);
 
   var self = this;
@@ -133,6 +129,8 @@ Ftp.prototype._createStreams = function(socket) {
     self.parseResponse.call(self, data);
   });
   this.pipeline.on('error', this.reemit('error'));
+
+  return socket;
 };
 
 Ftp.prototype.parseResponse = function(data) {
@@ -198,31 +196,28 @@ Ftp.prototype.nextCmd = function() {
  * @param {function} callback
  */
 Ftp.prototype.execute = function(action, callback) {
-  if (!callback) callback = NOOP;
-
   if (this.socket && this.socket.writable) {
-    return this.runCommand(action, callback);
+    return this.runCommand(action, callback || NOOP);
   }
 
   var self = this;
   this.authenticated = false;
   this.socket = this._createSocket(this.port, this.host, function() {
-    self.runCommand(action, callback);
+    self.runCommand(action, callback || NOOP);
   });
 };
 
 Ftp.prototype.runCommand = function(action, callback) {
-  var self = this;
-
-  function executeCmd() {
-    self.cmdBuffer_.push([action, callback]);
-    self.nextCmd();
-  }
+  var executeCmd = (function _executeCmd() {
+    this.cmdBuffer_.push([action, callback]);
+    this.nextCmd();
+  }).bind(this);
 
-  if (self.authenticated || /feat|syst|user|pass/.test(action)) {
+  if (this.authenticated || /feat|syst|user|pass/.test(action)) {
     return executeCmd();
   }
 
+  var self = this;
   this.getFeatures(function() {
     self.auth(self.user, self.pass, executeCmd);
   });
@@ -425,6 +420,11 @@ Ftp.prototype.get = function(remotePath, localPath, callback) {
         return callback(err);
       }
 
+      if (!socket) {
+        return callback(new Error(
+          'An unknown error occurred when trying to retrieve PASV socket'));
+      }
+
       var writeStream = fs.createWriteStream(localPath);
       writeStream.on('error', callback);
 
@@ -584,6 +584,12 @@ Ftp.prototype.getPasvSocket = function(callback) {
 
       var socket = Net.createConnection(res.port, res.host);
       socket.setTimeout(timeout || TIMEOUT);
+      socket.on('error', function(err) {
+        if (err.code === 'ECONNREFUSED') {
+          err.msg = 'Probably trying a PASV operation while one is in progress';
+        }
+        callback(err);
+      });
       callback(null, socket);
     });
   });
@@ -635,7 +641,7 @@ Ftp.prototype.ls = function(filePath, callback) {
       // and we set the variable `useList` to true, to avoid extra round
       // trips to the server to check.
       if ((err && (err.code === 502 || err.code === 500)) ||
-        (self.system && self.system.indexOf('hummingbird') > -1))
+          (self.system && self.system.indexOf('hummingbird') > -1))
       // Not sure if the 'hummingbird' system check ^^^ is still
       // necessary. If they support any standards, the 500 error
       // should have us covered. Let's leave it for now.

package.json

@@ -1,7 +1,7 @@
 {
   "name": "jsftp",
   "id": "jsftp",
-  "version": "1.3.6",
+  "version": "1.3.7",
   "description": "A sane FTP client implementation for NodeJS",
   "keywords": [ "ftp", "protocol", "files", "server", "client", "async" ],
   "author": "Sergi Mansilla <[email protected]> (http://sergimansilla.com)",

test/jsftp_test.js

@@ -20,7 +20,7 @@ var rimraf = require("rimraf");
 
 var concat = function(bufs) {
   var buffer, length = 0,
-    index = 0;
+  index = 0;
 
   if (!Array.isArray(bufs))
     bufs = Array.prototype.slice.call(arguments);
@@ -83,6 +83,7 @@ describe("jsftp test suite", function() {
     rimraf(getLocalPath(''), function() {
       Fs.mkdirSync(getLocalPath(''));
       Fs.writeFileSync(getLocalPath('testfile.txt'), "test");
+      Fs.writeFileSync(getLocalPath('testfile2.txt'), "test2");
 
       if (FTPCredentials.host === "localhost") {
         server = new ftpServer();
@@ -226,7 +227,7 @@ describe("jsftp test suite", function() {
         assert.equal(err.code, 530);
         assert.equal(data, null);
         next();
-    });
+      });
   });
 
   it("test invalid password", function(next) {
@@ -238,7 +239,7 @@ describe("jsftp test suite", function() {
         assert.equal(err.code, 530);
         assert.equal(data, null);
         next();
-    });
+      });
   });
 
   it("test getFeatures", function(next) {
@@ -767,4 +768,30 @@ describe("jsftp test suite", function() {
       next();
     });
   });
+
+  it("Test handling error on simultaneous PASV requests {#90}", function(next) {
+    var file1 = remoteCWD + "/testfile.txt";
+    var file2 = remoteCWD + "/testfile2.txt";
+
+    var counter = 0;
+    var args = [];
+    function onDone() {
+      counter += 1;
+      if (counter === 2) {
+        assert.ok(args.some(function(arg) {
+          return arg && arg.code === 'ECONNREFUSED' && arg.msg ===
+            'Probably trying a PASV operation while one is in progress';
+        }));
+        next();
+      }
+    }
+
+    function pushit() {
+      args.push(arguments[0]);
+      onDone();
+    }
+
+    ftp.get(file1, pushit);
+    ftp.get(file2, pushit);
+  });
 });