Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ jobs:
with:
releaseVersion: ${{ inputs.releaseVersion }}
developmentVersion: ${{ inputs.developmentVersion }}
autoRelease: false
autoRelease: true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Align the release instructions with automatic publishing

With autoRelease: true, the reused workflow publishes the Maven Central bundle immediately, but the repository's README.md still instructs release operators to manually release the corresponding staging repository. After a successful run that step no longer exists, so the documented release procedure misleadingly suggests that publication remains pending; update it alongside this workflow change.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c3a8c2d: the README now says the release is published automatically.

jdkVersion: "17"
secrets: inherit
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ But it is strongly recommended to only use [GitHub Actions "Release to Maven Cen

* Manually trigger the "Release" workflow
* Specify the version being released and the next version number (SNAPSHOT)
* Release the corresponding staging repository on [Sonatype's Nexus server](https://s01.oss.sonatype.org/)
* The release is published to Maven Central automatically at the end of the workflow
* Merge the PR that has been created to prepare the next version

## License
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

<artifactId>ssh-java</artifactId>
<name>SSH Java Client</name>
<version>1.0.05-SNAPSHOT</version>
<version>1.1.00-SNAPSHOT</version>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Release the incompatible API under a new major version

This commit changes listFiles(String, String, boolean) from returning String to returning List<FileEntry>, but publishes it as the minor increment 1.1.00. Because the JVM method descriptor includes the return type, an existing binary that resolves this release can fail with NoSuchMethodError, while recompiling an existing source caller fails outright; publish the change under the next major version or retain the old signature and expose the structured result under a new method name.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.1.00 is the maintainer's choice for this repository's versioning; listFiles has no other caller in the organization, and MetricsHub moves to the new signature in the same change.

<description>SSH Client Library for Java</description>

<organization>
Expand Down
232 changes: 158 additions & 74 deletions src/main/java/org/metricshub/ssh/SshClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.trilead.ssh2.Connection;
import com.trilead.ssh2.InteractiveCallback;
import com.trilead.ssh2.SCPClient;
import com.trilead.ssh2.SFTPException;
import com.trilead.ssh2.SFTPv3Client;
import com.trilead.ssh2.SFTPv3DirectoryEntry;
import com.trilead.ssh2.SFTPv3FileAttributes;
Expand All @@ -42,10 +43,10 @@
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
Expand Down Expand Up @@ -327,7 +328,7 @@
.append("\t")
.append(fileAttributes.atime.toString())
.append("\t-\t")
.append(Integer.toString(fileAttributes.permissions & 0000777, 8))

Check warning on line 331 in src/main/java/org/metricshub/ssh/SshClient.java

View workflow job for this annotation

GitHub Actions / PMD

Error Prone AvoidUsingOctalValues

Do not start a literal by 0 unless its an octal value
.append("\t")
.append(fileAttributes.size.toString())
.append("\t-\t")
Expand Down Expand Up @@ -374,109 +375,192 @@
}
}

