You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2021/11/26 17:21:19 UTC

[commons-vfs] branch master updated: fix the sftp channel don't return to the pool when exception on SftpFileObject.doGetOutputStream (#215)

This is an automated email from the ASF dual-hosted git repository.

ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-vfs.git


The following commit(s) were added to refs/heads/master by this push:
     new d2f6f34  fix the sftp channel don't return to the pool when exception on  SftpFileObject.doGetOutputStream (#215)
d2f6f34 is described below

commit d2f6f34f9700bd66a226d2ceb1495ac0d4c8f223
Author: zhouwenqing <zh...@users.noreply.github.com>
AuthorDate: Sat Nov 27 01:21:12 2021 +0800

    fix the sftp channel don't return to the pool when exception on  SftpFileObject.doGetOutputStream (#215)
    
    * fix the sftp channel don't return to the pool when exception on  SftpFileObject.doGetOutputStream
    
    * Add test case for SftpFileObject#doGetOutputStream return the channel to pool when there is some exceptions.
---
 .../commons/vfs2/provider/sftp/SftpFileObject.java |  10 +-
 .../sftp/AbstractSftpProviderTestCase.java         |   2 +-
 .../sftp/SftpPermissionExceptionTestCase.java      | 124 +++++++++++++++++++++
 3 files changed, 134 insertions(+), 2 deletions(-)

diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/sftp/SftpFileObject.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/sftp/SftpFileObject.java
index 45fe984..9ef7aa9 100644
--- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/sftp/SftpFileObject.java
+++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/sftp/SftpFileObject.java
@@ -217,7 +217,15 @@ public class SftpFileObject extends AbstractFileObject<SftpFileSystem> {
          */
 
         final ChannelSftp channel = getAbstractFileSystem().getChannel();
-        return new SftpOutputStream(channel, channel.put(relPath, bAppend ? ChannelSftp.APPEND : ChannelSftp.OVERWRITE));
+        try {
+            return new SftpOutputStream(channel, channel.put(relPath, bAppend ? ChannelSftp.APPEND : ChannelSftp.OVERWRITE));
+        } catch (Exception ex) {
+            // when channel.put throw exception eg. com.jcraft.jsch.SftpException: Permission denied
+            //   returns the channel to the pool
+            getAbstractFileSystem().putChannel(channel);
+            throw ex;
+        }
+
     }
 
     @Override
diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/AbstractSftpProviderTestCase.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/AbstractSftpProviderTestCase.java
index 49a8007..8ec4920 100644
--- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/AbstractSftpProviderTestCase.java
+++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/AbstractSftpProviderTestCase.java
@@ -420,7 +420,7 @@ abstract class AbstractSftpProviderTestCase extends AbstractProviderTestConfig {
 
     protected static String ConnectionUri;
 
-    private static SshServer Server;
+    protected static SshServer Server;
 
     private static final String TEST_URI = "test.sftp.uri";
 
diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpPermissionExceptionTestCase.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpPermissionExceptionTestCase.java
new file mode 100644
index 0000000..e7bed24
--- /dev/null
+++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpPermissionExceptionTestCase.java
@@ -0,0 +1,124 @@
+/*
+ * 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.commons.vfs2.provider.sftp;
+
+import java.io.File;
+import java.nio.file.Paths;
+
+import org.apache.commons.vfs2.AbstractVfsTestCase;
+import org.apache.commons.vfs2.Capability;
+import org.apache.commons.vfs2.FileObject;
+import org.apache.commons.vfs2.Selectors;
+import org.apache.commons.vfs2.VFS;
+import org.apache.sshd.server.channel.ChannelSession;
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Test SftpFileObject.doGetOutputStream return the channel to pool, even throw a sftp write permission exception.
+ */
+public class SftpPermissionExceptionTestCase extends AbstractSftpProviderTestCase {
+
+    /**
+     * Sets up a scratch folder for the test to use.
+     */
+    protected FileObject createScratchFolder() throws Exception {
+        final FileObject scratchFolder = getWriteFolder();
+
+        // Make sure the test folder is empty
+        scratchFolder.delete(Selectors.EXCLUDE_SELF);
+        scratchFolder.createFolder();
+        scratchFolder.setWritable(false, false);
+        return scratchFolder;
+    }
+
+    /**
+     * Returns the capabilities required by the tests of this test case.
+     */
+    @Override
+    protected Capability[] getRequiredCapabilities() {
+        return new Capability[] {Capability.CREATE, Capability.DELETE, Capability.GET_TYPE, Capability.LIST_CHILDREN,
+            Capability.READ_CONTENT, Capability.WRITE_CONTENT};
+    }
+
+
+    /**
+     * Test SftpFileObject.doGetOutputStream return the channel to pool, when there is a exception in channel.put .
+     */
+    @Test
+    public void testGetOutputStreamException() throws Exception {
+        final File localFile = new File("src/test/resources/test-data/test.zip");
+
+        final FileObject localFileObject = VFS.getManager().toFileObject(localFile);
+
+        final FileObject scratchFolder = createScratchFolder();
+
+
+        // try to create local file
+        String fileName = "filecopy.txt";
+        FileObject fileObjectCopy = scratchFolder.resolveFile(fileName);
+        fileObjectCopy.setWritable(false, false);
+        fileObjectCopy.copyFrom(localFileObject, Selectors.SELECT_SELF);
+
+        // try to set the local file to readonly
+        Paths.get(AbstractVfsTestCase.getTestDirectory(),scratchFolder.getName().getBaseName(), fileName).toFile().setWritable(false);
+        for (int i = 0; i < 30; i++) {
+            try{
+                fileObjectCopy = scratchFolder.resolveFile(fileName);
+                Assert.assertFalse(fileObjectCopy.isWriteable());
+                fileObjectCopy.copyFrom(localFileObject, Selectors.SELECT_SELF);
+                Assert.fail("permission fail");
+            } catch (Exception ex) {
+                // ignore no perminison
+            }
+        }
+
+        // try to get created channel number.
+        int channelId = Server.getActiveSessions().get(0).registerChannel(new ChannelSession());
+        Assert.assertTrue("create too many sftp channel more", channelId<30);
+
+        // try to set the local file to writable
+        Paths.get(AbstractVfsTestCase.getTestDirectory(),scratchFolder.getName().getBaseName(), fileName).toFile().setWritable(true);
+
+
+        fileObjectCopy = scratchFolder.resolveFile(fileName);
+        Assert.assertTrue(fileObjectCopy.isWriteable());
+        fileObjectCopy.copyFrom(localFileObject, Selectors.SELECT_SELF);
+
+
+
+    }
+
+    /**
+     * Creates the test suite for the sftp file system.
+     */
+    public static junit.framework.Test suite() throws Exception {
+        final SftpProviderTestSuite suite = new SftpProviderTestSuite(new SftpPermissionExceptionTestCase()){
+            @Override
+            protected void addBaseTests() throws Exception {
+                // Just tries to read
+                addTests(SftpPermissionExceptionTestCase.class);
+            }
+        };
+        return suite;
+    }
+
+    @Override
+    protected boolean isExecChannelClosed() {
+        return false;
+    }
+}