You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2019/02/24 17:33:30 UTC
[mina-sshd] branch master updated: [SSHD-899] Ignore file
attributes when opening an existing SFTP file handle
This is an automated email from the ASF dual-hosted git repository.
lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
The following commit(s) were added to refs/heads/master by this push:
new 19adb39 [SSHD-899] Ignore file attributes when opening an existing SFTP file handle
19adb39 is described below
commit 19adb39e4706929b6e5a1b2df056a2b2a29fac4d
Author: The-Yoda <yo...@gmail.com>
AuthorDate: Fri Feb 22 18:57:38 2019 +0200
[SSHD-899] Ignore file attributes when opening an existing SFTP file handle
---
.../apache/sshd/util/test/JUnitTestSupport.java | 4 +-
.../subsystem/sftp/SftpFileSystemAccessor.java | 10 +++
.../sshd/client/subsystem/sftp/SftpTest.java | 73 ++++++++++++++++++++++
3 files changed, 86 insertions(+), 1 deletion(-)
diff --git a/sshd-common/src/test/java/org/apache/sshd/util/test/JUnitTestSupport.java b/sshd-common/src/test/java/org/apache/sshd/util/test/JUnitTestSupport.java
index 175cd93..9d57473 100644
--- a/sshd-common/src/test/java/org/apache/sshd/util/test/JUnitTestSupport.java
+++ b/sshd-common/src/test/java/org/apache/sshd/util/test/JUnitTestSupport.java
@@ -351,13 +351,15 @@ public abstract class JUnitTestSupport extends Assert {
return folder;
}
- public static void assertObjectInstanceOf(String message, Class<?> expected, Object obj) {
+ public static <T> T assertObjectInstanceOf(String message, Class<? extends T> expected, Object obj) {
assertNotNull(message + " - no actual object", obj);
Class<?> actual = obj.getClass();
if (!expected.isAssignableFrom(actual)) {
fail(message + " - actual object type (" + actual.getName() + ") incompatible with expected (" + expected.getName() + ")");
}
+
+ return expected.cast(obj);
}
public static <E> void assertListEquals(String message, List<? extends E> expected, List<? extends E> actual) {
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpFileSystemAccessor.java b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpFileSystemAccessor.java
index f84b562..fa24760 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpFileSystemAccessor.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpFileSystemAccessor.java
@@ -37,6 +37,7 @@ import java.util.Set;
import org.apache.sshd.common.util.MapEntryUtils.NavigableMapBuilder;
import org.apache.sshd.common.util.io.FileInfoExtractor;
+import org.apache.sshd.common.util.io.IoUtils;
import org.apache.sshd.server.session.ServerSession;
/**
@@ -85,6 +86,15 @@ public interface SftpFileSystemAccessor {
ServerSession session, SftpEventListenerManager subsystem,
Path file, String handle, Set<? extends OpenOption> options, FileAttribute<?>... attrs)
throws IOException {
+ /*
+ * According to https://tools.ietf.org/html/draft-ietf-secsh-filexfer-13#page-33
+ *
+ * The 'attrs' field is ignored if an existing file is opened.
+ */
+ if (Files.exists(file)) {
+ attrs = IoUtils.EMPTY_FILE_ATTRIBUTES;
+ }
+
return FileChannel.open(file, options, attrs);
}
diff --git a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java
index d417225..29d9fdc 100644
--- a/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java
+++ b/sshd-sftp/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java
@@ -40,6 +40,7 @@ import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardOpenOption;
import java.nio.file.attribute.FileAttribute;
+import java.nio.file.attribute.PosixFilePermission;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@@ -66,6 +67,7 @@ import org.apache.sshd.client.subsystem.sftp.SftpClient.OpenMode;
import org.apache.sshd.client.subsystem.sftp.extensions.BuiltinSftpClientExtensions;
import org.apache.sshd.client.subsystem.sftp.extensions.SftpClientExtension;
import org.apache.sshd.client.subsystem.sftp.impl.AbstractSftpClient;
+import org.apache.sshd.client.subsystem.sftp.impl.DefaultCloseableHandle;
import org.apache.sshd.common.Factory;
import org.apache.sshd.common.FactoryManager;
import org.apache.sshd.common.NamedFactory;
@@ -87,7 +89,10 @@ import org.apache.sshd.common.subsystem.sftp.extensions.VersionsParser.Versions;
import org.apache.sshd.common.subsystem.sftp.extensions.openssh.AbstractOpenSSHExtensionParser.OpenSSHExtension;
import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.OsUtils;
+import org.apache.sshd.common.util.ValidateUtils;
+import org.apache.sshd.common.util.buffer.Buffer;
import org.apache.sshd.common.util.buffer.BufferUtils;
+import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
import org.apache.sshd.common.util.io.IoUtils;
import org.apache.sshd.server.command.Command;
import org.apache.sshd.server.session.ServerSession;
@@ -490,6 +495,74 @@ public class SftpTest extends AbstractSftpClientTestSupport {
}
}
+ @Test //SSHD-899
+ public void testNoAttributeImpactOnOpen() throws Exception {
+ Path targetPath = detectTargetFolder();
+ Path parentPath = targetPath.getParent();
+ Path lclSftp = CommonTestSupportUtils.resolve(
+ targetPath, SftpConstants.SFTP_SUBSYSTEM_NAME, getClass().getSimpleName(), getCurrentTestName());
+ Path clientFolder = lclSftp.resolve("client");
+ Path testFile = clientFolder.resolve("file.txt");
+ String file = CommonTestSupportUtils.resolveRelativeRemotePath(parentPath, testFile);
+
+ assertHierarchyTargetFolderExists(testFile.getParent());
+ try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port)
+ .verify(7L, TimeUnit.SECONDS)
+ .getSession()) {
+ session.addPasswordIdentity(getCurrentTestName());
+ session.auth().verify(5L, TimeUnit.SECONDS);
+
+ Files.deleteIfExists(testFile); // make sure starting fresh
+ Files.createFile(testFile, IoUtils.EMPTY_FILE_ATTRIBUTES);
+
+ try (SftpClient sftp = createSftpClient(session)) {
+ Collection<PosixFilePermission> initialPermissions = IoUtils.getPermissions(testFile);
+ assertTrue("File does not have enough initial permissions: " + initialPermissions,
+ initialPermissions.containsAll(EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE)));
+
+ try (CloseableHandle handle = sendRawAttributeImpactOpen(file, sftp)) {
+ outputDebugMessage("%s - handle=%s", getCurrentTestName(), handle);
+ }
+
+ Collection<PosixFilePermission> updatedPermissions = IoUtils.getPermissions(testFile);
+ assertEquals("Mismatched updated permissions count", initialPermissions.size(), updatedPermissions.size());
+ assertTrue("File does not preserve initial permissions: expected=" + initialPermissions + ", actual=" + updatedPermissions,
+ updatedPermissions.containsAll(initialPermissions));
+ } finally {
+ Files.delete(testFile);
+ }
+ }
+ }
+
+ private CloseableHandle sendRawAttributeImpactOpen(String path, SftpClient sftpClient) throws Exception {
+ RawSftpClient sftp = assertObjectInstanceOf("Not a raw SFTP client used", RawSftpClient.class, sftpClient);
+ Buffer buffer = new ByteArrayBuffer(path.length() + Long.SIZE, false);
+ buffer.putString(path, StandardCharsets.UTF_8);
+ //access
+ buffer.putInt(SftpConstants.ACE4_READ_DATA | SftpConstants.ACE4_READ_ATTRIBUTES);
+ //mode
+ buffer.putInt(SftpConstants.SSH_FXF_OPEN_EXISTING);
+ //flag
+ buffer.putInt(SftpConstants.SSH_FILEXFER_ATTR_PERMISSIONS);
+ buffer.putByte((byte) SftpConstants.SSH_FILEXFER_TYPE_REGULAR);
+
+ buffer.putInt(0);
+
+ int reqId = sftp.send(SftpConstants.SSH_FXP_OPEN, buffer);
+ Buffer response = sftp.receive(reqId);
+ byte[] rawHandle = getRawFileHandle(response);
+ return new DefaultCloseableHandle(sftpClient, path, rawHandle);
+ }
+
+ private byte[] getRawFileHandle(Buffer buffer) {
+ buffer.getInt(); //length
+ int type = buffer.getUByte();
+ assertEquals("Mismatched response type", SftpConstants.SSH_FXP_HANDLE, type);
+ buffer.getInt(); //id
+ return ValidateUtils.checkNotNullAndNotEmpty(
+ buffer.getBytes(), "Null/empty handle in buffer", GenericUtils.EMPTY_OBJECT_ARRAY);
+ }
+
@Test
public void testInputStreamSkipAndReset() throws Exception {
Path targetPath = detectTargetFolder();