From 4e820c90bf8251383b8d72dcc3d0b2cbfd91f45f Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Mon, 20 Apr 2026 21:21:44 +0200 Subject: [PATCH 1/2] Add test cases to AuthorizedKeysCertificateTest See [1]. Apache MINA SSHD never had that problem, but we also had no test for this. So add one just to be sure. [1] https://bugzilla.mindrot.org/show_bug.cgi?id=3950 --- .../server/auth/AuthorizedKeysCertificateTest.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/sshd-core/src/test/java/org/apache/sshd/server/auth/AuthorizedKeysCertificateTest.java b/sshd-core/src/test/java/org/apache/sshd/server/auth/AuthorizedKeysCertificateTest.java index d11404e2e..8d14db153 100644 --- a/sshd-core/src/test/java/org/apache/sshd/server/auth/AuthorizedKeysCertificateTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/server/auth/AuthorizedKeysCertificateTest.java @@ -114,7 +114,13 @@ private static Stream options() { Arguments.of("cert-authority", "", false, false), // CA, no principal: fail Arguments.of("cert-authority", "other", false, false), // CA, wrong principal: fail Arguments.of("cert-authority,principals=\"other\"", "user", false, false), // CA, principal overridden - Arguments.of("cert-authority,principals=\"other\"", "other", false, true) // CA, principal overridden + Arguments.of("cert-authority,principals=\"other\"", "other", false, true), // CA, principal overridden + Arguments.of("cert-authority,principals=\"other,user\"", "user", false, true), // + // One principal "root,user" in the certificate + Arguments.of("cert-authority,principals=\"other,user\"", "root,user", false, false), // + // Two principals "root" and "user" in the certificate + Arguments.of("cert-authority,principals=\"other,user\"", "root;user", false, true), // + Arguments.of("cert-authority,principals=\"user,other\"", "root;user", false, true) // ); } @@ -151,7 +157,7 @@ private void initCert(String caMarker, String... principals) throws Exception { @MethodSource("options") void testCertificate(String marker, String principals, boolean allowEmpty, boolean expectSuccess) throws Exception { CoreModuleProperties.ALLOW_EMPTY_CERTIFICATE_PRINCIPALS.set(sshd, allowEmpty); - String[] certPrincipals = GenericUtils.isEmpty(principals) ? new String[0] : principals.split(","); + String[] certPrincipals = GenericUtils.isEmpty(principals) ? new String[0] : principals.split(";"); initCert(marker, certPrincipals); try (ClientSession s = client.connect("user", TEST_LOCALHOST, port).verify(CONNECT_TIMEOUT) .getSession()) { From 1285419f2f1bef5fad23f001f74d0f07cc469b5b Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Fri, 24 Apr 2026 20:26:23 +0200 Subject: [PATCH 2/2] GH-743: Use a singleton SftpFileSystemProvider for the ServiceLoader When SFTP file systems are created through the standard Java mechanism via java.nio.file.FileSystems.newFileSystem(URI, Map, Classloader), the JDK uses the ServiceLoader to find registered file system providers. It remembers any it can find via the system classloader or via the platform classloader as "installed providers" and creates only exactly one instance of them. When it cannot find a provider matching the URI scheme in the "installed providers" and a classloader is given, it then tries to find a matching file system provider using that classloader. The ServiceLoader instantiates each provider it finds to check its scheme against the URI scheme. But it doesn't remember these file system providers, so if FileSystems.newFileSystem() is called again, another instance of SftpFileSystemProvider got created. When the returned file system is used, the SftpFileSystemProvider creates an SshClient, and an I/O thread pool is set up. If multiple SftpFileSystemProviders get created, there will also be multiple SshClients, and multiple thread pools. Avoid this by registering via the FileSystemProvider SPI only a stateless facade class. All instance of this facade class delegate to a singleton SftpFileSystemProvider. --- CHANGES.md | 1 + .../java.nio.file.spi.FileSystemProvider | 2 +- .../fs/SftpFileSystemProviderFacade.java | 182 ++++++++++++++++++ .../sftp/client/fs/SftpFileSystemTest.java | 2 +- 4 files changed, 185 insertions(+), 2 deletions(-) create mode 100644 sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProviderFacade.java diff --git a/CHANGES.md b/CHANGES.md index 8315e299b..578c9b23b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -31,6 +31,7 @@ ## Bug Fixes +* [GH-743](https://github.com/apache/mina-sshd/issues/743) Ensure the Java `ServiceLoader` use a singleton `SftpFileSystemProvider` * [GH-879](https://github.com/apache/mina-sshd/issues/879) Close SSH channel gracefully on exception in port forwarding ## New Features diff --git a/sshd-sftp/src/main/filtered-resources/META-INF/services/java.nio.file.spi.FileSystemProvider b/sshd-sftp/src/main/filtered-resources/META-INF/services/java.nio.file.spi.FileSystemProvider index db3bfd4d7..f66c60571 100644 --- a/sshd-sftp/src/main/filtered-resources/META-INF/services/java.nio.file.spi.FileSystemProvider +++ b/sshd-sftp/src/main/filtered-resources/META-INF/services/java.nio.file.spi.FileSystemProvider @@ -17,4 +17,4 @@ ## under the License. ## -org.apache.sshd.sftp.client.fs.SftpFileSystemProvider +org.apache.sshd.sftp.client.fs.SftpFileSystemProviderFacade diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProviderFacade.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProviderFacade.java new file mode 100644 index 000000000..06e4261d5 --- /dev/null +++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProviderFacade.java @@ -0,0 +1,182 @@ +/* + * 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 + * + * http://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.sshd.sftp.client.fs; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.net.URI; +import java.nio.channels.FileChannel; +import java.nio.channels.SeekableByteChannel; +import java.nio.file.AccessMode; +import java.nio.file.CopyOption; +import java.nio.file.DirectoryStream; +import java.nio.file.DirectoryStream.Filter; +import java.nio.file.FileStore; +import java.nio.file.FileSystem; +import java.nio.file.LinkOption; +import java.nio.file.OpenOption; +import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.FileAttribute; +import java.nio.file.attribute.FileAttributeView; +import java.nio.file.spi.FileSystemProvider; +import java.util.Map; +import java.util.Set; + +import org.apache.sshd.sftp.common.SftpConstants; + +/** + * SPI-registered SFTP file system provider using a singleton instance. + * + * @author Apache MINA SSHD Project + */ +public class SftpFileSystemProviderFacade extends FileSystemProvider { + + private static final class DefaultSftpFileSystemSingleton { + static final SftpFileSystemProvider INSTANCE = new SftpFileSystemProvider(); + + private DefaultSftpFileSystemSingleton() { + // No instantiation + } + } + + public SftpFileSystemProviderFacade() { + // Nothing + } + + @Override + public String getScheme() { + return SftpConstants.SFTP_SUBSYSTEM_NAME; + } + + @Override + public FileSystem newFileSystem(URI uri, Map env) throws IOException { + return DefaultSftpFileSystemSingleton.INSTANCE.newFileSystem(uri, env); + } + + @Override + public FileSystem getFileSystem(URI uri) { + return DefaultSftpFileSystemSingleton.INSTANCE.getFileSystem(uri); + } + + @Override + public Path getPath(URI uri) { + return DefaultSftpFileSystemSingleton.INSTANCE.getPath(uri); + } + + @Override + public SeekableByteChannel newByteChannel(Path path, Set options, FileAttribute... attrs) + throws IOException { + return DefaultSftpFileSystemSingleton.INSTANCE.newByteChannel(path, options, attrs); + } + + @Override + public FileChannel newFileChannel(Path path, Set options, FileAttribute... attrs) + throws IOException { + return DefaultSftpFileSystemSingleton.INSTANCE.newFileChannel(path, options, attrs); + } + + @Override + public InputStream newInputStream(Path path, OpenOption... options) throws IOException { + return DefaultSftpFileSystemSingleton.INSTANCE.newInputStream(path, options); + } + + @Override + public OutputStream newOutputStream(Path path, OpenOption... options) throws IOException { + return DefaultSftpFileSystemSingleton.INSTANCE.newOutputStream(path, options); + } + + @Override + public DirectoryStream newDirectoryStream(Path dir, Filter filter) throws IOException { + return DefaultSftpFileSystemSingleton.INSTANCE.newDirectoryStream(dir, filter); + } + + @Override + public void createSymbolicLink(Path link, Path target, FileAttribute... attrs) throws IOException { + DefaultSftpFileSystemSingleton.INSTANCE.createSymbolicLink(link, target, attrs); + } + + @Override + public Path readSymbolicLink(Path link) throws IOException { + return DefaultSftpFileSystemSingleton.INSTANCE.readSymbolicLink(link); + } + + @Override + public void createDirectory(Path dir, FileAttribute... attrs) throws IOException { + DefaultSftpFileSystemSingleton.INSTANCE.createDirectory(dir, attrs); + } + + @Override + public void delete(Path path) throws IOException { + DefaultSftpFileSystemSingleton.INSTANCE.delete(path); + } + + @Override + public void copy(Path source, Path target, CopyOption... options) throws IOException { + DefaultSftpFileSystemSingleton.INSTANCE.copy(source, target, options); + } + + @Override + public void move(Path source, Path target, CopyOption... options) throws IOException { + DefaultSftpFileSystemSingleton.INSTANCE.move(source, target, options); + } + + @Override + public boolean isSameFile(Path path, Path path2) throws IOException { + return DefaultSftpFileSystemSingleton.INSTANCE.isSameFile(path, path2); + } + + @Override + public boolean isHidden(Path path) throws IOException { + return DefaultSftpFileSystemSingleton.INSTANCE.isHidden(path); + } + + @Override + public FileStore getFileStore(Path path) throws IOException { + return DefaultSftpFileSystemSingleton.INSTANCE.getFileStore(path); + } + + @Override + public void checkAccess(Path path, AccessMode... modes) throws IOException { + DefaultSftpFileSystemSingleton.INSTANCE.checkAccess(path, modes); + } + + @Override + public V getFileAttributeView(Path path, Class type, LinkOption... options) { + return DefaultSftpFileSystemSingleton.INSTANCE.getFileAttributeView(path, type, options); + } + + @Override + public A readAttributes(Path path, Class type, LinkOption... options) + throws IOException { + return DefaultSftpFileSystemSingleton.INSTANCE.readAttributes(path, type, options); + } + + @Override + public Map readAttributes(Path path, String attributes, LinkOption... options) throws IOException { + return DefaultSftpFileSystemSingleton.INSTANCE.readAttributes(path, attributes, options); + } + + @Override + public void setAttribute(Path path, String attribute, Object value, LinkOption... options) throws IOException { + DefaultSftpFileSystemSingleton.INSTANCE.setAttribute(path, attribute, value, options); + } + +} diff --git a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/fs/SftpFileSystemTest.java b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/fs/SftpFileSystemTest.java index 2dec9e9c9..530658118 100644 --- a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/fs/SftpFileSystemTest.java +++ b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/fs/SftpFileSystemTest.java @@ -1081,7 +1081,7 @@ void fileSystemProviderServiceEntry() throws IOException { } assertFalse(found, "Multiple configurations: " + line); - assertEquals(SftpFileSystemProvider.class.getName(), line, "Mismatched configuration"); + assertEquals(SftpFileSystemProviderFacade.class.getName(), line, "Mismatched configuration"); found = true; } }