ssh2-streams icon indicating copy to clipboard operation
ssh2-streams copied to clipboard

Very confusing use case for sftpStream.name

Open adonisv79 opened this issue 7 years ago • 5 comments

In the doc, the instructions is as follows

.on('REALPATH', function(reqid, path) {
          var name = [{
            filename: '/tmp/foo.txt',
            longname: '-rwxrwxrwx 1 foo foo 3 Dec 8 2009 foo.txt',
            attrs: {}
          }];
          sftpStream.name(reqid, name);
        })

since it does not bother explaining anything at all, format and what is happening. I assume filename means the entire filepath. so if it can contain '/var/x/y/z/t/readme.txt' however when i navigate via filezilla. the root will contain these folders with the whole name [-] '/' [-]-> '/var/x/y/z/t/z' -> '/var/x/y/z/t/z/readme.txt' -> '/var/x/y/z/t/readme.txt' -> '/var/x/y/z/t/my_file.txt'

if however I do not include the previous path string. when I open for example the path '/var/x/y', the realpath event notes the the path being opened is '/y' which is worse... kinda frustrating to see the randomness in this module.

do we have a more elaborate guide here to use?

adonisv79 avatar Sep 22 '17 05:09 adonisv79

The filename is supposed to be an absolute path, that's more or less the whole point of "realpath" in general. Are you sure it's not READDIR that is your problem here, since it seems like the client is retrieving a directory listing (in which case you should not be using absolute paths for filename).

kinda frustrating to see the randomness in this module.

Welcome to software development, where nothing/nobody is perfect. I am open to PRs to improve documentation if that is the issue here.

However, if you can provide more details to reproduce the issue, please do so (including minimal code, FileZilla client version, etc.).

mscdex avatar Sep 22 '17 07:09 mscdex

what happens is run the service and connect using Filezilla 3.27.1, there are times it runs the following events: REALPATH->REALPATH->OPENDIR->CLOSE sometimes it just runs REALPATH and is done. :s

Here are my assumptions on what the code does (because there is not much reference I just ran the code and debug to see where things go) so please correct if my assumption is wrong

  1. RealPath's goal is to convert the path received from the client to the actual FS path we want which we must also normalize.
Ex: a '.' means the root so convert to '/'
a '/hello/my/name/../friend' should be '/hello/my/friend'

So this is my function for this
function onRequestRealPath(req_id, path) {
	try {
		//normalize and make sure its unix based
		let new_path = node_path.normalize(path).replace(/\\/g,'/');
		if (new_path === '.') {
			new_path = '/';
		}

		const name = [{
			filename: new_path,
			attrs: {}
		}];

		console.log('***** [REALPATH] on path from (' + path + ') to (' + new_path + ')');
		sftpStream.name(req_id, name);
	} catch (err) {
		console.error(err.message);
		sftpStream.status(req_id, STATUS_CODE.FAILURE, err.message);
	}
}
	
  1. OPENDIR is triggered when the target is type directory but only after REALPATH (and that response of realpath is equal to the same input path? else it will loop indefinitely?)

  2. READDIR is triggered after OPENDIR once handle is created. (this sometimes happens but sometimes filezilla will just say "Directory /: expected FXP_STATUS packet, got packet type 104" instead and never starts the READDIR. hence what I say is eratic behaviour. sometimes I will decide to work sometimes I just mop around. Its unpredictable.

adonisv79 avatar Sep 23 '17 23:09 adonisv79

  1. If you're working with actual filesystem paths, you should probably be using a proper fs.realpath().
  2. That makes sense. In general you should use absolute paths in SFTP since there is no real concept of changing directories (on the server side) like with other protocols such as FTP.
  3. Without seeing your code for READDIR handling, my guess is that you're sending multiple responses back to back without waiting for the client to send another READDIR (e.g. calling name() and immediately calling status() for EOF afterwards). If that is the case, you will need a bit of internal state to know when to send the EOF status. For example, if you use readdir() you will get all of the filenames at once which you can send back to the client (or you can chunk them, artificially or otherwise if you want), but then you will have to wait for the second READDIR request to send EOF.

With all of the above in mind, I tested a simple sftp server I wrote against FileZilla 3.27.1 on Linux and had no issues navigating directories. If you're not seeing expected READDIR requests and such, it's most likely because of FileZilla's directory cache, so in those cases you will need to press F5 to explicitly re-request a directory listing.

mscdex avatar Sep 25 '17 04:09 mscdex

after hours of testing. I believe absolute paths in READDIR name responses are fail.

let name = [];
if (path === '/') {
	name = [
		{
			filename: 'tmp',
			longname: 'drwxrwxrwx 1 foo foo 3 Dec 8 2009 tmp',
			attrs: {
				size: 1048576,
				uid: 1000,
				gid: 1000,
				mode: 16877,
				atime: 1353269007,
				mtime: 1353269007
			}
		}
	]
} else if (path === '/tmp') {
	name = [
		{
			filename: '/tmp/f1',
			longname: 'drwxrwxrwx 1 foo foo 3 Dec 8 2009 f1',
			attrs: {}
		}
	]
} else if (path === '/tmp/f1') {
	name = [
		{
			filename: '/tmp/f1/f1_1',
			longname: 'drwxrwxrwx 1 foo foo 3 Dec 8 2009 f1_1',
			attrs: {}
		},
		{
			filename: '/tmp/f1/f1_2',
			longname: 'drwxrwxrwx 1 foo foo 3 Dec 8 2009 f1_2',
			attrs: {}
		}
	]
} 
req.done = true;
console.dir(name);
sftpStream.name(req_id, name);

using the sample code above as response of readdir. I get the annoying result which happens across FILEZILLA and WINSCP

inside the /tmp folder, there is a literal foldar named '/tmp/f1' and iside that '/tmp/f1' folder, there are 2 folders named '/tmp/f1/f1_1' and '/tmp/f1/f1_2'

removing the absolute paths

if (path === '/') {
	name = [
		{
			filename: 'tmp',
			longname: 'drwxrwxrwx 1 foo foo 3 Dec 8 2009 tmp',
			attrs: {
				size: 1048576,
				uid: 1000,
				gid: 1000,
				mode: 16877,
				atime: 1353269007,
				mtime: 1353269007
			}
		}
	]
} else if (path === '/tmp') {
	name = [
		{
			filename: 'f1',
			longname: 'drwxrwxrwx 1 foo foo 3 Dec 8 2009 f1',
			attrs: {}
		}
	]
} else if (path === '/tmp/f1') {
	name = [
		{
			filename: 'f1_1',
			longname: 'drwxrwxrwx 1 foo foo 3 Dec 8 2009 f1_1',
			attrs: {}
		},
		{
			filename: 'f1_2',
			longname: 'drwxrwxrwx 1 foo foo 3 Dec 8 2009 f1_2',
			attrs: {}
		}
	]
} 

results in expected behaviour for both.

adonisv79 avatar Sep 27 '17 03:09 adonisv79

Right, that's what I mentioned earlier. For READDIR you should not be using absolute paths. It's REALPATH that needs them (for obvious reasons).

mscdex avatar Sep 27 '17 04:09 mscdex