Repository navigation
Issue #91: public listSubdirectories, structured listFiles entries, follow symbolic links - #92
Conversation
…ollow symbolic links - listFiles returns List<SshClient.FileEntry> (path, size, mtime) instead of a "path;mtime;size" string, lists regular files only, follows symbolic links to regular files (stat of the target) and skips dangling links. File names are no longer trimmed. - New public listSubdirectories(dir, mask): subdirectories whose name matches the mask, symbolic links to directories included. The mask is checked before a link is followed. - The SFTP client of both methods is closed when the listing fails. - Version 1.1.00 (breaking change of listFiles), release workflow publishes automatically (autoRelease: true). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… when listing subfolders Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57ee8ddf3e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } catch (SFTPException e) { | ||
| return null; |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
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.
| releaseVersion: ${{ inputs.releaseVersion }} | ||
| developmentVersion: ${{ inputs.developmentVersion }} | ||
| autoRelease: false | ||
| autoRelease: true |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
Fixed in c3a8c2d: the README now says the release is published automatically.
| final SFTPv3FileAttributes attributes = followSymlink(sftpClient, path, entry); | ||
| if (attributes != null && attributes.isDirectory()) { |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
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.
|
|
||
| // Update the response | ||
| return resultBuilder.toString(); | ||
| final String path = directoryPath + "/" + name; |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
Unchanged from the previous listSubDirectory, which built the paths the same way; callers pass absolute directory paths.
| <artifactId>ssh-java</artifactId> | ||
| <name>SSH Java Client</name> | ||
| <version>1.0.05-SNAPSHOT</version> | ||
| <version>1.1.00-SNAPSHOT</version> |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
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.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 participant
Closes #91. Needed by MetricsHub/metricshub-community#1363 (SSH File source resolves its path patterns over SFTP instead of
find/sh -c/ PowerShell commands).Changes
listFiles(dir, mask, includeSubfolders)returnsList<SshClient.FileEntry>(path,size,mtimein seconds since the epoch) instead of apath;mtime;sizestring. Breaking change, hence version 1.1.00. No other caller exists in the MetricsHub organization.statof the target, likefind -L): the entry carries the target's size and mtime. Dangling links are skipped. WithincludeSubfolders, symbolic links to directories are still not descended into (no loop).stat'ed.listSubdirectories(dir, mask), new and public: paths of the subdirectories whose name matches the mask, symbolic links to directories included, dangling links skipped. The mask is checked before a link is followed, so a directory full of unrelated links costs no extra round trip.Matcher.find(), every name when null or empty.finallyblock, also when the listing fails.release.yml:autoRelease: true, so the release is published to Maven Central without the manual step on the portal.pmd.xml): 16 violations onmain, 3 now; the remaining 3 are in code this PR does not touch. Checkstyle (checkstyle.xml): 0.Tests
mvn verify: 17 tests, 0 failures. NewtestListFilesandtestListSubdirectories(mockedSFTPv3Client) cover regular files, symbolic links to files and directories, dangling links, devices, sockets,./.., names with spaces and;, the mask applied before a link is followed, recursion, and the client closed on failure.bm-linux-slack), HP-UX (rx2800), AIX (toland) and an old AIX with an old SSH server (euclide): identical results on all four hosts. Symbolic links to files and directories were followed, dangling links, links to directories and a FIFO were skipped, a hidden directory was excluded by a(?!\.)mask,(?-i)made the mask case-sensitive, and names with spaces,;,$, backticks, quotes, brackets and leading or trailing spaces came back intact.tc-win2022), paths in the SFTP form/C:/mh1363/...:;,$, backticks, brackets and quotes came back intact.readFileaccepted the/C:/,C:/andC:\forms.//localhost/C$/...;///localhost/...is rejected withSSH_FX_BAD_MESSAGE.🤖 Generated with Claude Code