Skip to content

Commit 794122f

Browse files
authored
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.
1 parent 2bb67e6 commit 794122f

3 files changed

Lines changed: 89 additions & 2 deletions

File tree

sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1717,7 +1717,7 @@ protected void doRemoveFile(int id, String path) throws IOException {
17171717
LinkOption[] options = accessor.resolveFileAccessLinkOptions(
17181718
this, resolvedPath, SftpConstants.SSH_FXP_REMOVE, "", false);
17191719
WithFileAttributeCache.withAttributeCache(resolvedPath, p -> {
1720-
Boolean status = checkSymlinkState(p, followLinks, options);
1720+
Boolean status = checkSymlinkState(p, !followLinks, options);
17211721
if (status == null) {
17221722
throw signalRemovalPreConditionFailure(id, path, p,
17231723
new AccessDeniedException(p.toString(), p.toString(), "Cannot determine existence of remove candidate"),

sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -845,7 +845,7 @@ protected Map<String, Object> doFStat(int id, String handle, int flags) throws I
845845
this, file, SftpConstants.SSH_FXP_FSTAT, "", true);
846846

847847
boolean followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_FSTAT, handle, file);
848-
return resolveFileAttributes(file, flags, followLinks, options);
848+
return resolveFileAttributes(file, flags, !followLinks, options);
849849
}
850850

851851
@Override

sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpVersionsTest.java

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import org.apache.sshd.common.util.GenericUtils;
5050
import org.apache.sshd.common.util.MapEntryUtils;
5151
import org.apache.sshd.common.util.MapEntryUtils.NavigableMapBuilder;
52+
import org.apache.sshd.common.util.OsUtils;
5253
import org.apache.sshd.common.util.io.IoUtils;
5354
import org.apache.sshd.server.channel.ChannelSession;
5455
import org.apache.sshd.server.command.Command;
@@ -70,6 +71,7 @@
7071
import org.apache.sshd.sftp.server.SftpSubsystemEnvironment;
7172
import org.apache.sshd.sftp.server.SftpSubsystemFactory;
7273
import org.apache.sshd.util.test.CommonTestSupportUtils;
74+
import org.junit.jupiter.api.Assumptions;
7375
import org.junit.jupiter.api.MethodOrderer.MethodName;
7476
import org.junit.jupiter.api.TestMethodOrder;
7577
import org.junit.jupiter.params.ParameterizedTest;
@@ -569,6 +571,91 @@ public void endOfListIndicator(int version) throws Exception {
569571
}
570572
}
571573

574+
@MethodSource("parameters")
575+
@ParameterizedTest(name = "version={0}")
576+
public void sftpRemoveFileWithSymlinkInPath(int version) throws Exception {
577+
Assumptions.assumeFalse(OsUtils.isWin32(), "Symlinks not reliably supported on Windows");
578+
579+
initSftpVersionsTest(version);
580+
Path targetPath = detectTargetFolder();
581+
Path lclSftp = CommonTestSupportUtils.resolve(
582+
targetPath,
583+
SftpConstants.SFTP_SUBSYSTEM_NAME,
584+
getClass().getSimpleName());
585+
Path lclParent = assertHierarchyTargetFolderExists(lclSftp);
586+
String testFileName = "test-file.txt";
587+
588+
Path actualDir = lclParent.resolve("actual-dir-" + getTestedVersion());
589+
CommonTestSupportUtils.deleteRecursive(actualDir);
590+
Files.createDirectories(actualDir);
591+
592+
Path symlinkDir = lclParent.resolve("symlink-dir-" + getTestedVersion());
593+
Files.deleteIfExists(symlinkDir);
594+
Files.createSymbolicLink(symlinkDir, actualDir);
595+
596+
Path fileViaSymlink = symlinkDir.resolve(testFileName);
597+
Files.write(fileViaSymlink, "test content".getBytes(StandardCharsets.UTF_8));
598+
599+
assertTrue(Files.exists(fileViaSymlink), "File should exist via symlink path");
600+
assertTrue(Files.exists(actualDir.resolve(testFileName)), "File should exist in actual directory");
601+
602+
Path parentPath = targetPath.getParent();
603+
String remotePath = CommonTestSupportUtils.resolveRelativeRemotePath(parentPath, fileViaSymlink);
604+
605+
try (ClientSession session = createAuthenticatedClientSession();
606+
SftpClient sftp = createSftpClient(session, getTestedVersion())) {
607+
sftp.remove(remotePath);
608+
609+
assertFalse(Files.exists(fileViaSymlink), "File should be deleted via symlink path");
610+
assertFalse(Files.exists(actualDir.resolve(testFileName)), "File should be deleted from actual directory");
611+
} finally {
612+
Files.deleteIfExists(symlinkDir);
613+
CommonTestSupportUtils.deleteRecursive(actualDir);
614+
}
615+
}
616+
617+
@MethodSource("parameters")
618+
@ParameterizedTest(name = "version={0}")
619+
public void sftpFStatWithSymlinkInPath(int version) throws Exception {
620+
Assumptions.assumeFalse(OsUtils.isWin32(), "Symlinks not reliably supported on Windows");
621+
622+
initSftpVersionsTest(version);
623+
Path targetPath = detectTargetFolder();
624+
Path lclSftp = CommonTestSupportUtils.resolve(targetPath,
625+
SftpConstants.SFTP_SUBSYSTEM_NAME, getClass().getSimpleName());
626+
Path lclParent = assertHierarchyTargetFolderExists(lclSftp);
627+
628+
Path actualDir = lclParent.resolve("actual-fstat-" + getTestedVersion());
629+
CommonTestSupportUtils.deleteRecursive(actualDir);
630+
Files.createDirectories(actualDir);
631+
632+
Path symlinkDir = lclParent.resolve("symlink-fstat-" + getTestedVersion());
633+
Files.deleteIfExists(symlinkDir);
634+
Files.createSymbolicLink(symlinkDir, actualDir);
635+
636+
Path testFile = symlinkDir.resolve("test.txt");
637+
String content = getCurrentTestName();
638+
Files.write(testFile, content.getBytes(StandardCharsets.UTF_8));
639+
640+
Path parentPath = targetPath.getParent();
641+
String remotePath = CommonTestSupportUtils.resolveRelativeRemotePath(parentPath, testFile);
642+
643+
try (ClientSession session = createAuthenticatedClientSession();
644+
SftpClient sftp = createSftpClient(session, getTestedVersion())) {
645+
try (SftpClient.CloseableHandle handle = sftp.open(remotePath, SftpClient.OpenMode.Read)) {
646+
Attributes attrs = sftp.stat(handle);
647+
648+
assertNotNull(attrs, "Attributes should be retrieved");
649+
assertEquals(content.length(), attrs.getSize(), "File size mismatch");
650+
assertTrue(attrs.isRegularFile(), "Should be a regular file");
651+
}
652+
} finally {
653+
Files.deleteIfExists(testFile);
654+
Files.deleteIfExists(symlinkDir);
655+
CommonTestSupportUtils.deleteRecursive(actualDir);
656+
}
657+
}
658+
572659
@Override
573660
public String toString() {
574661
return getClass().getSimpleName() + "[" + getTestedVersion() + "]";

0 commit comments

Comments
 (0)