From 42ed127e973a3d332211cd599fb3074135599eab Mon Sep 17 00:00:00 2001
From: Patrick Juchli <patrickjuchli@gmail.com>
Date: Fri, 26 Mar 2021 20:47:34 +0100
Subject: [PATCH] Only use MLSD after querying support for feature

Fixes #187
---
 src/Client.ts        | 22 +++++++++++++---------
 test/downloadSpec.js | 10 +++++-----
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/src/Client.ts b/src/Client.ts
index ffcacda..fa4a759 100644
--- a/src/Client.ts
+++ b/src/Client.ts
@@ -49,19 +49,16 @@ export interface UploadOptions {
     localEndInclusive?: number
 }
 
+const LIST_COMMANDS_DEFAULT: readonly string[] = ["LIST -a", "LIST"]
+const LIST_COMMANDS_MLSD: readonly string[] = ["MLSD", "LIST -a", "LIST"]
+
 /**
  * High-level API to interact with an FTP server.
  */
 export class Client {
     prepareTransfer: TransferStrategy
     parseList: RawListParser
-    /**
-     * Multiple commands to retrieve a directory listing are possible. This instance
-     * will try all of them in the order presented the first time a directory listing
-     * is requested. After that, `availableListCommands` will  hold only the first
-     * entry that worked.
-     */
-    availableListCommands = ["MLSD", "LIST -a", "LIST"]
+    availableListCommands = LIST_COMMANDS_DEFAULT
     /** Low-level API to interact with FTP server. */
     readonly ftp: FTPContext
     /** Tracks progress of data transfers. */
@@ -234,10 +231,17 @@ export class Client {
      * * Additional settings for FTPS (PBSZ 0, PROT P)
      */
     async useDefaultSettings(): Promise<void> {
+        const features = await this.features()
+        // Use MLSD directory listing if possible. See https://tools.ietf.org/html/rfc3659#section-7.8:
+        // "The presence of the MLST feature indicates that both MLST and MLSD are supported."
+        const supportsMLSD = features.has("MLST")
+        this.availableListCommands = supportsMLSD ? LIST_COMMANDS_MLSD : LIST_COMMANDS_DEFAULT
         await this.send("TYPE I") // Binary mode
         await this.sendIgnoringError("STRU F") // Use file structure
         await this.sendIgnoringError("OPTS UTF8 ON") // Some servers expect UTF-8 to be enabled explicitly
-        await this.sendIgnoringError("OPTS MLST type;size;modify;unique;unix.mode;unix.owner;unix.group;unix.ownername;unix.groupname;") // Make sure MLSD listings include all we can parse
+        if (supportsMLSD) {
+            await this.sendIgnoringError("OPTS MLST type;size;modify;unique;unix.mode;unix.owner;unix.group;unix.ownername;unix.groupname;") // Make sure MLSD listings include all we can parse
+        }
         if (this.ftp.hasTLS) {
             await this.sendIgnoringError("PBSZ 0") // Set to 0 for TLS
             await this.sendIgnoringError("PROT P") // Protect channel (also for data connections)
@@ -263,9 +267,9 @@ export class Client {
             welcome = await this.connect(options.host, options.port)
         }
         if (useExplicitTLS) {
-            const secureOptions = options.secureOptions ?? {}
             // Fixes https://github.com/patrickjuchli/basic-ftp/issues/166 by making sure
             // host is set for any future data connection as well.
+            const secureOptions = options.secureOptions ?? {}
             secureOptions.host = secureOptions.host ?? options.host
             await this.useTLS(secureOptions)
         }
diff --git a/test/downloadSpec.js b/test/downloadSpec.js
index 27489ec..7ed8ffa 100644
--- a/test/downloadSpec.js
+++ b/test/downloadSpec.js
@@ -170,7 +170,7 @@ describe("Download directory listing", function() {
 
     it("sends the right default command", function() {
         client.ftp.socket.once("didSend", command => {
-            assert.equal(command, "MLSD\r\n");
+            assert.equal(command, "LIST -a\r\n");
             sendCompleteList()
         });
         // This will throw an unhandled exception because we close the client when
@@ -181,7 +181,7 @@ describe("Download directory listing", function() {
 
     it("sends the right default command with optional path", function() {
         client.ftp.socket.once("didSend", command => {
-            assert.equal(command, "MLSD my/path\r\n", "Unexpected list command");
+            assert.equal(command, "LIST -a my/path\r\n", "Unexpected list command");
             sendCompleteList()
         });
         // This will throw an unhandled exception because we close the client when
@@ -191,7 +191,7 @@ describe("Download directory listing", function() {
     });
 
     it("tries all other list commands if default one fails", function() {
-        const expectedCandidates = ["MLSD", "LIST -a", "LIST"]
+        const expectedCandidates = ["LIST -a", "LIST"]
         client.ftp.socket.on("didSend", command => {
             const expected = expectedCandidates.shift()
             assert.equal(command, expected + "\r\n", "Unexpected list command candidate");
@@ -212,14 +212,14 @@ describe("Download directory listing", function() {
             counter++
         });
         return client.list().catch(err => {
-            assert.equal(err.message, "501 Syntax error 3")
+            assert.equal(err.message, "501 Syntax error 2")
         });
     })
 
     it("uses first successful list command for all subsequent requests", function() {
         const promise = client.list().then(result => {
             assert.deepEqual(result, expList);
-            assert.deepEqual(["MLSD"], client.availableListCommands)
+            assert.deepEqual(["LIST -a"], client.availableListCommands)
         });
         setTimeout(() => sendCompleteList());
         return promise