From ff5b8da9295d8e76697166868b70bceb5b1df50a Mon Sep 17 00:00:00 2001 From: Vaishnavi Kumbhar Date: Fri, 27 Mar 2026 13:01:51 +0530 Subject: [PATCH 1/3] Add SFTP password authentication tests for Commons VFS2 --- .../provider/sftp/SftpPasswordAuthTest.java | 247 ++++++++++++++++++ 1 file changed, 247 insertions(+) create mode 100644 commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpPasswordAuthTest.java diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpPasswordAuthTest.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpPasswordAuthTest.java new file mode 100644 index 0000000000..9247964852 --- /dev/null +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpPasswordAuthTest.java @@ -0,0 +1,247 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.commons.vfs2.provider.sftp; + +import static org.apache.commons.vfs2.VfsTestUtils.getTestDirectoryFile; +import static org.junit.jupiter.api.Assertions.*; + +import java.io.File; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.time.Duration; +import java.util.ArrayList; +import java.util.List; + +import org.apache.commons.vfs2.*; +import org.apache.commons.vfs2.auth.StaticUserAuthenticator; +import org.apache.commons.vfs2.impl.DefaultFileSystemConfigBuilder; +import org.apache.commons.vfs2.impl.DefaultFileSystemManager; +import org.apache.commons.vfs2.provider.local.DefaultLocalFileProvider; +import org.apache.sshd.SshServer; +import org.apache.sshd.common.KeyPairProvider; +import org.apache.sshd.common.NamedFactory; +import org.apache.sshd.common.Session; +import org.apache.sshd.server.Command; +import org.apache.sshd.server.FileSystemFactory; +import org.apache.sshd.server.FileSystemView; +import org.apache.sshd.server.SshFile; +import org.apache.sshd.server.filesystem.NativeSshFile; +import org.apache.sshd.server.sftp.SftpSubsystem; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import com.jcraft.jsch.TestIdentityRepositoryFactory; + +/** + * Tests SFTP password authentication using {@link StaticUserAuthenticator}. + *

+ * Verifies that credentials supplied via {@link DefaultFileSystemConfigBuilder#setUserAuthenticator} + * are correctly propagated to the SFTP server. + *

+ *

+ * Uses SSHD 0.8.0 with an explicit RSA {@link KeyPairProvider} for Java 17 compatibility + * (the default DSA key generation is disabled on modern JDKs). + *

+ */ +public class SftpPasswordAuthTest { + + private static final String TEST_USERNAME = "testuser"; + private static final String TEST_PASSWORD = "testpass"; + + private static SshServer sshServer; + private static int serverPort; + private static DefaultFileSystemManager manager; + + @BeforeAll + static void setUp() throws Exception { + sshServer = SshServer.setUpDefaultServer(); + sshServer.setPort(0); + + final KeyPairGenerator hostKeyGen = KeyPairGenerator.getInstance("RSA"); + hostKeyGen.initialize(2048); + final KeyPair hostKey = hostKeyGen.generateKeyPair(); + sshServer.setKeyPairProvider(new KeyPairProvider() { + @Override + public KeyPair loadKey(final String type) { + return KeyPairProvider.SSH_RSA.equals(type) ? hostKey : null; + } + @Override + public String getKeyTypes() { + return KeyPairProvider.SSH_RSA; + } + }); + + sshServer.setPasswordAuthenticator( + (user, pass, session) -> TEST_USERNAME.equals(user) && TEST_PASSWORD.equals(pass)); + + final List> subsystems = new ArrayList<>(); + subsystems.add(new NamedFactory() { + @Override + public Command create() { return new SftpSubsystem(); } + @Override + public String getName() { return "sftp"; } + }); + sshServer.setSubsystemFactories(subsystems); + + sshServer.setFileSystemFactory(new TestFileSystemFactory()); + sshServer.start(); + + serverPort = sshServer.getPort(); + + manager = new DefaultFileSystemManager(); + manager.addProvider("sftp", new SftpFileProvider()); + manager.addProvider("file", new DefaultLocalFileProvider()); + manager.init(); + } + + @AfterAll + static void tearDown() throws Exception { + if (manager != null) { + try { + manager.close(); + } catch (final Exception e) { + // ignore + } + } + if (sshServer != null) { + stopServerWithTimeout(5000); + } + } + + private static void stopServerWithTimeout(final long timeoutMs) { + final Thread stopThread = new Thread(() -> { + try { + sshServer.stop(true); + } catch (final Exception e) { + // ignore + } + }, "sshd-stop"); + stopThread.setDaemon(true); + stopThread.start(); + try { + stopThread.join(timeoutMs); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + + private static String baseUri() { + return String.format("sftp://%s@localhost:%d", TEST_USERNAME, serverPort); + } + + private FileSystemOptions authOptions() throws FileSystemException { + final FileSystemOptions opts = new FileSystemOptions(); + final StaticUserAuthenticator auth = new StaticUserAuthenticator(null, TEST_USERNAME, TEST_PASSWORD); + DefaultFileSystemConfigBuilder.getInstance().setUserAuthenticator(opts, auth); + configureSftpOptions(opts); + return opts; + } + + private static void configureSftpOptions(final FileSystemOptions opts) throws FileSystemException { + final SftpFileSystemConfigBuilder builder = SftpFileSystemConfigBuilder.getInstance(); + builder.setStrictHostKeyChecking(opts, "no"); + builder.setUserInfo(opts, new TrustEveryoneUserInfo()); + builder.setIdentityRepositoryFactory(opts, new TestIdentityRepositoryFactory()); + builder.setConnectTimeout(opts, Duration.ofSeconds(60)); + builder.setSessionTimeout(opts, Duration.ofSeconds(60)); + } + + @Test + void testResolveFile() throws FileSystemException { + final FileSystemOptions opts = authOptions(); + try (FileObject file = manager.resolveFile(baseUri() + "/read-tests/file1.txt", opts)) { + assertNotNull(file); + assertTrue(file.exists(), "file1.txt should exist"); + assertEquals(FileType.FILE, file.getType()); + assertNotNull(file.getContent(), "Content should be readable when credentials are correct"); + } + } + + @Test + void testResolveFolder() throws FileSystemException { + final FileSystemOptions opts = authOptions(); + try (FileObject folder = manager.resolveFile(baseUri() + "/read-tests", opts)) { + assertNotNull(folder); + assertTrue(folder.exists(), "read-tests folder should exist"); + } + } + + @Test + void testResolveFolderWithTrailingSlash() throws FileSystemException { + final FileSystemOptions opts = authOptions(); + try (FileObject folder = manager.resolveFile(baseUri() + "/read-tests/", opts)) { + assertNotNull(folder); + assertTrue(folder.exists(), "read-tests/ folder should exist"); + } + } + + @Test + void testWrongCredentialsThrowsException() throws FileSystemException { + final FileSystemOptions opts = new FileSystemOptions(); + final StaticUserAuthenticator auth = new StaticUserAuthenticator(null, "wronguser", "wrongpassword"); + DefaultFileSystemConfigBuilder.getInstance().setUserAuthenticator(opts, auth); + configureSftpOptions(opts); + + final String wrongUserUri = String.format("sftp://wronguser@localhost:%d/read-tests/file1.txt", serverPort); + assertThrows(FileSystemException.class, () -> { + try (FileObject file = manager.resolveFile(wrongUserUri, opts)) { + file.exists(); + } + }, "Expected FileSystemException when accessing a resource with wrong credentials"); + } + + private static class TestFileSystemFactory implements FileSystemFactory { + @Override + public FileSystemView createFileSystemView(final Session session) { + final String home = getTestDirectoryFile().getAbsolutePath(); + final String user = session.getUsername(); + return new FileSystemView() { + @Override + public SshFile getFile(final SshFile baseDir, final String file) { + return getFile(baseDir.getAbsolutePath(), file); + } + @Override + public SshFile getFile(final String file) { + return getFile(home, file); + } + private SshFile getFile(final String dir, final String file) { + final String normalized = NativeSshFile.normalizeSeparateChar(file); + final String homeNorm = NativeSshFile.normalizeSeparateChar(home); + final String prefix = removePrefix(homeNorm); + String userFile = removePrefix(normalized); + final File f = userFile.startsWith(prefix) + ? new File(userFile) + : new File(prefix, userFile); + userFile = removePrefix(NativeSshFile.normalizeSeparateChar(f.getAbsolutePath())); + return new AccessibleNativeSshFile(userFile, f, user); + } + private String removePrefix(final String s) { + final int idx = s.indexOf('/'); + return idx < 1 ? s : s.substring(idx); + } + }; + } + } + + private static class AccessibleNativeSshFile extends NativeSshFile { + AccessibleNativeSshFile(final String fileName, final File file, final String userName) { + super(fileName, file, userName); + } + } +} From 2c12e7402c141cbadece05b7dd795edc653fc1ea Mon Sep 17 00:00:00 2001 From: Vaishnavi Kumbhar Date: Thu, 23 Apr 2026 12:03:48 +0530 Subject: [PATCH 2/3] Migrate SFTP test infrastructure from Apache MINA SSHD 0.x to 3.x API --- commons-vfs2/pom.xml | 6 +- .../provider/sftp/SftpPasswordAuthTest.java | 109 ++--- .../provider/sftp/SftpTestServerHelper.java | 418 ++++++------------ pom.xml | 12 +- 4 files changed, 167 insertions(+), 378 deletions(-) diff --git a/commons-vfs2/pom.xml b/commons-vfs2/pom.xml index 319e975638..29ffffecad 100644 --- a/commons-vfs2/pom.xml +++ b/commons-vfs2/pom.xml @@ -116,15 +116,15 @@ log4j-core test - + org.apache.sshd sshd-core test - org.bouncycastle - bcprov-jdk16 + org.apache.sshd + sshd-sftp test diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpPasswordAuthTest.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpPasswordAuthTest.java index 9247964852..0408630cbf 100644 --- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpPasswordAuthTest.java +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpPasswordAuthTest.java @@ -18,30 +18,29 @@ package org.apache.commons.vfs2.provider.sftp; import static org.apache.commons.vfs2.VfsTestUtils.getTestDirectoryFile; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.File; -import java.security.KeyPair; -import java.security.KeyPairGenerator; +import java.nio.file.Files; +import java.nio.file.Path; import java.time.Duration; -import java.util.ArrayList; -import java.util.List; +import java.util.Collections; -import org.apache.commons.vfs2.*; +import org.apache.commons.vfs2.FileObject; +import org.apache.commons.vfs2.FileSystemException; +import org.apache.commons.vfs2.FileSystemOptions; +import org.apache.commons.vfs2.FileType; import org.apache.commons.vfs2.auth.StaticUserAuthenticator; import org.apache.commons.vfs2.impl.DefaultFileSystemConfigBuilder; import org.apache.commons.vfs2.impl.DefaultFileSystemManager; import org.apache.commons.vfs2.provider.local.DefaultLocalFileProvider; -import org.apache.sshd.SshServer; -import org.apache.sshd.common.KeyPairProvider; -import org.apache.sshd.common.NamedFactory; -import org.apache.sshd.common.Session; -import org.apache.sshd.server.Command; -import org.apache.sshd.server.FileSystemFactory; -import org.apache.sshd.server.FileSystemView; -import org.apache.sshd.server.SshFile; -import org.apache.sshd.server.filesystem.NativeSshFile; -import org.apache.sshd.server.sftp.SftpSubsystem; +import org.apache.sshd.common.file.virtualfs.VirtualFileSystemFactory; +import org.apache.sshd.server.SshServer; +import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider; +import org.apache.sshd.sftp.server.SftpSubsystemFactory; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -54,10 +53,6 @@ * Verifies that credentials supplied via {@link DefaultFileSystemConfigBuilder#setUserAuthenticator} * are correctly propagated to the SFTP server. *

- *

- * Uses SSHD 0.8.0 with an explicit RSA {@link KeyPairProvider} for Java 17 compatibility - * (the default DSA key generation is disabled on modern JDKs). - *

*/ public class SftpPasswordAuthTest { @@ -73,35 +68,22 @@ static void setUp() throws Exception { sshServer = SshServer.setUpDefaultServer(); sshServer.setPort(0); - final KeyPairGenerator hostKeyGen = KeyPairGenerator.getInstance("RSA"); - hostKeyGen.initialize(2048); - final KeyPair hostKey = hostKeyGen.generateKeyPair(); - sshServer.setKeyPairProvider(new KeyPairProvider() { - @Override - public KeyPair loadKey(final String type) { - return KeyPairProvider.SSH_RSA.equals(type) ? hostKey : null; - } - @Override - public String getKeyTypes() { - return KeyPairProvider.SSH_RSA; - } - }); + final Path tmpKeyFile = Files.createTempFile("sshd-test-key", ".ser"); + tmpKeyFile.toFile().deleteOnExit(); + final SimpleGeneratorHostKeyProvider keyProvider = new SimpleGeneratorHostKeyProvider(tmpKeyFile); + keyProvider.setAlgorithm("RSA"); + sshServer.setKeyPairProvider(keyProvider); sshServer.setPasswordAuthenticator( (user, pass, session) -> TEST_USERNAME.equals(user) && TEST_PASSWORD.equals(pass)); - final List> subsystems = new ArrayList<>(); - subsystems.add(new NamedFactory() { - @Override - public Command create() { return new SftpSubsystem(); } - @Override - public String getName() { return "sftp"; } - }); - sshServer.setSubsystemFactories(subsystems); + sshServer.setSubsystemFactories(Collections.singletonList(new SftpSubsystemFactory())); - sshServer.setFileSystemFactory(new TestFileSystemFactory()); - sshServer.start(); + final File homeDir = getTestDirectoryFile(); + sshServer.setFileSystemFactory( + new VirtualFileSystemFactory(homeDir.toPath().toAbsolutePath())); + sshServer.start(); serverPort = sshServer.getPort(); manager = new DefaultFileSystemManager(); @@ -127,7 +109,7 @@ static void tearDown() throws Exception { private static void stopServerWithTimeout(final long timeoutMs) { final Thread stopThread = new Thread(() -> { try { - sshServer.stop(true); + sshServer.close(); } catch (final Exception e) { // ignore } @@ -205,43 +187,4 @@ void testWrongCredentialsThrowsException() throws FileSystemException { } }, "Expected FileSystemException when accessing a resource with wrong credentials"); } - - private static class TestFileSystemFactory implements FileSystemFactory { - @Override - public FileSystemView createFileSystemView(final Session session) { - final String home = getTestDirectoryFile().getAbsolutePath(); - final String user = session.getUsername(); - return new FileSystemView() { - @Override - public SshFile getFile(final SshFile baseDir, final String file) { - return getFile(baseDir.getAbsolutePath(), file); - } - @Override - public SshFile getFile(final String file) { - return getFile(home, file); - } - private SshFile getFile(final String dir, final String file) { - final String normalized = NativeSshFile.normalizeSeparateChar(file); - final String homeNorm = NativeSshFile.normalizeSeparateChar(home); - final String prefix = removePrefix(homeNorm); - String userFile = removePrefix(normalized); - final File f = userFile.startsWith(prefix) - ? new File(userFile) - : new File(prefix, userFile); - userFile = removePrefix(NativeSshFile.normalizeSeparateChar(f.getAbsolutePath())); - return new AccessibleNativeSshFile(userFile, f, user); - } - private String removePrefix(final String s) { - final int idx = s.indexOf('/'); - return idx < 1 ? s : s.substring(idx); - } - }; - } - } - - private static class AccessibleNativeSshFile extends NativeSshFile { - AccessibleNativeSshFile(final String fileName, final File file, final String userName) { - super(fileName, file, userName); - } - } } diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpTestServerHelper.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpTestServerHelper.java index 8e000614c4..3585c73444 100644 --- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpTestServerHelper.java +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpTestServerHelper.java @@ -19,39 +19,41 @@ import static org.apache.commons.vfs2.VfsTestUtils.getTestDirectoryFile; import java.io.File; -import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.net.InetSocketAddress; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermission; +import java.security.Principal; import java.util.ArrayList; +import java.util.Collections; +import java.util.EnumSet; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; -import java.util.TreeMap; +import java.util.Map; +import java.util.NavigableMap; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import org.apache.commons.io.file.PathUtils; import org.apache.commons.lang3.Strings; -import org.apache.sshd.SshServer; -import org.apache.sshd.common.NamedFactory; -import org.apache.sshd.common.Session; -import org.apache.sshd.common.session.AbstractSession; -import org.apache.sshd.common.util.Buffer; -import org.apache.sshd.common.util.SecurityUtils; -import org.apache.sshd.server.Command; +import org.apache.sshd.common.file.virtualfs.VirtualFileSystemFactory; import org.apache.sshd.server.Environment; import org.apache.sshd.server.ExitCallback; -import org.apache.sshd.server.FileSystemFactory; -import org.apache.sshd.server.FileSystemView; -import org.apache.sshd.server.ForwardingFilter; -import org.apache.sshd.server.SshFile; -import org.apache.sshd.server.auth.UserAuthNone; -import org.apache.sshd.server.command.ScpCommandFactory; -import org.apache.sshd.server.filesystem.NativeSshFile; -import org.apache.sshd.server.keyprovider.PEMGeneratorHostKeyProvider; +import org.apache.sshd.server.SshServer; +import org.apache.sshd.server.auth.UserAuthFactory; +import org.apache.sshd.server.auth.UserAuthNoneFactory; +import org.apache.sshd.server.auth.pubkey.AcceptAllPublickeyAuthenticator; +import org.apache.sshd.server.channel.ChannelSession; +import org.apache.sshd.server.command.Command; +import org.apache.sshd.server.command.CommandFactory; import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider; -import org.apache.sshd.server.session.ServerSession; -import org.apache.sshd.server.sftp.SftpSubsystem; +import org.apache.sshd.sftp.server.SftpFileSystemAccessor; +import org.apache.sshd.sftp.server.SftpSubsystemFactory; +import org.apache.sshd.sftp.server.SftpSubsystemProxy; /** * Helper class to start and stop an embedded Apache SSHd (MINA) server for SFTP testing. @@ -59,259 +61,145 @@ public final class SftpTestServerHelper { private static final String DEFAULT_USER = "testuser"; + private static final int TEST_UID = 1000; + private static final int TEST_GID = 1000; private static SshServer server; private static String connectionUri; private SftpTestServerHelper() { - // Utility class } /** - * Custom SFTP subsystem that handles permissions. + * Custom {@link SftpFileSystemAccessor} that tracks POSIX permissions in memory so that + * {@code setExecutable}/{@code setReadable}/{@code setWritable} round-trip correctly on + * platforms without native POSIX support (e.g. Windows). */ - private static class MySftpSubsystem extends SftpSubsystem { - // Static permissions cache shared across all SFTP subsystem instances - private static final TreeMap permissions = new TreeMap<>(); - private int _version; + private static class TestSftpFileSystemAccessor implements SftpFileSystemAccessor { + private static final ConcurrentHashMap> permissionsCache = + new ConcurrentHashMap<>(); @Override - protected void process(final Buffer buffer) throws IOException { - final int rpos = buffer.rpos(); - final int length = buffer.getInt(); - final int type = buffer.getByte(); - final int id = buffer.getInt(); - - switch (type) { - case SSH_FXP_SETSTAT: - case SSH_FXP_FSETSTAT: { - // Get the path - final String path = buffer.getString(); - // Get the permission - final SftpAttrs attrs = new SftpAttrs(buffer); - synchronized (permissions) { - permissions.put(path, attrs.permissions); - } - break; - } - - case SSH_FXP_REMOVE: { - // Remove cached attributes - final String path = buffer.getString(); - synchronized (permissions) { - permissions.remove(path); - } - break; - } - - case SSH_FXP_INIT: { - // Just grab the version here - _version = id; - break; - } + public void setFilePermissions( + final SftpSubsystemProxy subsystem, final Path file, + final Set perms, final LinkOption... options) throws IOException { + permissionsCache.put(file.toAbsolutePath().toString(), new HashSet<>(perms)); + try { + SftpFileSystemAccessor.super.setFilePermissions(subsystem, file, perms, options); + } catch (final UnsupportedOperationException | IOException ignored) { + // Silently swallow on systems without native POSIX support } - - buffer.rpos(rpos); - super.process(buffer); } @Override - protected void writeAttrs(final Buffer buffer, final SshFile file, final int flags) throws IOException { - if (!file.doesExist()) { - throw new FileNotFoundException(file.getAbsolutePath()); - } - - int p = 0; - - final Integer cached; - synchronized (permissions) { - cached = permissions.get(file.getAbsolutePath()); - } - if (cached != null) { - // Use cached permissions - p |= cached; - } else { - // Default permissions for testing: always readable and writable - // This ensures tests work regardless of underlying file system permissions - p |= S_IRUSR | S_IWUSR; - - // For directories, always add execute bit (needed to traverse/access the directory) - // For files, add execute bit only if the file is actually executable - if (file.isDirectory() || file.isExecutable()) { - p |= S_IXUSR; - } - } - - if (_version >= 4) { - if (file.isFile()) { - buffer.putInt(SSH_FILEXFER_ATTR_PERMISSIONS); - buffer.putByte((byte) SSH_FILEXFER_TYPE_REGULAR); - buffer.putInt(p); - } else if (file.isDirectory()) { - buffer.putInt(SSH_FILEXFER_ATTR_PERMISSIONS); - buffer.putByte((byte) SSH_FILEXFER_TYPE_DIRECTORY); - buffer.putInt(p); - } else { - buffer.putInt(0); - buffer.putByte((byte) SSH_FILEXFER_TYPE_UNKNOWN); - } - } else { - if (file.isFile()) { - p |= 0100000; - } - if (file.isDirectory()) { - p |= 0040000; - } - - // SSH_FILEXFER_ATTR_UIDGID constant doesn't exist in SSHD 0.8.0, use literal value 0x00000002 - final int SSH_FILEXFER_ATTR_UIDGID = 0x00000002; - // Use UID/GID 1000 to match what TestCommandFactory returns for "id -u" and "id -G" - final int TEST_UID = 1000; - final int TEST_GID = 1000; - - if (file.isFile()) { - buffer.putInt(SSH_FILEXFER_ATTR_SIZE | SSH_FILEXFER_ATTR_UIDGID | SSH_FILEXFER_ATTR_PERMISSIONS | SSH_FILEXFER_ATTR_ACMODTIME); - buffer.putLong(file.getSize()); - buffer.putInt(TEST_UID); // UID - matches "id -u" output - buffer.putInt(TEST_GID); // GID - matches "id -G" output - buffer.putInt(p); - buffer.putInt(file.getLastModified() / 1000); - buffer.putInt(file.getLastModified() / 1000); - } else if (file.isDirectory()) { - buffer.putInt(SSH_FILEXFER_ATTR_UIDGID | SSH_FILEXFER_ATTR_PERMISSIONS | SSH_FILEXFER_ATTR_ACMODTIME); - buffer.putInt(TEST_UID); // UID - matches "id -u" output - buffer.putInt(TEST_GID); // GID - matches "id -G" output - buffer.putInt(p); - buffer.putInt(file.getLastModified() / 1000); - buffer.putInt(file.getLastModified() / 1000); - } else { - buffer.putInt(0); - } - } + public void setFileOwner( + final SftpSubsystemProxy subsystem, final Path file, + final Principal owner, final LinkOption... options) throws IOException { + // No-op: the test server always reports TEST_UID/TEST_GID via + // resolveReportedFileAttributes, so the client sends those back + // in setStat. Silently ignore to avoid chown failures on + // non-root Unix processes. } - } - /** - * SFTP attributes helper class. - */ - private static class SftpAttrs { - int flags; - long size; - int permissions; - - SftpAttrs(final Buffer buffer) { - final int flags = buffer.getInt(); - this.flags = flags; - if ((flags & SftpSubsystem.SSH_FILEXFER_ATTR_SIZE) != 0) { - size = buffer.getLong(); - } - // Note: SSH_FILEXFER_ATTR_UIDGID constant doesn't exist in SSHD 0.8.0 - // Skip UID/GID if present (flag value is 0x00000002) - if ((flags & 0x00000002) != 0) { - buffer.getInt(); // uid - buffer.getInt(); // gid - } - if ((flags & SftpSubsystem.SSH_FILEXFER_ATTR_PERMISSIONS) != 0) { - permissions = buffer.getInt(); - } - if ((flags & SftpSubsystem.SSH_FILEXFER_ATTR_ACMODTIME) != 0) { - buffer.getInt(); // atime - buffer.getInt(); // mtime - } + @Override + public void setGroupOwner( + final SftpSubsystemProxy subsystem, final Path file, + final Principal group, final LinkOption... options) throws IOException { + // No-op: same rationale as setFileOwner. } - } - /** - * Simple file system factory that maps users to directories. - */ - private static class TestFileSystemFactory implements FileSystemFactory { @Override - public FileSystemView createFileSystemView(final Session session) throws IOException { - final String userName = session.getUsername(); - if (!DEFAULT_USER.equals(userName)) { - return null; + public Map readFileAttributes( + final SftpSubsystemProxy subsystem, final Path file, + final String view, final LinkOption... options) throws IOException { + try { + return SftpFileSystemAccessor.super.readFileAttributes(subsystem, file, view, options); + } catch (final UnsupportedOperationException | IllegalArgumentException e) { + if (view.startsWith("unix:") || view.startsWith("posix:")) { + final Map attrs = new HashMap<>( + Files.readAttributes(file, "basic:*", options)); + final Set cached = + permissionsCache.get(file.toAbsolutePath().toString()); + attrs.put("permissions", cached != null ? cached : defaultPermissions(file, options)); + attrs.put("uid", TEST_UID); + attrs.put("gid", TEST_GID); + return attrs; + } + throw e; } - final File homeDir = getTestDirectoryFile(); - return new TestFileSystemView(homeDir.getAbsolutePath(), userName); - } - } - - /** - * Simple file system view for testing. - */ - private static class TestFileSystemView implements FileSystemView { - private final String homeDirStr; - private final String userName; - - TestFileSystemView(final String homeDirStr, final String userName) { - this.homeDirStr = new File(homeDirStr).getAbsolutePath(); - this.userName = userName; } @Override - public SshFile getFile(final SshFile baseDir, final String file) { - return this.getFile(baseDir.getAbsolutePath(), file); + public NavigableMap resolveReportedFileAttributes( + final SftpSubsystemProxy subsystem, final Path file, final int flags, + final NavigableMap attrs, final LinkOption... options) throws IOException { + // Always override uid/gid to match TestCommandFactory's "id -u"/"id -G" responses, + // so that PosixPermissions owner checks are consistent across all platforms. + attrs.put("uid", TEST_UID); + attrs.put("gid", TEST_GID); + final Set cached = + permissionsCache.get(file.toAbsolutePath().toString()); + if (cached != null) { + attrs.put("permissions", cached); + } else if (!attrs.containsKey("permissions")) { + attrs.put("permissions", defaultPermissions(file, options)); + } + return attrs; } @Override - public SshFile getFile(final String file) { - return this.getFile(homeDirStr, file); + public void removeFile( + final SftpSubsystemProxy subsystem, final Path path, + final boolean isDirectory) throws IOException { + permissionsCache.remove(path.toAbsolutePath().toString()); + SftpFileSystemAccessor.super.removeFile(subsystem, path, isDirectory); } - protected SshFile getFile(final String dir, final String file) { - final String home = removePrefix(NativeSshFile.normalizeSeparateChar(homeDirStr)); - String userFileName = removePrefix(NativeSshFile.normalizeSeparateChar(file)); - final File sshFile = userFileName.startsWith(home) ? new File(userFileName) : new File(home, userFileName); - userFileName = removePrefix(NativeSshFile.normalizeSeparateChar(sshFile.getAbsolutePath())); - return new TestNativeSshFile(userFileName, sshFile, userName); - } - - private String removePrefix(final String s) { - final int index = s.indexOf('/'); - if (index < 1) { - return s; + private static Set defaultPermissions( + final Path file, final LinkOption... options) { + final Set perms = EnumSet.of( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE); + try { + if (Files.isDirectory(file, options)) { + perms.add(PosixFilePermission.OWNER_EXECUTE); + } + } catch (final Exception ignored) { + // ignore } - return s.substring(index); + return perms; } - } - /** - * Extends NativeSshFile for testing. - */ - private static class TestNativeSshFile extends NativeSshFile { - TestNativeSshFile(final String fileName, final File file, final String userName) { - super(fileName, file, userName); + static void clearCache() { + permissionsCache.clear(); } } /** - * Command factory for handling special commands. + * Command factory for handling special commands needed by VFS tests. */ - private static class TestCommandFactory extends ScpCommandFactory { + private static class TestCommandFactory implements CommandFactory { @Override - public Command createCommand(final String command) { - // Handle special commands for tests + public Command createCommand(final ChannelSession channel, final String command) throws IOException { if (command.startsWith("id -u")) { return createIdCommand("1000"); } if (command.startsWith("id -G")) { return createIdCommand("1000 1001 1002"); } - return super.createCommand(command); + throw new IOException("Unknown command: " + command); } private Command createIdCommand(final String output) { return new Command() { private ExitCallback callback; private OutputStream out; - private OutputStream err; @Override - public void destroy() { + public void destroy(final ChannelSession channel) throws Exception { } @Override public void setErrorStream(final OutputStream err) { - this.err = err; } @Override @@ -329,7 +217,7 @@ public void setOutputStream(final OutputStream out) { } @Override - public void start(final Environment env) throws IOException { + public void start(final ChannelSession channel, final Environment env) throws IOException { out.write((output + "\n").getBytes()); out.flush(); callback.onExit(0); @@ -345,80 +233,40 @@ public void start(final Environment env) throws IOException { */ public static synchronized void startServer() throws IOException { if (server != null) { - return; // Already started + return; } final Path tmpDir = PathUtils.getTempDirectory(); server = SshServer.setUpDefaultServer(); - server.setPort(0); // Use any available port - - // Set up key provider - if (SecurityUtils.isBouncyCastleRegistered()) { - final Path keyFile = Files.createTempFile(tmpDir, "key", ".pem"); - keyFile.toFile().deleteOnExit(); - Files.delete(keyFile); - final PEMGeneratorHostKeyProvider keyProvider = new PEMGeneratorHostKeyProvider(keyFile.toAbsolutePath().toString()); - server.setKeyPairProvider(keyProvider); - } else { - server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider(tmpDir.resolve("key.ser").toString())); - } - - // Set up SFTP subsystem - final List> list = new ArrayList<>(1); - list.add(new NamedFactory() { - @Override - public Command create() { - return new MySftpSubsystem(); - } - - @Override - public String getName() { - return "sftp"; - } - }); - server.setSubsystemFactories(list); - - // Set up authentication - server.setPasswordAuthenticator((username, password, session) -> Strings.CS.equals(username, password)); - server.setPublickeyAuthenticator((username, key, session) -> true); - - // Set up forwarding - server.setForwardingFilter(new ForwardingFilter() { - @Override - public boolean canConnect(final InetSocketAddress address, final ServerSession session) { - return true; - } + server.setPort(0); - @Override - public boolean canForwardAgent(final ServerSession session) { - return true; - } + final SimpleGeneratorHostKeyProvider keyProvider = + new SimpleGeneratorHostKeyProvider(tmpDir.resolve("hostkey.ser")); + keyProvider.setAlgorithm("RSA"); + server.setKeyPairProvider(keyProvider); - @Override - public boolean canForwardX11(final ServerSession session) { - return true; - } + final SftpSubsystemFactory sftpFactory = new SftpSubsystemFactory(); + sftpFactory.setFileSystemAccessor(new TestSftpFileSystemAccessor()); + server.setSubsystemFactories(Collections.singletonList(sftpFactory)); - @Override - public boolean canListen(final InetSocketAddress address, final ServerSession session) { - return true; - } - }); + server.setPasswordAuthenticator( + (username, password, session) -> Strings.CS.equals(username, password)); + server.setPublickeyAuthenticator(AcceptAllPublickeyAuthenticator.INSTANCE); - // Set up command factory - server.setCommandFactory(new ScpCommandFactory(new TestCommandFactory())); + server.setCommandFactory(new TestCommandFactory()); - // Set up file system - server.setFileSystemFactory(new TestFileSystemFactory()); + final File homeDir = getTestDirectoryFile(); + server.setFileSystemFactory( + new VirtualFileSystemFactory(homeDir.toPath().toAbsolutePath())); - // Start server server.start(); + final List authFactories = new ArrayList<>(server.getUserAuthFactories()); + authFactories.add(UserAuthNoneFactory.INSTANCE); + server.setUserAuthFactories(authFactories); + final int socketPort = server.getPort(); connectionUri = String.format("sftp://%s@localhost:%d", DEFAULT_USER, socketPort); - - // Allow no-auth for testing - server.getUserAuthFactories().add(new UserAuthNone.Factory()); } /** @@ -428,16 +276,14 @@ public boolean canListen(final InetSocketAddress address, final ServerSession se */ public static synchronized void stopServer() throws InterruptedException { if (server != null) { - // Close all active sessions - for (final AbstractSession session : server.getActiveSessions()) { - session.close(true); + try { + server.close(); + } catch (final IOException e) { + // ignore } - server.stop(); server = null; connectionUri = null; - - // Clear the permissions cache to avoid test interference - MySftpSubsystem.permissions.clear(); + TestSftpFileSystemAccessor.clearCache(); } } diff --git a/pom.xml b/pom.xml index 59e0ec0a39..51c59fb1d9 100644 --- a/pom.xml +++ b/pom.xml @@ -489,18 +489,18 @@ org.apache.sshd sshd-core - 0.8.0 + 3.0.0-M2 + + + org.apache.sshd + sshd-sftp + 3.0.0-M2 org.apache.mina mina-core 2.2.5 - - org.bouncycastle - bcprov-jdk16 - 1.46 - commons-io commons-io From 221ba6edc4be26eff8be0fdc7ad365af59930eaf Mon Sep 17 00:00:00 2001 From: Vaishnavi Kumbhar Date: Tue, 28 Apr 2026 19:13:41 +0530 Subject: [PATCH 3/3] Migrate SFTP test infrastructure from Apache MINA SSHD 0.x to 3.x API --- .../provider/sftp/SftpTestServerHelper.java | 53 +++++++++++++------ 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpTestServerHelper.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpTestServerHelper.java index 3585c73444..aa02f87a01 100644 --- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpTestServerHelper.java +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpTestServerHelper.java @@ -83,21 +83,12 @@ public void setFilePermissions( final SftpSubsystemProxy subsystem, final Path file, final Set perms, final LinkOption... options) throws IOException { permissionsCache.put(file.toAbsolutePath().toString(), new HashSet<>(perms)); - try { - SftpFileSystemAccessor.super.setFilePermissions(subsystem, file, perms, options); - } catch (final UnsupportedOperationException | IOException ignored) { - // Silently swallow on systems without native POSIX support - } } @Override public void setFileOwner( final SftpSubsystemProxy subsystem, final Path file, final Principal owner, final LinkOption... options) throws IOException { - // No-op: the test server always reports TEST_UID/TEST_GID via - // resolveReportedFileAttributes, so the client sends those back - // in setStat. Silently ignore to avoid chown failures on - // non-root Unix processes. } @Override @@ -107,25 +98,57 @@ public void setGroupOwner( // No-op: same rationale as setFileOwner. } + @Override + public void setFileAttribute( + final SftpSubsystemProxy subsystem, final Path file, + final String view, final String attribute, final Object value, + final LinkOption... options) throws IOException { + if ("uid".equals(attribute) || "gid".equals(attribute)) { + // No-op: SSHD routes integer uid/gid through setFileAttribute + // (not setFileOwner/setGroupOwner). The default calls + // Files.setAttribute(file, "unix:uid", ...) which throws + // IOException on non-root Unix when the uid doesn't match. + return; + } + try { + SftpFileSystemAccessor.super.setFileAttribute(subsystem, file, view, attribute, value, options); + } catch (final IOException | RuntimeException ignored) { + // Silently swallow; covers platforms without native POSIX + // support and edge-cases where the rooted file system + // rejects certain attribute writes. + } + } + @Override public Map readFileAttributes( final SftpSubsystemProxy subsystem, final Path file, final String view, final LinkOption... options) throws IOException { + Map result; + boolean synthetic = false; try { - return SftpFileSystemAccessor.super.readFileAttributes(subsystem, file, view, options); + result = SftpFileSystemAccessor.super.readFileAttributes(subsystem, file, view, options); } catch (final UnsupportedOperationException | IllegalArgumentException e) { if (view.startsWith("unix:") || view.startsWith("posix:")) { final Map attrs = new HashMap<>( Files.readAttributes(file, "basic:*", options)); - final Set cached = - permissionsCache.get(file.toAbsolutePath().toString()); - attrs.put("permissions", cached != null ? cached : defaultPermissions(file, options)); attrs.put("uid", TEST_UID); attrs.put("gid", TEST_GID); - return attrs; + result = attrs; + synthetic = true; + } else { + throw e; } - throw e; } + if (view.startsWith("unix:") || view.startsWith("posix:") || synthetic) { + final Map attrs = new HashMap<>(result); + final Set cached = + permissionsCache.get(file.toAbsolutePath().toString()); + attrs.put("permissions", cached != null ? cached : defaultPermissions(file, options)); + attrs.put("uid", TEST_UID); + attrs.put("gid", TEST_GID); + return attrs; + } + return result; } @Override