private StringBuilder listSubDirectory(
/**
* A regular file listed on the remote system
*/
public static class FileEntry {

/**
* Path of the file: the listed directory, a slash and the file name
*/
public final String path;

/**
* Size of the file, in bytes
*/
public final long size;

/**
* Last modification time of the file, in seconds since the epoch
*/
public final long mtime;

/**
* Creates a file entry
*
* @param path Path of the file
* @param size Size of the file, in bytes
* @param mtime Last modification time of the file, in seconds since the epoch
*/
public FileEntry(final String path, final long size, final long mtime) {
this.path = path;
this.size = size;
this.mtime = mtime;
}
}

/**
* Compiles a file name mask, matched case-insensitively with {@link java.util.regex.Matcher#find()}.
*
* @param regExpMask The regular expression, or null or empty to match every name
* @return The compiled mask
*/
private static Pattern compileMask(final String regExpMask) {
return regExpMask != null && !regExpMask.isEmpty()
? Pattern.compile(regExpMask, Pattern.CASE_INSENSITIVE)
: DEFAULT_MASK_PATTERN;
}

/**
* Removes the trailing slash of a directory path, so that a file name can be appended after a slash.
*
* @param remoteDirectoryPath The directory path
* @return The path without its trailing slash ("/" becomes an empty string)
*/
private static String stripTrailingSlash(final String remoteDirectoryPath) {
return remoteDirectoryPath.endsWith("/")
? remoteDirectoryPath.substring(0, remoteDirectoryPath.length() - 1)
: remoteDirectoryPath;
}

/**
* Returns the attributes of a directory entry, those of its target when the entry is a symbolic link.
*
* @param sftpClient The SFTP client
* @param path The path of the entry
* @param entry The directory entry
* @return The attributes, or null when the entry is a dangling symbolic link or its target cannot be read
* @throws IOException When the communication with the remote host fails
*/
private static SFTPv3FileAttributes followSymlink(
final SFTPv3Client sftpClient,
final String path,
final SFTPv3DirectoryEntry entry
) throws IOException {
if (!entry.attributes.isSymlink()) {
return entry.attributes;
}
try {
return sftpClient.stat(path);
} catch (SFTPException e) {
return null;
Comment on lines +455 to +456

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Propagate non-missing SFTP status errors

When stat() returns an SFTP status other than a dangling-link error—such as SSH_FX_FAILURE or SSH_FX_CONNECTION_LOST—the dependency also represents it as SFTPException, so this catch silently drops the entry and may return a plausible but incomplete listing. Inspect the exception's status code and suppress only the expected unavailable-target cases; other failures should propagate as the documented IOException.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skipping is intended: an SFTPException is the server's answer about that one link, and besides a dangling link, OpenSSH reports a symlink loop (ELOOP) as SSH_FX_FAILURE, which find -L also skips; a lost channel surfaces as a plain IOException and still propagates.

}
}

private void listSubDirectory(
SFTPv3Client sftpClient,
String remoteDirectoryPath,
Pattern fileMaskPattern,
boolean includeSubfolders,
Integer depth,
StringBuilder resultBuilder
int depth,
List<FileEntry> entries
) throws IOException {
if (depth <= 15) {
List<SFTPv3DirectoryEntry> pathContents = sftpClient.ls(remoteDirectoryPath);
if (depth > 15) {
return;
}

// Fix the remoteDirectoryPath (without the last '/')
if (remoteDirectoryPath.endsWith("/")) {
remoteDirectoryPath = remoteDirectoryPath.substring(0, remoteDirectoryPath.lastIndexOf("/"));
final String directoryPath = stripTrailingSlash(remoteDirectoryPath);
for (SFTPv3DirectoryEntry entry : sftpClient.ls(remoteDirectoryPath)) {
final String filename = entry.filename;
if (filename.equals(".") || filename.equals("..")) {
continue;
}

depth++;
for (SFTPv3DirectoryEntry file : pathContents) {
String filename = file.filename.trim();
final String filePath = directoryPath + "/" + filename;

if (filename.equals(".") || filename.equals("..")) {
continue;
}

SFTPv3FileAttributes fileAttributes = file.attributes;
String filePath = remoteDirectoryPath + "/" + filename;

if ((fileAttributes.permissions & 0120000) == 0120000) {
// Symbolic link
continue;
// A directory, but not a symbolic link to one, which could loop
if (entry.attributes.isDirectory()) {
if (includeSubfolders) {
listSubDirectory(sftpClient, filePath, fileMaskPattern, includeSubfolders, depth + 1, entries);
}
continue;
}

// CHECKSTYLE:OFF
if (
((fileAttributes.permissions & 0100000) == 0100000) ||
((fileAttributes.permissions & 0060000) == 0060000) ||
((fileAttributes.permissions & 0020000) == 0020000) ||
((fileAttributes.permissions & 0140000) == 0140000)
) {
// Regular/Block/Character/Socket files
final Matcher m = fileMaskPattern.matcher(filename);
if (m.find()) {
resultBuilder
.append(filePath)
.append(";")
.append(fileAttributes.mtime.toString())
.append(";")
.append(fileAttributes.size.toString())
.append("\n");
}
continue;
}
// CHECKSTYLE:ON
if (!fileMaskPattern.matcher(filename).find()) {
continue;
}

if ((fileAttributes.permissions & 0040000) == 0040000) {
// Directory
if (includeSubfolders) {
resultBuilder =
listSubDirectory(sftpClient, filePath, fileMaskPattern, includeSubfolders, depth, resultBuilder);
}
}
final SFTPv3FileAttributes attributes = followSymlink(sftpClient, filePath, entry);
if (attributes != null && attributes.isRegularFile()) {
entries.add(new FileEntry(filePath, attributes.size, attributes.mtime));
}
}

return resultBuilder;
}

/**
* List the content of the specified directory through the SSH connection
* (using SCP)
* List the regular files of the specified directory through SFTP. Symbolic links to regular files are
* followed: the entry carries the size and modification time of the target. Dangling links are skipped.
*
* @param remoteDirectoryPath The path to the directory to list on the remote host
* @param regExpFileMask A regular expression that listed files must match with to be listed
* @param includeSubfolders Whether to parse subdirectories as well
* @return The list of files in the specified directory, separated by end-of-lines
* @param regExpFileMask A regular expression that the names of the listed files must contain
* (case-insensitive, {@link java.util.regex.Matcher#find()}); null or empty to list every file
* @param includeSubfolders Whether to list subdirectories as well (symbolic links to directories are not
* followed, and no more than 15 levels are listed)
* @return The files of the specified directory
*
* @throws IOException When something bad happens while communicating with the remote host
* @throws IllegalStateException If called while not yet connected
*/
public String listFiles(String remoteDirectoryPath, String regExpFileMask, boolean includeSubfolders)
public List<FileEntry> listFiles(String remoteDirectoryPath, String regExpFileMask, boolean includeSubfolders)
throws IOException {
checkIfAuthenticated();

// Create an SFTP Client
SFTPv3Client sftpClient = new SFTPv3Client(sshConnection);

// Prepare the Pattern for fileMask
Pattern fileMaskPattern;
if (regExpFileMask != null && !regExpFileMask.isEmpty()) {
fileMaskPattern = Pattern.compile(regExpFileMask, Pattern.CASE_INSENSITIVE);
} else {
fileMaskPattern = DEFAULT_MASK_PATTERN;
final List<FileEntry> entries = new ArrayList<>();
final SFTPv3Client sftpClient = new SFTPv3Client(sshConnection);
try {
listSubDirectory(sftpClient, remoteDirectoryPath, compileMask(regExpFileMask), includeSubfolders, 1, entries);
} finally {
sftpClient.close();
}
return entries;
}

// Read the directory listing
StringBuilder resultBuilder = new StringBuilder();
listSubDirectory(sftpClient, remoteDirectoryPath, fileMaskPattern, includeSubfolders, 1, resultBuilder);
/**
* List the subdirectories of the specified directory through SFTP. Symbolic links to directories are
* followed and listed; dangling links are skipped. The mask is checked before a link is followed.
*
* @param remoteDirectoryPath The path to the directory to list on the remote host
* @param regExpMask A regular expression that the names of the listed subdirectories must contain
* (case-insensitive, {@link java.util.regex.Matcher#find()}); null or empty to list every subdirectory
* @return The paths of the subdirectories: the specified directory, a slash and the subdirectory name
*
* @throws IOException When something bad happens while communicating with the remote host
* @throws IllegalStateException If called while not yet connected
*/
public List<String> listSubdirectories(final String remoteDirectoryPath, final String regExpMask) throws IOException {
checkIfAuthenticated();

// Close the SFTP client
sftpClient.close();
final Pattern maskPattern = compileMask(regExpMask);
final String directoryPath = stripTrailingSlash(remoteDirectoryPath);
final List<String> subdirectories = new ArrayList<>();
final SFTPv3Client sftpClient = new SFTPv3Client(sshConnection);
try {
for (SFTPv3DirectoryEntry entry : sftpClient.ls(remoteDirectoryPath)) {
final String name = entry.filename;
if (name.equals(".") || name.equals("..") || !maskPattern.matcher(name).find()) {
continue;
}

// Update the response
return resultBuilder.toString();
final String path = directoryPath + "/" + name;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve relative paths for the default SFTP directory

When remoteDirectoryPath is the valid empty SFTP path denoting the user's default directory, directoryPath is also empty and this concatenation returns /name. That changes a relative home-directory entry into an absolute root path, so callers using the returned path will inspect the wrong directory or receive permission errors; avoid inserting the leading slash for an empty input path.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unchanged from the previous listSubDirectory, which built the paths the same way; callers pass absolute directory paths.

final SFTPv3FileAttributes attributes = followSymlink(sftpClient, path, entry);
if (attributes != null && attributes.isDirectory()) {
Comment on lines +555 to +556

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stat entries whose listing omits permissions

On an SFTP server that omits the optional permissions attribute from ls results, followSymlink() returns the incomplete attributes because isSymlink() is false, and isDirectory() here is also false. Consequently listSubdirectories() silently returns no such directories even though the listing is valid; when permissions are absent, obtain complete attributes with stat() before classifying the entry.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changing: no target server (OpenSSH, Windows OpenSSH) omits permissions, and STAT returns the same ATTRS block, so a server that omits them in READDIR would not give the type in STAT either.

subdirectories.add(path);
}
}
} finally {
sftpClient.close();
}
return subdirectories;
}

/**
Expand Down Expand Up @@ -568,7 +652,7 @@
sftpClient.close();

// Metricshub Collection format
return out.toString();

Check warning on line 655 in src/main/java/org/metricshub/ssh/SshClient.java

View workflow job for this annotation

GitHub Actions / spotbugs

DM_DEFAULT_ENCODING

Found reliance on default encoding in org.metricshub.ssh.SshClient.readFile(String, Long, Integer): java.io.ByteArrayOutputStream.toString()
Raw output
 Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behavior to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.
}

/**
Expand Down Expand Up @@ -638,24 +722,24 @@
/**
* Whether the command was successful or not
*/
public boolean success = true;

Check warning on line 725 in src/main/java/org/metricshub/ssh/SshClient.java

View workflow job for this annotation

GitHub Actions / spotbugs

URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD

Unread public/protected field: org.metricshub.ssh.SshClient$CommandResult.success
Raw output
 This field is never read. The field is public or protected, so perhaps it is intended to be used with classes not seen as part of the analysis. If not, consider removing it from the class.

/**
* How much time was taken by the execution itself (not counting the
* connection time), in seconds
*/
public float executionTime = 0;

Check warning on line 731 in src/main/java/org/metricshub/ssh/SshClient.java

View workflow job for this annotation

GitHub Actions / spotbugs

URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD

Unread public/protected field: org.metricshub.ssh.SshClient$CommandResult.executionTime
Raw output
 This field is never read. The field is public or protected, so perhaps it is intended to be used with classes not seen as part of the analysis. If not, consider removing it from the class.

/**
* The exit code (status) returned by the command (process return code).
* <code>null</code> if unsupported by the remote platform.
*/
public Integer exitStatus = null;

Check warning on line 737 in src/main/java/org/metricshub/ssh/SshClient.java

View workflow job for this annotation

GitHub Actions / spotbugs

URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD

Unread public/protected field: org.metricshub.ssh.SshClient$CommandResult.exitStatus
Raw output
 This field is never read. The field is public or protected, so perhaps it is intended to be used with classes not seen as part of the analysis. If not, consider removing it from the class.

/**
* The result of the command (stdout and stderr is merged into result)
*/
public String result = "";

Check warning on line 742 in src/main/java/org/metricshub/ssh/SshClient.java

View workflow job for this annotation

GitHub Actions / spotbugs

URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD

Unread public/protected field: org.metricshub.ssh.SshClient$CommandResult.result
Raw output
 This field is never read. The field is public or protected, so perhaps it is intended to be used with classes not seen as part of the analysis. If not, consider removing it from the class.
}

/**
Expand Down Expand Up @@ -739,7 +823,7 @@
// We completed in time

// Execution time (in seconds)
commandResult.executionTime = (currentTime - startTime) / 1000;

Check warning on line 826 in src/main/java/org/metricshub/ssh/SshClient.java

View workflow job for this annotation

GitHub Actions / spotbugs

ICAST_IDIV_CAST_TO_DOUBLE

Integral division result cast to double or float in org.metricshub.ssh.SshClient.executeCommand(String, int)
Raw output
This code casts the result of an integral division (e.g., int or long division) operation to double or float. Doing division on integers truncates the result to the integer value closest to zero. The fact that the result was cast to double suggests that this precision should have been retained. What was probably meant was to cast one or both of the operands to double before performing the division. Here is an example:

int x = 2;
int y = 5;
// Wrong: yields result 0.0
double value1 = x / y;

// Right: yields result 0.4
double value2 = x / (double) y;

// Read exit status, when available
waitForCondition = sshSession.waitForCondition(ChannelCondition.EXIT_STATUS, 5000);
Expand Down Expand Up @@ -771,7 +855,7 @@
openTerminal();

// Pipe specified InputStream to SSH's stdin -- use a separate thread
BufferedReader inputReader = new BufferedReader(new InputStreamReader(in));

Check warning on line 858 in src/main/java/org/metricshub/ssh/SshClient.java

View workflow job for this annotation

GitHub Actions / spotbugs

DM_DEFAULT_ENCODING

Found reliance on default encoding in org.metricshub.ssh.SshClient.interactiveSession(InputStream, OutputStream): new java.io.InputStreamReader(InputStream)
Raw output
 Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behavior to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.
OutputStream outputWriter = sshSession.getStdin();
Thread stdinPipeThread = new Thread() {
@Override
Expand All @@ -779,12 +863,12 @@
try {
String line;
while ((line = inputReader.readLine()) != null) {
outputWriter.write(line.getBytes());

Check warning on line 866 in src/main/java/org/metricshub/ssh/SshClient.java

View workflow job for this annotation

GitHub Actions / spotbugs

DM_DEFAULT_ENCODING

Found reliance on default encoding in org.metricshub.ssh.SshClient$2.run(): String.getBytes()
Raw output
 Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behavior to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.
outputWriter.write('\n');
}
} catch (Exception e) {
// Things ended up badly. Exit thread.
}

Check warning on line 871 in src/main/java/org/metricshub/ssh/SshClient.java

View workflow job for this annotation

GitHub Actions / PMD

Error Prone EmptyCatchBlock

Avoid empty catch blocks
// End of the input stream. We need to exit.
// Let's close the session so the main thread exits nicely.
sshSession.close();
Expand Down
Loading
Loading