Skip to content

Commit

Permalink
Only use MLSD after querying support for feature
Browse files Browse the repository at this point in the history
Fixes #187
  • Loading branch information
patrickjuchli committed Mar 26, 2021
1 parent 4db4f4a commit 42ed127
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 14 deletions.
22 changes: 13 additions & 9 deletions src/Client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down
10 changes: 5 additions & 5 deletions test/downloadSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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");
Expand All @@ -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
Expand Down

0 comments on commit 42ed127

Please sign in to comment.