basic-ftp - npm Package Compare versions

Comparing version 2.8.1 to 2.8.2

CHANGELOG.md

 # Changelog
 
+## 2.8.2
+
+- When downloading, handle incoming data before announcement from control socket arrives.
+- More tests for uploading and downloading data including directory listings.
+- Use download mechanism for directory listings as well.
+
 ## 2.8.1
@@ -4,0 +10,0 @@

lib/ftp.js

@@ -7,2 +7,3 @@ "use strict";
 const path = require("path");
+const EventEmitter = require("events");
 const promisify = require("util").promisify;
@@ -100,3 +101,4 @@ const defaultParseList = require("./parseList");
     /**
-     * Return true if TLS is enabled for the control socket.
+     * Return true if the control socket is using TLS. This does not mean that a session
+     * has already been negotiated.
      * 
@@ -454,3 +456,4 @@ * @returns {boolean}
         await this.prepareTransfer(this.ftp);
-        return download(this.ftp, writableStream, remoteFilename, startAt);
+        const command = startAt > 0 ? `REST ${startAt}` : `RETR ${remoteFilename}`;
+        return download(this.ftp, writableStream, command, remoteFilename);
     }
@@ -465,3 +468,6 @@
         await this.prepareTransfer(this.ftp);
-        return list(this.ftp, this.parseList);
+        const writable = new StringWriter(this.ftp.encoding);
+        await download(this.ftp, writable, "LIST"); 
+        this.ftp.log(writable.text);
+        return this.parseList(writable.text);
     }
@@ -582,2 +588,6 @@
     
+    /**
+     * Instantiate a TransferResolver
+     * @param {FTPContext} ftp 
+     */
     constructor(ftp) {
@@ -775,3 +785,3 @@ this.ftp = ftp;
 function parseIPv4PasvResponse(message) {
-    // From something like "227 Entering Passive Mode (192,168,3,200,10,229)",
+    // From something like "227 Entering Passive Mode (192,168,1,100,10,229)",
     // extract host and port.
@@ -789,32 +799,2 @@ const groups = message.match(/([-\d]+,[-\d]+,[-\d]+,[-\d]+),([-\d]+),([-\d]+)/);
 /**
- * List files and folders of current directory.`
- * 
- * @param {FTP} ftp
- * @param {(rawList: string) => FileInfo[]} parseList
- * @return {Promise<FileInfo[]>}
- */
-function list(ftp, parseList = defaultParseList) {
-    const resolver = new TransferResolver(ftp);    
-    let rawList = "";
-    return ftp.handle("LIST", (res, task) => {
-        if (res.code === 150 || res.code === 125) { // Ready to download
-            ftp.log(`Downloading list (${describeTLS(ftp.dataSocket)})`);
-            ftp.dataSocket.on("data", data => {
-                rawList += data.toString(ftp.encoding);
-            });
-            ftp.dataSocket.once("end", () => {
-                ftp.log(rawList);
-                resolver.resolve(task, parseList(rawList));
-            });
-        }
-        else if (positiveCompletion(res.code)) { // Transfer complete
-            resolver.confirm(task);
-        }
-        else if (res.code >= 400 || res.error) {
-            resolver.reject(task, res);
-        }
-    }); 
-}
-
-/**
  * Upload stream data as a file. For example:
@@ -834,4 +814,6 @@ *
         if (res.code === 150 || res.code === 125) { // Ready to upload
-            // The actual upload mechanism.
-            const execute = function() {
+            // If we are using TLS, we have to wait until the dataSocket issued
+            // 'secureConnect'. If this hasn't happened yet, getCipher() returns undefined.
+            const canUpload = ftp.hasTLS === false || ftp.dataSocket.getCipher() !== undefined;
+            conditionOrEvent(canUpload, ftp.dataSocket, "secureConnect", () => {
                 ftp.log(`Uploading (${describeTLS(ftp.dataSocket)})`);
@@ -842,13 +824,4 @@ readableStream.pipe(ftp.dataSocket).once("finish", () => {
                     resolver.confirm(task);             
-                });                
-            };
-            // If we are using TLS, we have to wait until the dataSocket issued
-            // 'secureConnect'. If this hasn't happened yet, getCipher() returns undefined.
-            const waitForSecureConnect = ftp.hasTLS && ftp.dataSocket.getCipher() === undefined;
-            if (waitForSecureConnect) {
-                ftp.dataSocket.once("secureConnect", execute);
-            }
-            else {
-                execute();                
-            }
+                });                                
+            });
         }
@@ -865,20 +838,23 @@ else if (positiveCompletion(res.code)) { // Transfer complete
 /**
- * Download a remote file as a stream. For example:
+ * Download data from the data connection. Used for downloading files and directory listings.
  * 
- * `download(ftp, fs.createWriteStream(localFilePath), remoteFilename)`
- * 
  * @param {FTP} ftp 
  * @param {stream.Writable} writableStream 
- * @param {string} remoteFilename 
- * @param {number} startAt 
+ * @param {string} command 
+ * @param {filename} [remoteFilename]
  * @returns {Promise<PositiveResponse>}
  */
-function download(ftp, writableStream, remoteFilename, startAt = 0) {
+function download(ftp, writableStream, command, remoteFilename = "") {
+    // It's possible that data transmission begins before the control socket
+    // receives the announcement. Start listening for data immediately.
+    ftp.dataSocket.pipe(writableStream);
     const resolver = new TransferResolver(ftp);
-    const command = startAt > 0 ? `REST ${startAt}` : `RETR ${remoteFilename}`;
     return ftp.handle(command, (res, task) => {
         if (res.code === 150 || res.code === 125) { // Ready to download
             ftp.log(`Downloading (${describeTLS(ftp.dataSocket)})`);
-            ftp.dataSocket.once("end", () => resolver.confirm(task));
-            ftp.dataSocket.pipe(writableStream);
+            // Confirm the transfer as soon as the data socket transmission ended.
+            // It's possible, though, that the data transmission is complete before
+            // the control socket receives the accouncement that it will begin.
+            // Check if the data socket is not already closed.
+            conditionOrEvent(ftp.dataSocket.destroyed, ftp.dataSocket, "end", () => resolver.confirm(task));
         }
@@ -898,2 +874,35 @@ else if (res.code === 350) { // Restarting at startAt.
 /**
+ * Calls a function immediately if a condition is met or subscribes to an event and calls
+ * it once the event is emitted.
+ * 
+ * @param {boolean} condition  The condition to test.
+ * @param {*} emitter  The emitter to use if the condition is not met.
+ * @param {string} eventName  The event to subscribe to if the condition is not met.
+ * @param {() => any} action  The function to call.
+ */
+function conditionOrEvent(condition, emitter, eventName, action) {
+    if (condition === true) {
+        action();
+    }
+    else {
+        emitter.once(eventName, () => action());
+    }
+}
+
+class StringWriter extends EventEmitter {
+    constructor(encoding) {
+        super();
+        this.encoding = encoding;
+        this.text = "";
+        this.write = this.end = this.append;
+    }
+
+    append(chunk) {
+        if (chunk) {
+            this.text += chunk.toString(this.encoding);
+        }
+    }
+}
+
+/**
  * Upload the contents of a local directory to the working directory. This will overwrite
@@ -900,0 +909,0 @@ * existing files and reuse existing directories.

package.json

 {
   "name": "basic-ftp",
-  "version": "2.8.1",
+  "version": "2.8.2",
   "description": "FTP client for Node.js with support for explicit FTPS over TLS.",
@@ -13,3 +13,3 @@ "main": "./lib/ftp",
     "type": "git",
-    "url" : "https://github.com/patrickjuchli/basic-ftp.git"
+    "url": "https://github.com/patrickjuchli/basic-ftp.git"
   },
@@ -16,0 +16,0 @@ "author": "Patrick Juchli <[e-mail]>",

test/ftpContextSpec.js

 const assert = require("assert");
 const FTPContext = require("../lib/ftp").FTPContext;
-const EventEmitter = require("events");
+const SocketMock = require("./SocketMock");
+const tls = require("tls");
+const net = require("net");
 
-class SocketMock extends EventEmitter {
-    constructor() {
-        super();
-        this.destroyed = false;
-    }
-    removeAllListeners() {
-    }
-    setKeepAlive() {
-    }
-    setTimeout() {
-    }
-    destroy() {
-        this.destroyed = true;
-    }
-    write() {
-    }
-}
-
 describe("FTPContext", function() {
@@ -32,9 +16,15 @@
 
-    it("Setting data socket undefined destroys current", function() {
+    it("Setting new control socket doesn't destroy current", function() {
+        const old = ftp.socket;
+        ftp.socket = undefined;
+        assert.equal(old.destroyed, false, "Socket not destroyed.");                
+    });
+
+    it("Setting new data socket destroys current", function() {
         const old = ftp.dataSocket;
         ftp.dataSocket = undefined;
-        assert.equal(old.destroyed, true, "Socket not destroyed.");
+        assert.equal(old.destroyed, true, "Socket destroyed.");
     });
 
-    it("Relays control timeout event", function(done) {
+    it("Relays control socket timeout event", function(done) {
         ftp.handle(undefined, (res, task) => {
@@ -47,3 +37,3 @@ assert.deepEqual(res, { error: "Timeout" });
 
-    it("Relays control error event", function(done) {
+    it("Relays control socket error event", function(done) {
         ftp.handle(undefined, (res, task) => {
@@ -56,2 +46,18 @@ assert.deepEqual(res, { error: { foo: "bar" } });
 
+    it("Relays data socket timeout event", function(done) {
+        ftp.handle(undefined, (res, task) => {
+            assert.deepEqual(res, { error: "Timeout" });
+            done();
+        });
+        ftp.dataSocket.emit("timeout");
+    });
+
+    it("Relays data socket error event", function(done) {
+        ftp.handle(undefined, (res, task) => {
+            assert.deepEqual(res, { error: { foo: "bar" } });
+            done();
+        });
+        ftp.dataSocket.emit("error", { foo: "bar" });
+    });
+
     it("Relays single line control response", function(done) {
@@ -106,2 +112,31 @@ ftp.handle(undefined, (res, task) => {
     });
+
+    it("can send a command", function(done) {
+        ftp.socket.once("didSend", buf => {
+            assert.equal(buf.toString(), "HELLO TEST\r\n");
+            done();
+        });
+        ftp.send("HELLO TEST");
+    });
+
+    it("is using UTF-8 by default", function(done) {
+        ftp.socket.once("didSend", buf => {
+            assert.equal(buf.toString(), "HELLO 直己\r\n");
+            done();
+        }); 
+        ftp.send("HELLO 直己");
+    });
+
+    it("destroys sockets when closing", function() {
+        ftp.close();
+        assert(ftp.socket.destroyed, "Control socket");
+        assert(ftp.dataSocket.destroyed, "Data socket");
+    });
+
+    it("reports whether socket has TLS", function() {
+        ftp.socket = new net.Socket();
+        assert(!ftp.hasTLS);
+        ftp.socket = new tls.TLSSocket();
+        assert(ftp.hasTLS);
+    });
 });

No alert changes

Improved metrics

Total package byte prevSize

108571

increased by14.54%

Number of package files

23

increased by43.75%

Lines of code

1737

increased by24.52%

Worsened metrics

Number of low supply chain risk alerts

2

increased by100%

Number of medium supply chain risk alerts

4

increased by100%

No dependency changes