theophilusx / ssh2-sftp-client

a client for SSH2 SFTP
Apache License 2.0
819 stars 200 forks source link

Call to list is hanging indefinitley #466

Closed theabuitendyk closed 1 year ago

theabuitendyk commented 1 year ago

Hello, I have a scheduled task that connects to an SFTP server, lists the files in a directory, and then disconnects. After connecting successfully, the call to list the files in the directory is hanging indefinitely. Is there a way to abort list?

ssh2-sftp-client version: 8.0.0 ssh2 version: 1.10.0 node version: 16.13

const client = new Ssh2SftpClient();

try {
  await client.connect({
    host: options.host,
    port: options.port,
    username: options.username,
    password: options.password,
    readyTimeout: 60000,
  });
} catch (error) {
  this.handleError(error)
}

const objects;
try {
  objects = await client.list(remotePath); // <-- Listing hangs indefinitely
} catch (error) {
  this.handleError(error);
}

await client.end();
theophilusx commented 1 year ago

THe ability to cancel an async request was only added to node in march

  1. As this functionality is not yet available in all supported versions for this module, it has not yet been incorporated. So currently, there is no clean way to cancel an async request.

The first thing you need to do to track down this issue is upgrade to the current version i.e. 9.0.4. You should also add a debug logging function using the debug property of the config object to get more information.

Finally, if your still having problems, provide a working script which reproduces the issue i.e. something I can just run with

node ./your-script.js

and include log output (with a debug logging function enabled).

Also good to know what platform the client and the server are running on. For example, there have been numerous issues reported which have all turned out to be due to docker misconfiguration, WSL network bugs or non-standard or buggy SAAS sftp implementations or some weirdness with an 'optimised' cloud container.

I need to be able to reproduce the issue in order to help debug the problem. The closer I can get to the same environment you have the more likely it is I can reproduce the issue.

Things should not hang indefinitely - they may hang for quite some time, depending on various timeout settings (depends on node version, client and server OS and server settings), but they should eventually time out. In the past, similar issues have turned out to be due to firewalls dropping (not rejecting, just dropping) packets. Therefore, it can be useful to try running your script from different environments or to different servers. Often people spend hours debugging the JS script only to find out the issue was with the network or the remote server.

There are no other reports of this issue, so it is likely specific to your environment.

You could also try using the current head of the master branch in the git repository. There are some bug fixes which are currently being tested in that version. Of course, there could also be other issues due to the changes in that version which are still being tested.

theabuitendyk commented 1 year ago

@theophilusx thanks for the quick reply!

For some more context, I'm able to connect to and list from the directories of many sftp servers with no issue. This issue is intermittent and likely isolated to a single sftp server that I don't have control of.

Are there any plans to add support for setting a timeout for methods other than connect?

I'm working on coming up with an example that reproduces the issue.

theophilusx commented 1 year ago

Are there any plans to add support for setting a timeout for methods other than connect?

No, not at this point. This isn't a common issue and not a feature which ahs been requested previously.

You might be able to simulate a timeout mechanism using Promise.any and a promise wrapped around a setTimeout? The idea would be either the list() promise resolves or the setTimeout resolves if list() has not resolved before the timeOut period.

n0minal commented 1 year ago

@theophilusx we're still facing the same issue in the latest lib version, would you accept a PR to introduce a timeout to get/put/delete/list/fast* operations?

theophilusx commented 1 year ago

