basic-ftp - npm Package Compare versions

Comparing version 2.14.4 to 2.15.0

CHANGELOG.md

 # Changelog
 
+## 2.15.0
+
+- Changed: Timeout on control socket is only considered during active task, not when idle. This also fixes #27, as well as #26.
+
 ## 2.14.4
@@ -4,0 +8,0 @@

lib/ftp.js

@@ -749,7 +749,7 @@ "use strict";
                 // of timeouts again once data transfer is complete or failed.
-                ftp.suspendControlTimeout(true);
+                ftp.enableControlTimeout(false);
                 progress.start(ftp.dataSocket, remoteFilename, "upload");
                 readableStream.pipe(ftp.dataSocket).once("finish", () => {
                     ftp.dataSocket.destroy(); // Explicitly close/destroy the socket to signal the end.
-                    ftp.suspendControlTimeout(false);                    
+                    ftp.enableControlTimeout(true);                    
                     progress.updateAndStop();
@@ -764,3 +764,3 @@ resolver.confirm(task);
         else if (res.code >= 400 || res.error) {
-            ftp.suspendControlTimeout(false);            
+            ftp.enableControlTimeout(true);            
             progress.updateAndStop();
@@ -791,3 +791,3 @@ resolver.reject(task, res);
             // Let the data connection be in charge of tracking timeouts during transfer.
-            ftp.suspendControlTimeout(true);
+            ftp.enableControlTimeout(false);
             progress.start(ftp.dataSocket, remoteFilename, "download");
@@ -799,3 +799,3 @@ // Confirm the transfer as soon as the data socket transmission ended.
             conditionOrEvent(ftp.dataSocket.destroyed, ftp.dataSocket, "end", () => {
-                ftp.suspendControlTimeout(false);
+                ftp.enableControlTimeout(true);
                 progress.updateAndStop();
@@ -812,3 +812,3 @@ resolver.confirm(task);
         else if (res.code >= 400 || res.error) {
-            ftp.suspendControlTimeout(false);
+            ftp.enableControlTimeout(true);
             progress.updateAndStop();
@@ -815,0 +815,0 @@ resolver.reject(task, res);

lib/FtpContext.js

@@ -93,4 +93,3 @@ "use strict";
         this.log("Closing connections.");
-        this._handler = undefined;
-        this._task = undefined;
+        this._stopTrackingTask();
         this._partialResponse = "";
@@ -122,9 +121,4 @@ this._closeSocket(this._socket);
         if (socket) {
+            // Don't set a timeout yet. Timeout for control sockets is only active during a task, see handle() below.
             socket.setKeepAlive(true);
-            // Timeout on control socket should be active as soon as the control socket is really
-            // connected to something. Otherwise we run into the following bug: 
-            // When we close the context, we also create a new socket instance just in case the user 
-            // wants to reconnect. If this socket throws a timeout, the context will catch it, close itself
-            // again, create a new socket which will in turn provoke the next timeout. This will loop forever.
-            socket.once("connect", () => this.suspendControlTimeout(false));
             socket.on("data", data => this._onControlSocketData(data));
@@ -159,2 +153,36 @@ this._setupErrorHandlers(socket, "control");
     /**
+     * Send an FTP command without waiting for or handling the result.
+     * 
+     * @param {string} command
+     */
+    send(command) {
+        // Don't log passwords.
+        const message = command.startsWith("PASS") ? "> PASS ###" : `> ${command}`;
+        this.log(message);
+        this._socket.write(command + "\r\n", this.encoding);
+    }
+
+    /**
+     * Log message if set to be verbose.
+     * 
+     * @param {string} message 
+     */
+    log(message) {
+        if (this.verbose) {
+            console.log(message);
+        }
+    }
+
+    /**
+     * Enable timeout on the control socket connection. Disabling it can be useful if
+     * a timeout should be caught by the current data connection instead of the 
+     * control connection that sits idle during transfers anyway.
+     * 
+     * @param {boolean} enabled 
+     */
+    enableControlTimeout(enabled) {
+        this.socket.setTimeout(enabled ? this._timeout : 0);
+    }
+
+    /**
      * Return true if the control socket is using TLS. This does not mean that a session
@@ -183,2 +211,5 @@ * has already been negotiated.
         }
+        // Only track control socket timeout during the lifecycle of a task associated with a handler.
+        // That way we avoid timeouts on idle sockets, a behaviour that is not expected by most users.
+        this.enableControlTimeout(true);
         return new Promise((resolvePromise, rejectPromise) => {
@@ -190,7 +221,7 @@ this._handler = handler;
                 resolve: (...args) => {
-                    this._handler = undefined;
+                    this._stopTrackingTask();
                     resolvePromise(...args);
                 },
                 reject: (...args) => {
-                    this._handler = undefined;
+                    this._stopTrackingTask();
                     rejectPromise(...args);
@@ -206,36 +237,12 @@ }
     /**
-     * Send an FTP command without waiting for or handling the result.
-     * 
-     * @param {string} command
+     * Removes reference to current task and handler. This won't resolve or reject the task.
      */
-    send(command) {
-        // Don't log passwords.
-        const message = command.startsWith("PASS") ? "> PASS ###" : `> ${command}`;
-        this.log(message);
-        this._socket.write(command + "\r\n", this.encoding);
+    _stopTrackingTask() {
+        // Disable timeout on control socket if there is no task active.
+        this.enableControlTimeout(false);
+        this._task = undefined;
+        this._handler = undefined;
     }
 
     /**
-     * Log message if set to be verbose.
-     * 
-     * @param {string} message 
-     */
-    log(message) {
-        if (this.verbose) {
-            console.log(message);
-        }
-    }
-
-    /**
-     * Suspend timeout on the control socket connection. This can be useful if
-     * a timeout should be caught by the current data connection instead of the 
-     * control connection that sits idle during transfers anyway.
-     * 
-     * @param {boolean} suspended 
-     */
-    suspendControlTimeout(suspended) {
-        this.socket.setTimeout(suspended ? 0 : this._timeout);
-    }
-
-    /**
      * Handle incoming data on the control socket.
@@ -242,0 +249,0 @@ *

package.json

 {
   "name": "basic-ftp",
-  "version": "2.14.4",
+  "version": "2.15.0",
   "description": "FTP client for Node.js with support for explicit FTPS over TLS.",
@@ -30,5 +30,5 @@ "main": "./lib/ftp",
   "devDependencies": {
-    "eslint": "5.7.0",
+    "eslint": "5.8.0",
     "mocha": "5.2.0"
   }
 }

No alert changes

Improved metrics

Total package byte prevSize

113179

increased by0.19%

Lines of code

2468

increased by0.24%

No dependency changes