This is an automated email from the ASF dual-hosted git repository.
twolf pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
The following commit(s) were added to refs/heads/master by this push:
new 794122f0b fix: negate followLinks flag for checkSymlinkState (#843)
794122f0b is described below
commit 794122f0bfb3df30cac8a7d61b2337d97a9986c0
Author: Joshua Winkler <[email protected]>
AuthorDate: Sat Oct 18 16:12:14 2025 +0200
fix: negate followLinks flag for checkSymlinkState (#843)
Missing boolean negations in two places led to wrong behavior.
Add tests for the two cases: fstat and file deletion with symlinks in the
path.
---
.../sftp/server/AbstractSftpSubsystemHelper.java | 2 +-
.../org/apache/sshd/sftp/server/SftpSubsystem.java | 2 +-
.../apache/sshd/sftp/client/SftpVersionsTest.java | 87 ++++++++++++++++++++++
3 files changed, 89 insertions(+), 2 deletions(-)
diff --git
a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java
b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java
index 1c974f538..a2e66fcf3 100755
---
a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java
+++
b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java
@@ -1717,7 +1717,7 @@ public abstract class AbstractSftpSubsystemHelper
LinkOption[] options = accessor.resolveFileAccessLinkOptions(
this, resolvedPath, SftpConstants.SSH_FXP_REMOVE, "", false);
WithFileAttributeCache.withAttributeCache(resolvedPath, p -> {
- Boolean status = checkSymlinkState(p, followLinks, options);
+ Boolean status = checkSymlinkState(p, !followLinks, options);
if (status == null) {
throw signalRemovalPreConditionFailure(id, path, p,
new AccessDeniedException(p.toString(), p.toString(),
"Cannot determine existence of remove candidate"),
diff --git
a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java
b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java
index 956b5aba3..f619c7783 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java
@@ -845,7 +845,7 @@ public class SftpSubsystem
this, file, SftpConstants.SSH_FXP_FSTAT, "", true);
boolean followLinks =
resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_FSTAT, handle, file);
- return resolveFileAttributes(file, flags, followLinks, options);
+ return resolveFileAttributes(file, flags, !followLinks, options);
}
@Override
diff --git
a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpVersionsTest.java
b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpVersionsTest.java
index 1a8ad9c98..f43f6b502 100644
--- a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpVersionsTest.java
+++ b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpVersionsTest.java
@@ -49,6 +49,7 @@ import org.apache.sshd.client.session.ClientSession;
import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.MapEntryUtils;
import org.apache.sshd.common.util.MapEntryUtils.NavigableMapBuilder;
+import org.apache.sshd.common.util.OsUtils;
import org.apache.sshd.common.util.io.IoUtils;
import org.apache.sshd.server.channel.ChannelSession;
import org.apache.sshd.server.command.Command;
@@ -70,6 +71,7 @@ import org.apache.sshd.sftp.server.SftpSubsystem;
import org.apache.sshd.sftp.server.SftpSubsystemEnvironment;
import org.apache.sshd.sftp.server.SftpSubsystemFactory;
import org.apache.sshd.util.test.CommonTestSupportUtils;
+import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.MethodOrderer.MethodName;
import org.junit.jupiter.api.TestMethodOrder;
import org.junit.jupiter.params.ParameterizedTest;
@@ -569,6 +571,91 @@ public class SftpVersionsTest extends
AbstractSftpClientTestSupport {
}
}
+ @MethodSource("parameters")
+ @ParameterizedTest(name = "version={0}")
+ public void sftpRemoveFileWithSymlinkInPath(int version) throws Exception {
+ Assumptions.assumeFalse(OsUtils.isWin32(), "Symlinks not reliably
supported on Windows");
+
+ initSftpVersionsTest(version);
+ Path targetPath = detectTargetFolder();
+ Path lclSftp = CommonTestSupportUtils.resolve(
+ targetPath,
+ SftpConstants.SFTP_SUBSYSTEM_NAME,
+ getClass().getSimpleName());
+ Path lclParent = assertHierarchyTargetFolderExists(lclSftp);
+ String testFileName = "test-file.txt";
+
+ Path actualDir = lclParent.resolve("actual-dir-" + getTestedVersion());
+ CommonTestSupportUtils.deleteRecursive(actualDir);
+ Files.createDirectories(actualDir);
+
+ Path symlinkDir = lclParent.resolve("symlink-dir-" +
getTestedVersion());
+ Files.deleteIfExists(symlinkDir);
+ Files.createSymbolicLink(symlinkDir, actualDir);
+
+ Path fileViaSymlink = symlinkDir.resolve(testFileName);
+ Files.write(fileViaSymlink, "test
content".getBytes(StandardCharsets.UTF_8));
+
+ assertTrue(Files.exists(fileViaSymlink), "File should exist via
symlink path");
+ assertTrue(Files.exists(actualDir.resolve(testFileName)), "File should
exist in actual directory");
+
+ Path parentPath = targetPath.getParent();
+ String remotePath =
CommonTestSupportUtils.resolveRelativeRemotePath(parentPath, fileViaSymlink);
+
+ try (ClientSession session = createAuthenticatedClientSession();
+ SftpClient sftp = createSftpClient(session, getTestedVersion())) {
+ sftp.remove(remotePath);
+
+ assertFalse(Files.exists(fileViaSymlink), "File should be deleted
via symlink path");
+ assertFalse(Files.exists(actualDir.resolve(testFileName)), "File
should be deleted from actual directory");
+ } finally {
+ Files.deleteIfExists(symlinkDir);
+ CommonTestSupportUtils.deleteRecursive(actualDir);
+ }
+ }
+
+ @MethodSource("parameters")
+ @ParameterizedTest(name = "version={0}")
+ public void sftpFStatWithSymlinkInPath(int version) throws Exception {
+ Assumptions.assumeFalse(OsUtils.isWin32(), "Symlinks not reliably
supported on Windows");
+
+ initSftpVersionsTest(version);
+ Path targetPath = detectTargetFolder();
+ Path lclSftp = CommonTestSupportUtils.resolve(targetPath,
+ SftpConstants.SFTP_SUBSYSTEM_NAME, getClass().getSimpleName());
+ Path lclParent = assertHierarchyTargetFolderExists(lclSftp);
+
+ Path actualDir = lclParent.resolve("actual-fstat-" +
getTestedVersion());
+ CommonTestSupportUtils.deleteRecursive(actualDir);
+ Files.createDirectories(actualDir);
+
+ Path symlinkDir = lclParent.resolve("symlink-fstat-" +
getTestedVersion());
+ Files.deleteIfExists(symlinkDir);
+ Files.createSymbolicLink(symlinkDir, actualDir);
+
+ Path testFile = symlinkDir.resolve("test.txt");
+ String content = getCurrentTestName();
+ Files.write(testFile, content.getBytes(StandardCharsets.UTF_8));
+
+ Path parentPath = targetPath.getParent();
+ String remotePath =
CommonTestSupportUtils.resolveRelativeRemotePath(parentPath, testFile);
+
+ try (ClientSession session = createAuthenticatedClientSession();
+ SftpClient sftp = createSftpClient(session, getTestedVersion())) {
+ try (SftpClient.CloseableHandle handle = sftp.open(remotePath,
SftpClient.OpenMode.Read)) {
+ Attributes attrs = sftp.stat(handle);
+
+ assertNotNull(attrs, "Attributes should be retrieved");
+ assertEquals(content.length(), attrs.getSize(), "File size
mismatch");
+ assertTrue(attrs.isRegularFile(), "Should be a regular file");
+ }
+ } finally {
+ Files.deleteIfExists(testFile);
+ Files.deleteIfExists(symlinkDir);
+ CommonTestSupportUtils.deleteRecursive(actualDir);
+ }
+ }
+
@Override
public String toString() {
return getClass().getSimpleName() + "[" + getTestedVersion() + "]";