I would consider a PR, but here are some things to consider.

  1. The next release of ssh2-sftp-client will only support node v18+ (Node v16 is end of life as of today!).
  2. Node v20 introduces timers which have support for promise timeouts, so this functionality will be made available in ssh2-sftp-client once node v18 is end-of-life. (it is possible you could use this functionality if using node v20 without modifications to the ssh2-sftp-client library - I haven't looked at it closely yet).
  3. I am unlikely to be in a position to release a new version of the library for at least 2 months. THis is partly due to time/resource constraints and partly due to neding to fix a fairly significant bug in donwloadDir/uploadDir and I want that fix in the next release.
  4. Anyh PR which introduces new functionality must also include a set of unit tests.

At this time, you are the only user who is reporting this issue. On this basis, I'm reluctant to add functionality which will likely complicate the API and introduce other unforeseen issues just to resolve an issue which is likely unique to your environment and probably the symptom of some other deeper cause. Experience has been that such PRs often become more of a burden than a benefit.

My suggestion would be to upgrade your node environment to v20 and see if you can resolve the issues using the timers support in v20. If necessary, you could fork ssh2-sftp-client and make adjustments ini that fork. Once node v18 is EOL, it would then be possible to merge your fork back into the main code base, removing the need for you to maintain a fork and at the same time, providing a solution for your issue in the short term.

stunited-thien commented 8 months ago

Hi @theophilusx , I had the same issue with the codebase as @theabuitendyk where calling the list function caused the SFTP server (built from the ssh2 Node.js package) to fire the READDIR event endlessly. This issue did not occur when I replaced the ssh2-sftp-client package with the ss2 package for SFTP client and use sftp.opendir with sftp.readdir. But this issue will happen if sftp.readdir is used without sftp.opendir. I'm currently using Node.js v21.7.0 and ssh2 v1.15.0

theophilusx commented 8 months ago

What version of ssh2-sftp-client? What version of node?

stunited-thien commented 8 months ago

What version of ssh2-sftp-client? What version of node?

ssh2-sftp-client is v10.0.3 and node is v21.7.0 (for both SFTP Client and SFTP Server)

stunited-thien commented 8 months ago

HI @theophilusx, here is my SFTP server code

const { Server, utils: { generateKeyPairSync, sftp: { STATUS_CODE, flagsToString, OPEN_MODE } } } = require('ssh2')
const fs = require('fs')
const path = require('path');
const isSftpFlag = require('./utils/isSftpFlag');

const keys = generateKeyPairSync('ecdsa', { bits: 256, comment: 'node.js rules!' });
/**@type {import('ssh2').ServerConfig} */
const config = {
  hostKeys: [keys.private],
  // debug: console.log
}

new Server(config, function (client, info) {
  console.log('Client connected!')
  console.dir(info)
  client.on('authentication', function (ctx) {
    if (ctx.method === 'password' && ctx.username === 'admin' && ctx.password === 'secret') {
      ctx.accept()
    } else {
      ctx.reject(['password'])
    }
  }).on('ready', function () {
    client.on('session', function (accept, reject) {
      const session = accept()
      console.log('Session accepted!')
      session.on('sftp', function (accept, reject) {
        const sftp = accept()
        console.log('SFTP accepted!')
        sftp.on('ready', function () {
          console.log('SFTP session ready!')
        }).on('OPENDIR', function (reqid, dirPath) {
          console.log('OPENDIR:', dirPath)
          fs.readdir(dirPath, (err, files) => {
            if (err) {
              return sftp.status(reqid, STATUS_CODE.FAILURE, err.message);
            }
            files = files.map((file) => path.join(dirPath, file))
            const handle = Buffer.from(files.join(';'))
            sftp.handle(reqid, handle)
          });
        }).on('READDIR', function (reqid, handle) {
          const filesString = handle.toString();
          const files = filesString ? filesString.split(';') : []
          /**@type {import('ssh2').FileEntry[]} */
          const names = files.map((filePath) => {
            const stats = fs.statSync(filePath)
            const filename = path.basename(filePath)
            return {
              filename,
              longname: filePath,
              attrs: {
                mode: stats.mode,
                uid: stats.uid,
                gid: stats.gid,
                size: stats.size,
                atime: stats.atimeMs,
                mtime: stats.mtimeMs
              }
            }
          })
          console.log('READDIR:', names) // this continues logging indefinitely
          sftp.name(reqid, names)
        })
      })
    })
  }).on('end', function () {
    console.log('Client disconnected!')
    client.end()
  }).on('close', function () {
    client.emit('end')
  }).on('error', function (err) {
    console.log(err)
  })
}).listen(22, '127.0.0.1', function () {
  console.log('Listening on port ' + this.address().port);
})

here is my SFTP Client (using ssh2-sftp-client)

const sftp = new Client();
await sftp.connect(config)
const listFile = await sftp.list(remotePath, (fileInfo) => fileInfo.name.endsWith('txt')); // this is handing indefinitley
console.log(listFile.length)
const promiseAll = listFile.map((file) => {
  const remoteFile = `${remotePath}/${file.name}`;
  console.log('remoteFile', remoteFile);
  return sftp.get(remoteFile)
})
const resolvedAll = await Promise.all(promiseAll);
const result = resolvedAll.map((file) => file.toString());
await sftp.end();
res.send(result);

Note: when calling READDIR event the OPENDIR always gets fired first

theophilusx commented 8 months ago

Are you sure your server implementation is protocol compliant? It has been awhile since I implemented a server and your readdir implementation doesn't look right.

Have you tried running your client code against a standard openSSH sftp server?

Hav you tried getting a list from your server using openSSH's sftp client?

On Wed, 13 Mar 2024 at 13:27, Thien Tran @.***> wrote:

HI @theophilusx https://github.com/theophilusx, here is my SFTP server code

const { Server, utils: { generateKeyPairSync, sftp: { STATUS_CODE, flagsToString, OPEN_MODE } } } = require('ssh2')const fs = require('fs')const path = require('path');const isSftpFlag = require('./utils/isSftpFlag'); const keys = generateKeyPairSync('ecdsa', { bits: 256, comment: 'node.js rules!' @. {import('ssh2').ServerConfig} /const config = { hostKeys: [keys.private], // debug: console.log} new Server(config, function (client, info) { console.log('Client connected!') console.dir(info) client.on('authentication', function (ctx) { if (ctx.method === 'password' && ctx.username === 'admin' && ctx.password === 'secret') { ctx.accept() } else { ctx.reject(['password']) } }).on('ready', function () { client.on('session', function (accept, reject) { const session = accept() console.log('Session accepted!') session.on('sftp', function (accept, reject) { const sftp = accept() console.log('SFTP accepted!') sftp.on('ready', function () { console.log('SFTP session ready!') }).on('OPENDIR', function (reqid, dirPath) { console.log('OPENDIR:', dirPath) fs.readdir(dirPath, (err, files) => { if (err) { return sftp.status(reqid, STATUS_CODE.FAILURE, err.message); } files = files.map((file) => path.join(dirPath, file)) const handle = Buffer.from(files.join(';')) sftp.handle(reqid, handle) }); }).on('READDIR', function (reqid, handle) { const filesString = handle.toString(); const files = filesString ? filesString.split(';') : [] **@.** {import('ssh2').FileEntry[]} / const names = files.map((filePath) => { const stats = fs.statSync(filePath) const filename = path.basename(filePath) return { filename, longname: filePath, attrs: { mode: stats.mode, uid: stats.uid, gid: stats.gid, size: stats.size, atime: stats.atimeMs, mtime: stats.mtimeMs } } }) console.log('READDIR:', names) // this continues logging indefinitely sftp.name(reqid, names) }) }) }) }).on('end', function () { console.log('Client disconnected!') client.end() }).on('close', function () { client.emit('end') }).on('error', function (err) { console.log(err) })}).listen(22, '127.0.0.1', function () { console.log('Listening on port ' + this.address().port);})

here is my SFTP Client (using ssh2-sftp-client)

const sftp = new Client();await sftp.connect(config)const listFile = await sftp.list(remotePath, (fileInfo) => fileInfo.name.endsWith('txt')); // this is handing indefinitleyconsole.log(listFile.length)const promiseAll = listFile.map((file) => { const remoteFile = ${remotePath}/${file.name}; console.log('remoteFile', remoteFile); return sftp.get(remoteFile)})const resolvedAll = await Promise.all(promiseAll);const result = resolvedAll.map((file) => file.toString());await sftp.end();res.send(result);

— Reply to this email directly, view it on GitHub https://github.com/theophilusx/ssh2-sftp-client/issues/466#issuecomment-1993168680, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMFIKIU5UGPIYQVP3U4GITYX62PVAVCNFSM6AAAAAAWEOC7E2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJTGE3DQNRYGA . You are receiving this because you were mentioned.Message ID: @.***>

-- regards,

Tim

-- Tim Cross

stunited-thien commented 8 months ago

Hi @theophilusx , like I said above, with ssh2 package, I can get the list of files from my SFTP Server without getting hanged. Here is my SFTP Client with using ssh2 package

client.sftp((err, sftp) => {
  //need sftp.opendir to not get hanged like ssh2-sftp-client list function
  sftp.opendir(remotePath, (err, handle) => {
    if (err) {
      throw err
    } else {
      sftp.readdir(handle, async (err, list) => {
        if (err) {
          throw err
        }
        const extension = 'txt'
        const filteredList = list.filter((file) => file.filename.endsWith(extension))
        res.send(filteredList)
      })
    }
  })
})
stunited-thien commented 8 months ago

Hi @theophilusx , regarding testing my SFTP server with openSSH's sftp client, I tried to connect to Win SCP application and my Server still hanged due to triggering the same READDIR event indefinitely. Noted: both my Server and Client are running on Windows 11

theophilusx commented 8 months ago

As I said, pretty sure the problem is due to bugs in your server implementation.

If you look at the ssh2 documentation, you are supposed to send back a status() to tell the client you have returned all the directory data. Your implementation does not do this, so the client does not know the server has completed sending back the directory contents and is therefore waiting for more data.

From the ssh2 docs

READDIR(< integer >reqID, < Buffer >handle)

Respond using one of the following:

name() - Use this to send one or more directory listings for the open directory back to the client.

status() - Use this to indicate either end of directory contents (STATUS_CODE.EOF) or if an error occurred while reading the directory contents.
stunited-thien commented 8 months ago

As I said, pretty sure the problem is due to bugs in your server implementation. If you look at the ssh2 documentation, you are supposed to send back a status() to tell the client you have returned all the directory data. Your implementation does not do this, so the client does not know the server has completed sending back the directory contents and is therefore waiting for more data. From the ssh2 docs READDIR(< integer >reqID, < Buffer >handle) Respond using one of the following: name() - Use this to send one or more directory listings for the open directory back to the client. status() - Use this to indicate either end of directory contents (STATUS_CODE.EOF) or if an error occurred while reading the directory contents.

Hi @theophilusx , like you said the READDIR event uses one of those 2 functions to end the response and my Server code above already had it right below the console.log that I included with a comment.

theophilusx commented 8 months ago

I'm not going to go back and forth arguing the point.

The FTP protocl is a well defined protocol with specific sequence of commands and data formats. Your server implementation is not comply with the FTP specification.

I would recommend you add a debug funciton to your client and perform some basic operations against openSSH's sftp server to see the sequence of commands/responses seen with a compliant ftp server. Also, you should read RFC 959 to understand the FTP protocol format.