From 1be5e7b5821fd9f6eda408c823a1f9a01adc4477 Mon Sep 17 00:00:00 2001
From: Sergi Mansilla <sergi.mansilla@gmail.com>
Date: Tue, 23 Dec 2014 12:53:43 +0100
Subject: [PATCH] Ensure that when using `get` the consumer always receives
 feedback regardless of whether the signal is an `end`, `close` or an `error`.
 Fixes #106.

---
 lib/jsftp.js | 66 +++++++++++++++++++++++++++++++++-------------------
 package.json |  2 +-
 2 files changed, 43 insertions(+), 25 deletions(-)

diff --git a/lib/jsftp.js b/lib/jsftp.js
index efd79a4..fbf204a 100644
--- a/lib/jsftp.js
+++ b/lib/jsftp.js
@@ -146,11 +146,14 @@ Ftp.prototype.parseResponse = function(data) {
   var next = this.cmdBuffer_[0][1];
   if (data.isMark) {
     // If we receive a Mark and it is not expected, we ignore that command
-    if (!next.expectsMark || next.expectsMark.marks.indexOf(data.code) === -1)
+    if (!next.expectsMark || next.expectsMark.marks.indexOf(data.code) === -1) {
       return;
+    }
+
     // We might have to ignore the command that comes after the mark.
-    if (next.expectsMark.ignore)
+    if (next.expectsMark.ignore) {
       this.ignoreCmdCode = next.expectsMark.ignore;
+    }
   }
 
   if (this.ignoreCmdCode && this.ignoreCmdCode === data.code) {
@@ -300,8 +303,9 @@ Ftp.prototype.auth = function(user, pass, callback) {
 
   function notifyAll(err, res) {
     var cb;
-    while (cb = self.pending.shift())
+    while (cb = self.pending.shift()) {
       cb(err, res);
+    }
   }
 
   if (this.authenticating) return;
@@ -410,26 +414,22 @@ Ftp.prototype.emitProgress = function(data) {
  * case the operation was not successful.
  *
  * @param {String} remotePath File to be retrieved from the FTP server
- * @param {String} localPath Local path where the new file will be created
- * @param {Function} callback Gets called on either success or failure
+ * @param {Function|String} localPath Local path where we create the new file
+ * @param {Function} [callback] Gets called on either success or failure
  */
 Ftp.prototype.get = function(remotePath, localPath, callback) {
   var self = this;
-  if (arguments.length === 2) {
-    callback = once(localPath || NOOP);
-    this.getGetSocket(remotePath, callback);
+  var finalCallback;
+
+  if (typeof localPath === 'function') {
+    finalCallback = once(localPath || NOOP);
   } else {
     callback = once(callback || NOOP);
-    this.getGetSocket(remotePath, function(err, socket) {
+    finalCallback = function(err, socket) {
       if (err) {
         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);
 
@@ -440,11 +440,20 @@ Ftp.prototype.get = function(remotePath, localPath, callback) {
           socket: this
         });
       });
+
+      // This ensures that any expected outcome is handled. There is no
+      // danger of the callback being executed several times, because it is
+      // wrapped in `once`.
+      socket.on('error', callback);
       socket.on('end', callback);
+      socket.on('close', callback);
+
       socket.pipe(writeStream);
       socket.resume();
-    });
+    };
   }
+
+  this.getGetSocket(remotePath, finalCallback);
 };
 
 /**
@@ -473,12 +482,19 @@ Ftp.prototype.getGetSocket = function(path, callback) {
     socket.pause();
 
     function cmdCallback(err, res) {
-      if (err) return callback(err);
+      if (err) {
+        return callback(err);
+      }
 
-      if (res.code === 125 || res.code === 150)
-        callback(null, socket);
-      else
-        callback(new Error('Unexpected command ' + res.text));
+      if (!socket) {
+        return callback(new Error('Error when retrieving PASV socket'));
+      }
+
+      if (res.code === 125 || res.code === 150) {
+        return callback(null, socket);
+      }
+
+      return callback(new Error('Unexpected command ' + res.text));
     }
 
     cmdCallback.expectsMark = {
@@ -547,7 +563,7 @@ Ftp.prototype.getPutSocket = function(path, callback, doneCallback) {
       callback(err);
       return doneCallback(err);
     }
-    return callback(err, _socket);
+    return callback(null, _socket);
   });
 
   var self = this;
@@ -563,15 +579,17 @@ Ftp.prototype.getPutSocket = function(path, callback, doneCallback) {
         socket.on('close', doneCallback);
         socket.on('error', doneCallback);
         self.pasvTimeout.call(self, socket, doneCallback);
-        _callback(null, socket);
-      } else {
-        return _callback(new Error('Unexpected command ' + res.text));
+        return _callback(null, socket);
       }
+
+      return _callback(new Error('Unexpected command ' + res.text));
     });
+
     putCallback.expectsMark = {
       marks: [125, 150],
       ignore: 226
     };
+
     self.execute('stor ' + path, putCallback);
   });
 };
diff --git a/package.json b/package.json
index b518ce8..fe01c00 100644
--- a/package.json
+++ b/package.json
@@ -1,7 +1,7 @@
 {
   "name": "jsftp",
   "id": "jsftp",
-  "version": "1.3.8",
+  "version": "1.3.9",
   "description": "A sane FTP client implementation for NodeJS",
   "keywords": [ "ftp", "protocol", "files", "server", "client", "async" ],
   "author": "Sergi Mansilla <sergi.mansilla@gmail.com> (http://sergimansilla.com)",