Community
Participate
Working Groups
The removeTrailingSlash() method in org.eclipse.ptp.remotetools.internal.ssh.FileTools does not work for the root path ("/"). Removing the trailing slash from "/" should result in the same path, not in an empty path as happens currently. I suggest explicitly testing for this case: if (!path.equals("/") && path.endsWith("/")) { //$NON-NLS-1$ //$NON-NLS-2 return path.substring(0, path.length() - 1); } else { return path; } Comments?
I agree with the proposed fix. But here are my thoughts about this issue: According to the the most recent documentation that I found available for sftp (http://tools.ietf.org/wg/secsh/draft-ietf-secsh-filexfer/), paths are strings with directories separated with backslashes. It is quite similar to paths on Unix, but not identical. Therefore, some additional care should be taken. The documentation states that an empty path is to interpreted as the user's default directory (in most cases, the home directory). As removeTrailingSlash("/") returns "", this is clearly a bug. However, im my oppinion, the sftp documentation does not clearly state how to represent the path for the root directory, although it is very reasonable to suppose to be "/". And practice has shown that sftp servers understood "/" as the root directory. Some further consideration that were taken in the design of RemoteTools: Paths may be absolute (first character is a backslash) or relative. Since servers may vary how they handle relative paths, it was decided that RemoteTools will always call a sftp command with an absolute path. The sftp documentation does not state how to handle paths with a trailing backslash, as it is used often on Linux for a directory path. Therefore, to avoid potential compatibility issues with implementation of ssh servers, it was decided to never call a sftp command with a path that ends with backslash. This is the reason why removeTrailingSlash() is called in org.eclipse.ptp.remotetools.internal.ssh.FileTools.
Created attachment 88148 [details] A patch that fixes the bug as proposed by Greg Watson
Thanks. I'll apply the patch. I don't see any immediate issues that might be caused by absolute paths and removing trailing slashes.
Committed.
Closed, since bug is fixed.
