GH-500: SftpFileSystemProvider: close SftpClient on exception

If client.read() or client.write() throw an exception, the client must be closed. Bug: #500
apache · May 11, 2024 · 71b842f · 71b842f
1 parent 4f2ccf8
commit 71b842f
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 21 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -36,7 +36,7 @@
 * [GH-455](https://github.com/apache/mina-sshd/issues/455) Fix `BaseCipher`: make sure all bytes are processed
 * [GH-470](https://github.com/apache/mina-sshd/issues/470) MontgomeryCurve: synchronize access to KeyPairGenerator
 * [GH-489](https://github.com/apache/mina-sshd/issues/489) SFTP v3 client: better file type determination
-
+* [GH-500](https://github.com/apache/mina-sshd/issues/500) SFTP file system: fix memory leak on exceptions
 
 * [PR-472](https://github.com/apache/mina-sshd/pull/472) sshd-spring-sftp: fix client start
 * [PR-476](https://github.com/apache/mina-sshd/pull/476) Fix Android detection

diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java
@@ -616,16 +616,25 @@ public InputStream newInputStream(Path path, OpenOption... options) throws IOExc
         modes = EnumSet.of(OpenMode.Read);
         SftpPath p = toSftpPath(path);
         SftpClient client = p.getFileSystem().getClient();
-        return new FilterInputStream(client.read(p.toString(), modes)) {
-            @Override
-            public void close() throws IOException {
-                try {
-                    super.close();
-                } finally {
-                    client.close();
+        try {
+            SftpClient inner = client;
+            InputStream result = new FilterInputStream(client.read(p.toString(), modes)) {
+                @Override
+                public void close() throws IOException {
+                    try {
+                        super.close();
+                    } finally {
+                        inner.close();
+                    }
                 }
+            };
+            client = null; // Prevent closing in finally
+            return result;
+        } finally {
+            if (client != null) {
+                client.close();
             }
-        };
+        }
     }
 
     @Override
@@ -648,22 +657,31 @@ public OutputStream newOutputStream(Path path, OpenOption... options) throws IOE
         }
         SftpPath p = toSftpPath(path);
         SftpClient client = p.getFileSystem().getClient();
-        return new FilterOutputStream(client.write(p.toString(), modes)) {
+        try {
+            SftpClient inner = client;
+            OutputStream result = new FilterOutputStream(client.write(p.toString(), modes)) {
 
-            @Override
-            public void close() throws IOException {
-                try {
-                    super.close();
-                } finally {
-                    client.close();
+                @Override
+                public void close() throws IOException {
+                    try {
+                        super.close();
+                    } finally {
+                        inner.close();
+                    }
                 }
-            }
 
-            @Override
-            public void write(byte[] b, int off, int len) throws IOException {
-                out.write(b, off, len);
+                @Override
+                public void write(byte[] b, int off, int len) throws IOException {
+                    out.write(b, off, len);
+                }
+            };
+            client = null; // Prevent closing in finally
+            return result;
+        } finally {
+            if (client != null) {
+                client.close();
             }
-        };
+        }
     }
 
     @Override