From 9f8bcc2e2a9f967a48ea5f1e634fd792d1dd420a Mon Sep 17 00:00:00 2001 From: Florian Klemenz Date: Thu, 27 Oct 2022 12:37:36 +0200 Subject: [PATCH 01/10] adds fallback to posix-rename@openssh.com extension if possible and communicates possible problems with flags to the developer --- .../net/schmizz/sshj/sftp/SFTPEngine.java | 69 +++++++++++++++++-- 1 file changed, 62 insertions(+), 7 deletions(-) diff --git a/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java b/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java index 9114af35..93321bd6 100644 --- a/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java +++ b/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java @@ -237,16 +237,71 @@ public void rename(String oldPath, String newPath, Set flags) if (operativeVersion < 1) throw new SFTPException("RENAME is not supported in SFTPv" + operativeVersion); - final Request request = newRequest(PacketType.RENAME).putString(oldPath, sub.getRemoteCharset()).putString(newPath, sub.getRemoteCharset()); - // SFTP Version 5 introduced rename flags according to Section 6.5 of the specification - if (operativeVersion >= 5) { - long renameFlagMask = 0L; - for (RenameFlags flag : flags) { - renameFlagMask = renameFlagMask | flag.longValue(); + // request variables to be determined + PacketType type = PacketType.RENAME; // Default + long renameFlagMask = 0L; + String serverExtension = null; + + if (!flags.isEmpty()) { + // SFTP Version 5 introduced rename flags according to Section 6.5 of the specification + if (operativeVersion >= 5) { + for (RenameFlags flag : flags) { + renameFlagMask = renameFlagMask | flag.longValue(); + } } - request.putUInt32(renameFlagMask); + // Try to find a fallback solution if flags are not supported by the server. + + // "posix-rename@openssh.com" provides ATOMIC and OVERWRITE behaviour. + // From the SFTP-spec, Section 6.5: + // "If SSH_FXP_RENAME_OVERWRITE is specified, the server MAY perform an atomic rename even if it is + // not requested." + // So, if overwrite is allowed we can always use the posix-rename as a fallback. + else if (flags.contains(RenameFlags.OVERWRITE) && + supportsServerExtension("posix-rename","openssh.com")) { + + type = PacketType.EXTENDED; + serverExtension = "posix-rename@openssh.com"; + } + + // Because the OVERWRITE flag changes the behaviour in a possibly unintended way, it has to be + // explicitly requested for the above fallback to be applicable. + // Tell this to the developer if ATOMIC is requested without OVERWRITE. + else if (flags.contains(RenameFlags.ATOMIC) && + !flags.contains(RenameFlags.OVERWRITE) && + !flags.contains(RenameFlags.NATIVE) && // see next case below + supportsServerExtension("posix-rename","openssh.com")) { + + throw new SFTPException("RENAME-FLAGS are not supported in SFTPv" + operativeVersion + " but " + + "the \"posix-rename@openssh.com\" extension could be used as fallback if OVERWRITE " + + "behaviour is acceptable (needs to be activated via RenameFlags.OVERWRITE)."); + } + + // From the SFTP-spec, Section 6.5: + // "If flags includes SSH_FXP_RENAME_NATIVE, the server is free to do the rename operation in whatever + // fashion it deems appropriate. Other flag values are considered hints as to desired behavior, but not + // requirements." + else if (flags.contains(RenameFlags.NATIVE)) { + log.debug("Flags are not supported but NATIVE-flag allows to ignore other requested flags: " + + flags.toString()); + } + + // finally: let the user know that the server does not support what was asked + else throw new SFTPException("RENAME-FLAGS are not supported in SFTPv" + operativeVersion + " and no " + + "supported server extension could be found to achieve a similar result."); } + // build and send request + final Request request = newRequest(type); + + if (serverExtension != null) + request.putString(serverExtension); + + request.putString(oldPath, sub.getRemoteCharset()) + .putString(newPath, sub.getRemoteCharset()); + + if (renameFlagMask != 0L) + request.putUInt32(renameFlagMask); + doRequest(request).ensureStatusPacketIsOK(); } From 7a580f3a16c43cbe815cc78c3e6a995d09533c4f Mon Sep 17 00:00:00 2001 From: Florian Klemenz Date: Mon, 31 Oct 2022 11:51:33 +0100 Subject: [PATCH 02/10] Adds '{}' around if/else statements --- src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java b/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java index 93321bd6..e4c0c99d 100644 --- a/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java +++ b/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java @@ -234,8 +234,9 @@ public FileAttributes lstat(String path) public void rename(String oldPath, String newPath, Set flags) throws IOException { - if (operativeVersion < 1) + if (operativeVersion < 1) { throw new SFTPException("RENAME is not supported in SFTPv" + operativeVersion); + } // request variables to be determined PacketType type = PacketType.RENAME; // Default @@ -286,21 +287,25 @@ else if (flags.contains(RenameFlags.NATIVE)) { } // finally: let the user know that the server does not support what was asked - else throw new SFTPException("RENAME-FLAGS are not supported in SFTPv" + operativeVersion + " and no " + + else { + throw new SFTPException("RENAME-FLAGS are not supported in SFTPv" + operativeVersion + " and no " + "supported server extension could be found to achieve a similar result."); + } } // build and send request final Request request = newRequest(type); - if (serverExtension != null) + if (serverExtension != null) { request.putString(serverExtension); + } request.putString(oldPath, sub.getRemoteCharset()) .putString(newPath, sub.getRemoteCharset()); - if (renameFlagMask != 0L) + if (renameFlagMask != 0L) { request.putUInt32(renameFlagMask); + } doRequest(request).ensureStatusPacketIsOK(); } From c6c48499ad039e68797a2e0e14cf93cd991d29d1 Mon Sep 17 00:00:00 2001 From: Florian Klemenz Date: Thu, 3 Nov 2022 11:16:29 +0100 Subject: [PATCH 03/10] adds basic tests for file rename --- .../hierynomus/sshj/sftp/RemoteFileTest.java | 85 ++++++++++++++++++- 1 file changed, 81 insertions(+), 4 deletions(-) diff --git a/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java b/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java index c69d1240..c8775546 100644 --- a/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java +++ b/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java @@ -18,10 +18,7 @@ import com.hierynomus.sshj.test.SshFixture; import net.schmizz.sshj.SSHClient; import net.schmizz.sshj.common.ByteArrayUtils; -import net.schmizz.sshj.sftp.OpenMode; -import net.schmizz.sshj.sftp.RemoteFile; -import net.schmizz.sshj.sftp.SFTPEngine; -import net.schmizz.sshj.sftp.SFTPException; +import net.schmizz.sshj.sftp.*; import org.apache.sshd.common.util.io.IoUtils; import org.junit.Rule; import org.junit.Test; @@ -30,7 +27,9 @@ import java.io.*; import java.security.SecureRandom; import java.util.EnumSet; +import java.util.HashSet; import java.util.Random; +import java.util.Set; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; @@ -225,4 +224,82 @@ rf.new ReadAheadRemoteFileInputStream(10), assertThat("The file should be read correctly", ByteArrayUtils.equals(expected, 0, actual, 0, expected.length)); } + + @Test + public void shouldOverwriteFileWhenRequested() throws IOException { + SSHClient ssh = fixture.setupConnectedDefaultClient(); + ssh.authPassword("test", "test"); + SFTPEngine sftp = new SFTPEngine(ssh).init(); + + // create source file + final byte[] sourceBytes = new byte[32]; + new SecureRandom(new byte[]{31}).nextBytes(sourceBytes); + + File sourceFile = temp.newFile("shouldOverwriteFileWhenRequested-source.bin"); + try (OutputStream fStream = new FileOutputStream(sourceFile)) { + IoUtils.copy(new ByteArrayInputStream(sourceBytes), fStream); + } + + // create target file + final byte[] targetBytes = new byte[32]; + new SecureRandom(new byte[]{31}).nextBytes(targetBytes); + + File targetFile = temp.newFile("shouldOverwriteFileWhenRequested-target.bin"); + try (OutputStream fStream = new FileOutputStream(targetFile)) { + IoUtils.copy(new ByteArrayInputStream(targetBytes), fStream); + } + + // atomic rename with overwrite + Set flags = EnumSet.of(RenameFlags.OVERWRITE); + sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); + + // check if rename was successful + assertThat("The source file should not exist anymore", !sourceFile.exists()); + assertThat("The contents of the target file should be equal to the contents previously written " + + "to the source file", + ByteArrayUtils.equals(IoUtils.toByteArray(new FileInputStream(targetFile)), 0, + sourceBytes, 0, sourceBytes.length)); + } + + @Test + public void shouldNotOverwriteFileWhenNotRequested() throws IOException { + SSHClient ssh = fixture.setupConnectedDefaultClient(); + ssh.authPassword("test", "test"); + SFTPEngine sftp = new SFTPEngine(ssh).init(); + + // create source file + final byte[] sourceBytes = new byte[32]; + new SecureRandom(new byte[]{31}).nextBytes(sourceBytes); + + File sourceFile = temp.newFile("shouldOverwriteFileWhenRequested-source.bin"); + try (OutputStream fStream = new FileOutputStream(sourceFile)) { + IoUtils.copy(new ByteArrayInputStream(sourceBytes), fStream); + } + + // create target file + final byte[] targetBytes = new byte[32]; + new SecureRandom(new byte[]{31}).nextBytes(targetBytes); + + File targetFile = temp.newFile("shouldOverwriteFileWhenRequested-target.bin"); + try (OutputStream fStream = new FileOutputStream(targetFile)) { + IoUtils.copy(new ByteArrayInputStream(targetBytes), fStream); + } + + // atomic rename with overwrite + Boolean exceptionThrown = false; + try { + Set flags = new HashSet<>(); + sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); + } + catch (net.schmizz.sshj.sftp.SFTPException e) { + exceptionThrown = true; + } + + // check if rename failed as it should + assertThat("The source file should still exist", sourceFile.exists()); + assertThat("The contents of the target file should be equal to the contents previously written to it", + ByteArrayUtils.equals(IoUtils.toByteArray(new FileInputStream(targetFile)), 0, + targetBytes, 0, targetBytes.length)); + assertThat("An appropriate exception should have been thrown", exceptionThrown); + } } From 13ed40b7f4b00cc7602c7c91bd9efb406eb28db2 Mon Sep 17 00:00:00 2001 From: Florian Klemenz Date: Thu, 3 Nov 2022 11:53:51 +0100 Subject: [PATCH 04/10] fix comments --- src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java b/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java index c8775546..8bc87507 100644 --- a/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java +++ b/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java @@ -249,7 +249,7 @@ public void shouldOverwriteFileWhenRequested() throws IOException { IoUtils.copy(new ByteArrayInputStream(targetBytes), fStream); } - // atomic rename with overwrite + // rename with overwrite Set flags = EnumSet.of(RenameFlags.OVERWRITE); sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); @@ -285,7 +285,7 @@ public void shouldNotOverwriteFileWhenNotRequested() throws IOException { IoUtils.copy(new ByteArrayInputStream(targetBytes), fStream); } - // atomic rename with overwrite + // rename without overwrite -> should fail Boolean exceptionThrown = false; try { Set flags = new HashSet<>(); From 274d4e97f8eb4e29fc4884606e1469239ca20456 Mon Sep 17 00:00:00 2001 From: Florian Klemenz Date: Thu, 24 Nov 2022 11:59:33 +0100 Subject: [PATCH 05/10] fixes indentation --- src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java b/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java index e4c0c99d..b681e451 100644 --- a/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java +++ b/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java @@ -271,8 +271,7 @@ else if (flags.contains(RenameFlags.ATOMIC) && !flags.contains(RenameFlags.OVERWRITE) && !flags.contains(RenameFlags.NATIVE) && // see next case below supportsServerExtension("posix-rename","openssh.com")) { - - throw new SFTPException("RENAME-FLAGS are not supported in SFTPv" + operativeVersion + " but " + + throw new SFTPException("RENAME-FLAGS are not supported in SFTPv" + operativeVersion + " but " + "the \"posix-rename@openssh.com\" extension could be used as fallback if OVERWRITE " + "behaviour is acceptable (needs to be activated via RenameFlags.OVERWRITE)."); } From 837fa966bedd63c092d247bc14206811cf1b01bd Mon Sep 17 00:00:00 2001 From: Florian Klemenz Date: Fri, 20 Oct 2023 10:56:33 +0200 Subject: [PATCH 06/10] adds helper methods to make existing sftp rename tests more concise --- .../hierynomus/sshj/sftp/RemoteFileTest.java | 84 +++++++++---------- 1 file changed, 41 insertions(+), 43 deletions(-) diff --git a/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java b/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java index 5400ab64..6ef45b07 100644 --- a/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java +++ b/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java @@ -227,69 +227,39 @@ rf.new ReadAheadRemoteFileInputStream(10), @Test public void shouldOverwriteFileWhenRequested() throws IOException { - SSHClient ssh = fixture.setupConnectedDefaultClient(); - ssh.authPassword("test", "test"); - SFTPEngine sftp = new SFTPEngine(ssh).init(); - // create source file - final byte[] sourceBytes = new byte[32]; - new SecureRandom(new byte[]{31}).nextBytes(sourceBytes); - - File sourceFile = temp.newFile("shouldOverwriteFileWhenRequested-source.bin"); - try (OutputStream fStream = new FileOutputStream(sourceFile)) { - IoUtils.copy(new ByteArrayInputStream(sourceBytes), fStream); - } + final byte[] sourceBytes = generateBytes(32); + File sourceFile = newTempFile("shouldAtomicOverwriteFileWhenRequested-source.bin", sourceBytes); // create target file - final byte[] targetBytes = new byte[32]; - new SecureRandom(new byte[]{31}).nextBytes(targetBytes); - - File targetFile = temp.newFile("shouldOverwriteFileWhenRequested-target.bin"); - try (OutputStream fStream = new FileOutputStream(targetFile)) { - IoUtils.copy(new ByteArrayInputStream(targetBytes), fStream); - } + final byte[] targetBytes = generateBytes(32); + File targetFile = newTempFile("shouldAtomicOverwriteFileWhenRequested-target.bin", targetBytes); // rename with overwrite Set flags = EnumSet.of(RenameFlags.OVERWRITE); - sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); + sftpRenameFile(sourceFile, targetFile, flags); // check if rename was successful assertThat("The source file should not exist anymore", !sourceFile.exists()); assertThat("The contents of the target file should be equal to the contents previously written " + - "to the source file", - ByteArrayUtils.equals(IoUtils.toByteArray(new FileInputStream(targetFile)), 0, - sourceBytes, 0, sourceBytes.length)); + "to the source file", fileContentEquals(targetFile, sourceBytes)); } @Test public void shouldNotOverwriteFileWhenNotRequested() throws IOException { - SSHClient ssh = fixture.setupConnectedDefaultClient(); - ssh.authPassword("test", "test"); - SFTPEngine sftp = new SFTPEngine(ssh).init(); - // create source file - final byte[] sourceBytes = new byte[32]; - new SecureRandom(new byte[]{31}).nextBytes(sourceBytes); - - File sourceFile = temp.newFile("shouldOverwriteFileWhenRequested-source.bin"); - try (OutputStream fStream = new FileOutputStream(sourceFile)) { - IoUtils.copy(new ByteArrayInputStream(sourceBytes), fStream); - } + final byte[] sourceBytes = generateBytes(32); + File sourceFile = newTempFile("shouldAtomicOverwriteFileWhenRequested-source.bin", sourceBytes); // create target file - final byte[] targetBytes = new byte[32]; - new SecureRandom(new byte[]{31}).nextBytes(targetBytes); - - File targetFile = temp.newFile("shouldOverwriteFileWhenRequested-target.bin"); - try (OutputStream fStream = new FileOutputStream(targetFile)) { - IoUtils.copy(new ByteArrayInputStream(targetBytes), fStream); - } + final byte[] targetBytes = generateBytes(32); + File targetFile = newTempFile("shouldAtomicOverwriteFileWhenRequested-target.bin", targetBytes); // rename without overwrite -> should fail Boolean exceptionThrown = false; try { Set flags = new HashSet<>(); - sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); + sftpRenameFile(sourceFile, targetFile, flags); } catch (net.schmizz.sshj.sftp.SFTPException e) { exceptionThrown = true; @@ -298,8 +268,36 @@ public void shouldNotOverwriteFileWhenNotRequested() throws IOException { // check if rename failed as it should assertThat("The source file should still exist", sourceFile.exists()); assertThat("The contents of the target file should be equal to the contents previously written to it", - ByteArrayUtils.equals(IoUtils.toByteArray(new FileInputStream(targetFile)), 0, - targetBytes, 0, targetBytes.length)); + fileContentEquals(targetFile, targetBytes)); assertThat("An appropriate exception should have been thrown", exceptionThrown); } + + private byte[] generateBytes(Integer size) { + byte[] randomBytes = new byte[size]; + Random rnd = new Random(); + rnd.nextBytes(randomBytes); + return randomBytes; + } + + private File newTempFile(String name, byte[] content) throws IOException { + File tmpFile = new File(temp, name); + try (OutputStream fStream = new FileOutputStream(tmpFile)) { + IoUtils.copy(new ByteArrayInputStream(content), fStream); + } + return tmpFile; + } + + private boolean fileContentEquals(File testFile, byte[] testBytes) throws IOException { + return ByteArrayUtils.equals( + IoUtils.toByteArray(new FileInputStream(testFile)), 0, + testBytes, 0, + testBytes.length); + } + + private void sftpRenameFile(File sourceFile, File targetFile, Set flags) throws IOException { + SSHClient ssh = fixture.setupConnectedDefaultClient(); + ssh.authPassword("test", "test"); + SFTPEngine sftp = new SFTPEngine(ssh).init(); + sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); + } } From c6099d988c7035a288af5d6a18e675e34e53abd3 Mon Sep 17 00:00:00 2001 From: Florian Klemenz Date: Fri, 20 Oct 2023 10:57:01 +0200 Subject: [PATCH 07/10] adds basic test for atomic rewrite --- .../hierynomus/sshj/sftp/RemoteFileTest.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java b/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java index 6ef45b07..11f84eda 100644 --- a/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java +++ b/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java @@ -272,6 +272,26 @@ public void shouldNotOverwriteFileWhenNotRequested() throws IOException { assertThat("An appropriate exception should have been thrown", exceptionThrown); } + @Test + public void shouldUseAtomicRenameWhenRequested() throws IOException { + // create source file + final byte[] sourceBytes = generateBytes(32); + File sourceFile = newTempFile("shouldAtomicOverwriteFileWhenRequested-source.bin", sourceBytes); + + // create target file + final byte[] targetBytes = generateBytes(32); + File targetFile = newTempFile("shouldAtomicOverwriteFileWhenRequested-target.bin", targetBytes); + + // atomic rename with overwrite -> should work + Set flags = EnumSet.of(RenameFlags.OVERWRITE, RenameFlags.ATOMIC); + sftpRenameFile(sourceFile, targetFile, flags); + + assertThat("The source file should not exist anymore", !sourceFile.exists()); + assertThat("The contents of the target file should be equal to the contents previously written " + + "to the source file", fileContentEquals(targetFile, sourceBytes)); + + } + private byte[] generateBytes(Integer size) { byte[] randomBytes = new byte[size]; Random rnd = new Random(); From 23c538dcb1de25f7b641706865c1acb7a4855d40 Mon Sep 17 00:00:00 2001 From: Florian Klemenz Date: Fri, 20 Oct 2023 12:07:43 +0200 Subject: [PATCH 08/10] adds possibility to request a specific client version (e.g. for testing purposes) --- .../java/net/schmizz/sshj/sftp/SFTPEngine.java | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java b/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java index b681e451..83f10a17 100644 --- a/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java +++ b/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java @@ -81,7 +81,18 @@ public String canonicalize(String path) public SFTPEngine init() throws IOException { - transmit(new SFTPPacket(PacketType.INIT).putUInt32(MAX_SUPPORTED_VERSION)); + return init(MAX_SUPPORTED_VERSION); + } + + public SFTPEngine init(int requestedVersion) + throws IOException { + if (requestedVersion > MAX_SUPPORTED_VERSION) + throw new SFTPException("You requested an unsupported protocol version: " + requestedVersion + " (requested) > " + MAX_SUPPORTED_VERSION + " (supported)"); + + if (requestedVersion < MAX_SUPPORTED_VERSION) + log.debug("Client version {} is smaller than MAX_SUPPORTED_VERSION {}", requestedVersion, MAX_SUPPORTED_VERSION); + + transmit(new SFTPPacket(PacketType.INIT).putUInt32(requestedVersion)); final SFTPPacket response = reader.readPacket(); @@ -91,7 +102,7 @@ public SFTPEngine init() operativeVersion = response.readUInt32AsInt(); log.debug("Server version {}", operativeVersion); - if (MAX_SUPPORTED_VERSION < operativeVersion) + if (requestedVersion < operativeVersion) throw new SFTPException("Server reported incompatible protocol version: " + operativeVersion); while (response.available() > 0) From 981d2686f30bc145ce49f940b465711e6a5870c0 Mon Sep 17 00:00:00 2001 From: Florian Klemenz Date: Fri, 20 Oct 2023 12:27:15 +0200 Subject: [PATCH 09/10] adds testcases for SFTP rename flags fallback behaviour --- .../hierynomus/sshj/sftp/RemoteFileTest.java | 123 ++++++++++++++++-- 1 file changed, 109 insertions(+), 14 deletions(-) diff --git a/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java b/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java index 11f84eda..37401b85 100644 --- a/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java +++ b/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java @@ -229,15 +229,16 @@ rf.new ReadAheadRemoteFileInputStream(10), public void shouldOverwriteFileWhenRequested() throws IOException { // create source file final byte[] sourceBytes = generateBytes(32); - File sourceFile = newTempFile("shouldAtomicOverwriteFileWhenRequested-source.bin", sourceBytes); + File sourceFile = newTempFile("shouldOverwriteFileWhenRequested-source.bin", sourceBytes); // create target file final byte[] targetBytes = generateBytes(32); - File targetFile = newTempFile("shouldAtomicOverwriteFileWhenRequested-target.bin", targetBytes); + File targetFile = newTempFile("shouldOverwriteFileWhenRequested-target.bin", targetBytes); // rename with overwrite Set flags = EnumSet.of(RenameFlags.OVERWRITE); - sftpRenameFile(sourceFile, targetFile, flags); + SFTPEngine sftp = sftpInit(); + sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); // check if rename was successful assertThat("The source file should not exist anymore", !sourceFile.exists()); @@ -249,17 +250,18 @@ public void shouldOverwriteFileWhenRequested() throws IOException { public void shouldNotOverwriteFileWhenNotRequested() throws IOException { // create source file final byte[] sourceBytes = generateBytes(32); - File sourceFile = newTempFile("shouldAtomicOverwriteFileWhenRequested-source.bin", sourceBytes); + File sourceFile = newTempFile("shouldNotOverwriteFileWhenNotRequested-source.bin", sourceBytes); // create target file final byte[] targetBytes = generateBytes(32); - File targetFile = newTempFile("shouldAtomicOverwriteFileWhenRequested-target.bin", targetBytes); + File targetFile = newTempFile("shouldNotOverwriteFileWhenNotRequested-target.bin", targetBytes); // rename without overwrite -> should fail Boolean exceptionThrown = false; + Set flags = new HashSet<>(); + SFTPEngine sftp = sftpInit(); try { - Set flags = new HashSet<>(); - sftpRenameFile(sourceFile, targetFile, flags); + sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); } catch (net.schmizz.sshj.sftp.SFTPException e) { exceptionThrown = true; @@ -273,23 +275,112 @@ public void shouldNotOverwriteFileWhenNotRequested() throws IOException { } @Test - public void shouldUseAtomicRenameWhenRequested() throws IOException { + public void shouldUseAtomicRenameWhenRequestedWithOverwriteOnInsufficientProtocolVersion() throws IOException { // create source file final byte[] sourceBytes = generateBytes(32); - File sourceFile = newTempFile("shouldAtomicOverwriteFileWhenRequested-source.bin", sourceBytes); + File sourceFile = newTempFile("shouldUseAtomicRenameWhenRequestedWithOverwriteOnInsufficientProtocolVersion-source.bin", sourceBytes); // create target file final byte[] targetBytes = generateBytes(32); - File targetFile = newTempFile("shouldAtomicOverwriteFileWhenRequested-target.bin", targetBytes); + File targetFile = newTempFile("shouldUseAtomicRenameWhenRequestedWithOverwriteOnInsufficientProtocolVersion-target.bin", targetBytes); // atomic rename with overwrite -> should work Set flags = EnumSet.of(RenameFlags.OVERWRITE, RenameFlags.ATOMIC); - sftpRenameFile(sourceFile, targetFile, flags); + int version = Math.min(SFTPEngine.MAX_SUPPORTED_VERSION, 4); // choose a supported version smaller than 5 + SFTPEngine sftp = sftpInit(version); + sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); + assertThat("The connection should use the requested protocol version", sftp.getOperativeProtocolVersion() == version); assertThat("The source file should not exist anymore", !sourceFile.exists()); assertThat("The contents of the target file should be equal to the contents previously written " + "to the source file", fileContentEquals(targetFile, sourceBytes)); + } + + + @Test + public void shouldIgnoreAtomicFlagWhenRequestedWithNativeOnInsufficientProtocolVersion() throws IOException { + // create source file + final byte[] sourceBytes = generateBytes(32); + File sourceFile = newTempFile("shouldIgnoreAtomicFlagWhenRequestedWithNativeOnInsufficientProtocolVersion-source.bin", sourceBytes); + + // create target file + final byte[] targetBytes = generateBytes(32); + File targetFile = newTempFile("shouldIgnoreAtomicFlagWhenRequestedWithNativeOnInsufficientProtocolVersion-target.bin", targetBytes); + + // atomic flag should be ignored with native + // -> should fail because target exists and overwrite behaviour is not requested + Boolean exceptionThrown = false; + Set flags = EnumSet.of(RenameFlags.NATIVE, RenameFlags.ATOMIC); + int version = Math.min(SFTPEngine.MAX_SUPPORTED_VERSION, 4); // choose a supported version smaller than 5 + SFTPEngine sftp = sftpInit(version); + try { + sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); + } + catch (net.schmizz.sshj.sftp.SFTPException e) { + exceptionThrown = true; + } + + assertThat("The connection should use the requested protocol version", sftp.getOperativeProtocolVersion() == version); + assertThat("The source file should still exist", sourceFile.exists()); + assertThat("The contents of the target file should be equal to the contents previously written to it", + fileContentEquals(targetFile, targetBytes)); + assertThat("An appropriate exception should have been thrown", exceptionThrown); + + } + + @Test + public void shouldFailAtomicRenameWithoutOverwriteOnInsufficientProtocolVersion() throws IOException { + // create source file + final byte[] sourceBytes = generateBytes(32); + File sourceFile = newTempFile("shouldFailAtomicRenameWithoutOverwriteOnInsufficientProtocolVersion-source.bin", sourceBytes); + + // create target file + File targetFile = new File(temp, "shouldFailAtomicRenameWithoutOverwriteOnInsufficientProtocolVersion-target.bin"); + + // atomic rename without overwrite -> should fail + Boolean exceptionThrown = false; + Set flags = EnumSet.of(RenameFlags.ATOMIC); + int version = Math.min(SFTPEngine.MAX_SUPPORTED_VERSION, 4); // choose a supported version smaller than 5 + SFTPEngine sftp = sftpInit(version); + try { + sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); + } + catch (net.schmizz.sshj.sftp.SFTPException e) { + exceptionThrown = true; + } + + // check if rename failed as it should (for version < 5) + assertThat("The connection should use the requested protocol version", sftp.getOperativeProtocolVersion() == version); + assertThat("The source file should still exist", sourceFile.exists()); + assertThat("The target file should not exist", !targetFile.exists()); + assertThat("An appropriate exception should have been thrown", exceptionThrown); + } + + @Test + public void shouldDoAtomicRenameOnSufficientProtocolVersion() throws IOException { + // This test will be relevant as soon as sshj supports SFTP protocol version >= 5 + if (SFTPEngine.MAX_SUPPORTED_VERSION >= 5) { + // create source file + final byte[] sourceBytes = generateBytes(32); + File sourceFile = newTempFile("shouldDoAtomicRenameOnSufficientProtocolVersion-source.bin", sourceBytes); + + // create target file + File targetFile = new File(temp, "shouldDoAtomicRenameOnSufficientProtocolVersion-target.bin"); + + // atomic rename without overwrite -> should work on version >= 5 + Set flags = EnumSet.of(RenameFlags.ATOMIC); + SFTPEngine sftp = sftpInit(); + sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); + + // check if rename worked as it should (for version >= 5) + assertThat("The connection should use the requested protocol version", sftp.getOperativeProtocolVersion() >= 5); + assertThat("The source file should not exist anymore", !sourceFile.exists()); + assertThat("The target file should exist", targetFile.exists()); + } + else { + // Ignored - cannot test because client does not support protocol version >= 5 + } } private byte[] generateBytes(Integer size) { @@ -314,10 +405,14 @@ private boolean fileContentEquals(File testFile, byte[] testBytes) throws IOExce testBytes.length); } - private void sftpRenameFile(File sourceFile, File targetFile, Set flags) throws IOException { + private SFTPEngine sftpInit() throws IOException { + return sftpInit(SFTPEngine.MAX_SUPPORTED_VERSION); + } + + private SFTPEngine sftpInit(int version) throws IOException { SSHClient ssh = fixture.setupConnectedDefaultClient(); ssh.authPassword("test", "test"); - SFTPEngine sftp = new SFTPEngine(ssh).init(); - sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); + SFTPEngine sftp = new SFTPEngine(ssh).init(version); + return sftp; } } From ab6f9941a5ee734c18ea93bc7c9a4120aaf621d6 Mon Sep 17 00:00:00 2001 From: Florian Klemenz Date: Mon, 23 Oct 2023 10:32:18 +0200 Subject: [PATCH 10/10] refactoring to make SFTPEngine.init(int requestedVersion) protected --- .../net/schmizz/sshj/sftp/SFTPEngine.java | 7 +- .../hierynomus/sshj/sftp/RemoteFileTest.java | 198 +-------------- .../sshj/sftp/RemoteFileRenameTest.java | 237 ++++++++++++++++++ 3 files changed, 247 insertions(+), 195 deletions(-) create mode 100644 src/test/java/net/schmizz/sshj/sftp/RemoteFileRenameTest.java diff --git a/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java b/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java index 83f10a17..ecac5afc 100644 --- a/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java +++ b/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java @@ -84,7 +84,12 @@ public SFTPEngine init() return init(MAX_SUPPORTED_VERSION); } - public SFTPEngine init(int requestedVersion) + /** + * Introduced for internal use by testcases. + * @param requestedVersion + * @throws IOException + */ + protected SFTPEngine init(int requestedVersion) throws IOException { if (requestedVersion > MAX_SUPPORTED_VERSION) throw new SFTPException("You requested an unsupported protocol version: " + requestedVersion + " (requested) > " + MAX_SUPPORTED_VERSION + " (supported)"); diff --git a/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java b/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java index 37401b85..b105962b 100644 --- a/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java +++ b/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java @@ -18,7 +18,10 @@ import com.hierynomus.sshj.test.SshServerExtension; import net.schmizz.sshj.SSHClient; import net.schmizz.sshj.common.ByteArrayUtils; -import net.schmizz.sshj.sftp.*; +import net.schmizz.sshj.sftp.OpenMode; +import net.schmizz.sshj.sftp.RemoteFile; +import net.schmizz.sshj.sftp.SFTPEngine; +import net.schmizz.sshj.sftp.SFTPException; import org.apache.sshd.common.util.io.IoUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -27,9 +30,7 @@ import java.io.*; import java.security.SecureRandom; import java.util.EnumSet; -import java.util.HashSet; import java.util.Random; -import java.util.Set; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; @@ -224,195 +225,4 @@ rf.new ReadAheadRemoteFileInputStream(10), assertThat("The file should be read correctly", ByteArrayUtils.equals(expected, 0, actual, 0, expected.length)); } - - @Test - public void shouldOverwriteFileWhenRequested() throws IOException { - // create source file - final byte[] sourceBytes = generateBytes(32); - File sourceFile = newTempFile("shouldOverwriteFileWhenRequested-source.bin", sourceBytes); - - // create target file - final byte[] targetBytes = generateBytes(32); - File targetFile = newTempFile("shouldOverwriteFileWhenRequested-target.bin", targetBytes); - - // rename with overwrite - Set flags = EnumSet.of(RenameFlags.OVERWRITE); - SFTPEngine sftp = sftpInit(); - sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); - - // check if rename was successful - assertThat("The source file should not exist anymore", !sourceFile.exists()); - assertThat("The contents of the target file should be equal to the contents previously written " + - "to the source file", fileContentEquals(targetFile, sourceBytes)); - } - - @Test - public void shouldNotOverwriteFileWhenNotRequested() throws IOException { - // create source file - final byte[] sourceBytes = generateBytes(32); - File sourceFile = newTempFile("shouldNotOverwriteFileWhenNotRequested-source.bin", sourceBytes); - - // create target file - final byte[] targetBytes = generateBytes(32); - File targetFile = newTempFile("shouldNotOverwriteFileWhenNotRequested-target.bin", targetBytes); - - // rename without overwrite -> should fail - Boolean exceptionThrown = false; - Set flags = new HashSet<>(); - SFTPEngine sftp = sftpInit(); - try { - sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); - } - catch (net.schmizz.sshj.sftp.SFTPException e) { - exceptionThrown = true; - } - - // check if rename failed as it should - assertThat("The source file should still exist", sourceFile.exists()); - assertThat("The contents of the target file should be equal to the contents previously written to it", - fileContentEquals(targetFile, targetBytes)); - assertThat("An appropriate exception should have been thrown", exceptionThrown); - } - - @Test - public void shouldUseAtomicRenameWhenRequestedWithOverwriteOnInsufficientProtocolVersion() throws IOException { - // create source file - final byte[] sourceBytes = generateBytes(32); - File sourceFile = newTempFile("shouldUseAtomicRenameWhenRequestedWithOverwriteOnInsufficientProtocolVersion-source.bin", sourceBytes); - - // create target file - final byte[] targetBytes = generateBytes(32); - File targetFile = newTempFile("shouldUseAtomicRenameWhenRequestedWithOverwriteOnInsufficientProtocolVersion-target.bin", targetBytes); - - // atomic rename with overwrite -> should work - Set flags = EnumSet.of(RenameFlags.OVERWRITE, RenameFlags.ATOMIC); - int version = Math.min(SFTPEngine.MAX_SUPPORTED_VERSION, 4); // choose a supported version smaller than 5 - SFTPEngine sftp = sftpInit(version); - sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); - - assertThat("The connection should use the requested protocol version", sftp.getOperativeProtocolVersion() == version); - assertThat("The source file should not exist anymore", !sourceFile.exists()); - assertThat("The contents of the target file should be equal to the contents previously written " + - "to the source file", fileContentEquals(targetFile, sourceBytes)); - } - - - @Test - public void shouldIgnoreAtomicFlagWhenRequestedWithNativeOnInsufficientProtocolVersion() throws IOException { - // create source file - final byte[] sourceBytes = generateBytes(32); - File sourceFile = newTempFile("shouldIgnoreAtomicFlagWhenRequestedWithNativeOnInsufficientProtocolVersion-source.bin", sourceBytes); - - // create target file - final byte[] targetBytes = generateBytes(32); - File targetFile = newTempFile("shouldIgnoreAtomicFlagWhenRequestedWithNativeOnInsufficientProtocolVersion-target.bin", targetBytes); - - // atomic flag should be ignored with native - // -> should fail because target exists and overwrite behaviour is not requested - Boolean exceptionThrown = false; - Set flags = EnumSet.of(RenameFlags.NATIVE, RenameFlags.ATOMIC); - int version = Math.min(SFTPEngine.MAX_SUPPORTED_VERSION, 4); // choose a supported version smaller than 5 - SFTPEngine sftp = sftpInit(version); - try { - sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); - } - catch (net.schmizz.sshj.sftp.SFTPException e) { - exceptionThrown = true; - } - - assertThat("The connection should use the requested protocol version", sftp.getOperativeProtocolVersion() == version); - assertThat("The source file should still exist", sourceFile.exists()); - assertThat("The contents of the target file should be equal to the contents previously written to it", - fileContentEquals(targetFile, targetBytes)); - assertThat("An appropriate exception should have been thrown", exceptionThrown); - - } - - - @Test - public void shouldFailAtomicRenameWithoutOverwriteOnInsufficientProtocolVersion() throws IOException { - // create source file - final byte[] sourceBytes = generateBytes(32); - File sourceFile = newTempFile("shouldFailAtomicRenameWithoutOverwriteOnInsufficientProtocolVersion-source.bin", sourceBytes); - - // create target file - File targetFile = new File(temp, "shouldFailAtomicRenameWithoutOverwriteOnInsufficientProtocolVersion-target.bin"); - - // atomic rename without overwrite -> should fail - Boolean exceptionThrown = false; - Set flags = EnumSet.of(RenameFlags.ATOMIC); - int version = Math.min(SFTPEngine.MAX_SUPPORTED_VERSION, 4); // choose a supported version smaller than 5 - SFTPEngine sftp = sftpInit(version); - try { - sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); - } - catch (net.schmizz.sshj.sftp.SFTPException e) { - exceptionThrown = true; - } - - // check if rename failed as it should (for version < 5) - assertThat("The connection should use the requested protocol version", sftp.getOperativeProtocolVersion() == version); - assertThat("The source file should still exist", sourceFile.exists()); - assertThat("The target file should not exist", !targetFile.exists()); - assertThat("An appropriate exception should have been thrown", exceptionThrown); - } - - @Test - public void shouldDoAtomicRenameOnSufficientProtocolVersion() throws IOException { - // This test will be relevant as soon as sshj supports SFTP protocol version >= 5 - if (SFTPEngine.MAX_SUPPORTED_VERSION >= 5) { - // create source file - final byte[] sourceBytes = generateBytes(32); - File sourceFile = newTempFile("shouldDoAtomicRenameOnSufficientProtocolVersion-source.bin", sourceBytes); - - // create target file - File targetFile = new File(temp, "shouldDoAtomicRenameOnSufficientProtocolVersion-target.bin"); - - // atomic rename without overwrite -> should work on version >= 5 - Set flags = EnumSet.of(RenameFlags.ATOMIC); - SFTPEngine sftp = sftpInit(); - sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); - - // check if rename worked as it should (for version >= 5) - assertThat("The connection should use the requested protocol version", sftp.getOperativeProtocolVersion() >= 5); - assertThat("The source file should not exist anymore", !sourceFile.exists()); - assertThat("The target file should exist", targetFile.exists()); - } - else { - // Ignored - cannot test because client does not support protocol version >= 5 - } - } - - private byte[] generateBytes(Integer size) { - byte[] randomBytes = new byte[size]; - Random rnd = new Random(); - rnd.nextBytes(randomBytes); - return randomBytes; - } - - private File newTempFile(String name, byte[] content) throws IOException { - File tmpFile = new File(temp, name); - try (OutputStream fStream = new FileOutputStream(tmpFile)) { - IoUtils.copy(new ByteArrayInputStream(content), fStream); - } - return tmpFile; - } - - private boolean fileContentEquals(File testFile, byte[] testBytes) throws IOException { - return ByteArrayUtils.equals( - IoUtils.toByteArray(new FileInputStream(testFile)), 0, - testBytes, 0, - testBytes.length); - } - - private SFTPEngine sftpInit() throws IOException { - return sftpInit(SFTPEngine.MAX_SUPPORTED_VERSION); - } - - private SFTPEngine sftpInit(int version) throws IOException { - SSHClient ssh = fixture.setupConnectedDefaultClient(); - ssh.authPassword("test", "test"); - SFTPEngine sftp = new SFTPEngine(ssh).init(version); - return sftp; - } } diff --git a/src/test/java/net/schmizz/sshj/sftp/RemoteFileRenameTest.java b/src/test/java/net/schmizz/sshj/sftp/RemoteFileRenameTest.java new file mode 100644 index 00000000..8f3e6a44 --- /dev/null +++ b/src/test/java/net/schmizz/sshj/sftp/RemoteFileRenameTest.java @@ -0,0 +1,237 @@ +/* + * Copyright (C)2009 - SSHJ Contributors + * + * Licensed 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 net.schmizz.sshj.sftp; + +import com.hierynomus.sshj.test.SshServerExtension; +import net.schmizz.sshj.SSHClient; +import net.schmizz.sshj.common.ByteArrayUtils; +import org.apache.sshd.common.util.io.IoUtils; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.api.io.TempDir; + +import java.io.*; +import java.util.EnumSet; +import java.util.HashSet; +import java.util.Random; +import java.util.Set; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Testing of remote file rename using different combinations of net.schmizz.sshj.sftp.RenameFlags with + * possible workarounds for SFTP protocol versions lower than 5 that do not natively support these flags. + */ +public class RemoteFileRenameTest { + @RegisterExtension + public SshServerExtension fixture = new SshServerExtension(); + + @TempDir + public File temp; + + @Test + public void shouldOverwriteFileWhenRequested() throws IOException { + // create source file + final byte[] sourceBytes = generateBytes(32); + File sourceFile = newTempFile("shouldOverwriteFileWhenRequested-source.bin", sourceBytes); + + // create target file + final byte[] targetBytes = generateBytes(32); + File targetFile = newTempFile("shouldOverwriteFileWhenRequested-target.bin", targetBytes); + + // rename with overwrite + Set flags = EnumSet.of(RenameFlags.OVERWRITE); + SFTPEngine sftp = sftpInit(); + sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); + + // check if rename was successful + assertThat("The source file should not exist anymore", !sourceFile.exists()); + assertThat("The contents of the target file should be equal to the contents previously written " + + "to the source file", fileContentEquals(targetFile, sourceBytes)); + } + + @Test + public void shouldNotOverwriteFileWhenNotRequested() throws IOException { + // create source file + final byte[] sourceBytes = generateBytes(32); + File sourceFile = newTempFile("shouldNotOverwriteFileWhenNotRequested-source.bin", sourceBytes); + + // create target file + final byte[] targetBytes = generateBytes(32); + File targetFile = newTempFile("shouldNotOverwriteFileWhenNotRequested-target.bin", targetBytes); + + // rename without overwrite -> should fail + Boolean exceptionThrown = false; + Set flags = new HashSet<>(); + SFTPEngine sftp = sftpInit(); + try { + sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); + } + catch (SFTPException e) { + exceptionThrown = true; + } + + // check if rename failed as it should + assertThat("The source file should still exist", sourceFile.exists()); + assertThat("The contents of the target file should be equal to the contents previously written to it", + fileContentEquals(targetFile, targetBytes)); + assertThat("An appropriate exception should have been thrown", exceptionThrown); + } + + @Test + public void shouldUseAtomicRenameWhenRequestedWithOverwriteOnInsufficientProtocolVersion() throws IOException { + // create source file + final byte[] sourceBytes = generateBytes(32); + File sourceFile = newTempFile("shouldUseAtomicRenameWhenRequestedWithOverwriteOnInsufficientProtocolVersion-source.bin", sourceBytes); + + // create target file + final byte[] targetBytes = generateBytes(32); + File targetFile = newTempFile("shouldUseAtomicRenameWhenRequestedWithOverwriteOnInsufficientProtocolVersion-target.bin", targetBytes); + + // atomic rename with overwrite -> should work + Set flags = EnumSet.of(RenameFlags.OVERWRITE, RenameFlags.ATOMIC); + int version = Math.min(SFTPEngine.MAX_SUPPORTED_VERSION, 4); // choose a supported version smaller than 5 + SFTPEngine sftp = sftpInit(version); + sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); + + assertThat("The connection should use the requested protocol version", sftp.getOperativeProtocolVersion() == version); + assertThat("The source file should not exist anymore", !sourceFile.exists()); + assertThat("The contents of the target file should be equal to the contents previously written " + + "to the source file", fileContentEquals(targetFile, sourceBytes)); + } + + + @Test + public void shouldIgnoreAtomicFlagWhenRequestedWithNativeOnInsufficientProtocolVersion() throws IOException { + // create source file + final byte[] sourceBytes = generateBytes(32); + File sourceFile = newTempFile("shouldIgnoreAtomicFlagWhenRequestedWithNativeOnInsufficientProtocolVersion-source.bin", sourceBytes); + + // create target file + final byte[] targetBytes = generateBytes(32); + File targetFile = newTempFile("shouldIgnoreAtomicFlagWhenRequestedWithNativeOnInsufficientProtocolVersion-target.bin", targetBytes); + + // atomic flag should be ignored with native + // -> should fail because target exists and overwrite behaviour is not requested + Boolean exceptionThrown = false; + Set flags = EnumSet.of(RenameFlags.NATIVE, RenameFlags.ATOMIC); + int version = Math.min(SFTPEngine.MAX_SUPPORTED_VERSION, 4); // choose a supported version smaller than 5 + SFTPEngine sftp = sftpInit(version); + try { + sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); + } + catch (SFTPException e) { + exceptionThrown = true; + } + + assertThat("The connection should use the requested protocol version", sftp.getOperativeProtocolVersion() == version); + assertThat("The source file should still exist", sourceFile.exists()); + assertThat("The contents of the target file should be equal to the contents previously written to it", + fileContentEquals(targetFile, targetBytes)); + assertThat("An appropriate exception should have been thrown", exceptionThrown); + + } + + + @Test + public void shouldFailAtomicRenameWithoutOverwriteOnInsufficientProtocolVersion() throws IOException { + // create source file + final byte[] sourceBytes = generateBytes(32); + File sourceFile = newTempFile("shouldFailAtomicRenameWithoutOverwriteOnInsufficientProtocolVersion-source.bin", sourceBytes); + + // create target file + File targetFile = new File(temp, "shouldFailAtomicRenameWithoutOverwriteOnInsufficientProtocolVersion-target.bin"); + + // atomic rename without overwrite -> should fail + Boolean exceptionThrown = false; + Set flags = EnumSet.of(RenameFlags.ATOMIC); + int version = Math.min(SFTPEngine.MAX_SUPPORTED_VERSION, 4); // choose a supported version smaller than 5 + SFTPEngine sftp = sftpInit(version); + try { + sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); + } + catch (SFTPException e) { + exceptionThrown = true; + } + + // check if rename failed as it should (for version < 5) + assertThat("The connection should use the requested protocol version", sftp.getOperativeProtocolVersion() == version); + assertThat("The source file should still exist", sourceFile.exists()); + assertThat("The target file should not exist", !targetFile.exists()); + assertThat("An appropriate exception should have been thrown", exceptionThrown); + } + + @Test + public void shouldDoAtomicRenameOnSufficientProtocolVersion() throws IOException { + // This test will be relevant as soon as sshj supports SFTP protocol version >= 5 + if (SFTPEngine.MAX_SUPPORTED_VERSION >= 5) { + // create source file + final byte[] sourceBytes = generateBytes(32); + File sourceFile = newTempFile("shouldDoAtomicRenameOnSufficientProtocolVersion-source.bin", sourceBytes); + + // create target file + File targetFile = new File(temp, "shouldDoAtomicRenameOnSufficientProtocolVersion-target.bin"); + + // atomic rename without overwrite -> should work on version >= 5 + Set flags = EnumSet.of(RenameFlags.ATOMIC); + SFTPEngine sftp = sftpInit(); + sftp.rename(sourceFile.getPath(), targetFile.getPath(), flags); + + // check if rename worked as it should (for version >= 5) + assertThat("The connection should use the requested protocol version", sftp.getOperativeProtocolVersion() >= 5); + assertThat("The source file should not exist anymore", !sourceFile.exists()); + assertThat("The target file should exist", targetFile.exists()); + } + else { + // Ignored - cannot test because client does not support protocol version >= 5 + } + } + + private byte[] generateBytes(Integer size) { + byte[] randomBytes = new byte[size]; + Random rnd = new Random(); + rnd.nextBytes(randomBytes); + return randomBytes; + } + + private File newTempFile(String name, byte[] content) throws IOException { + File tmpFile = new File(temp, name); + try (OutputStream fStream = new FileOutputStream(tmpFile)) { + IoUtils.copy(new ByteArrayInputStream(content), fStream); + } + return tmpFile; + } + + private boolean fileContentEquals(File testFile, byte[] testBytes) throws IOException { + return ByteArrayUtils.equals( + IoUtils.toByteArray(new FileInputStream(testFile)), 0, + testBytes, 0, + testBytes.length); + } + + private SFTPEngine sftpInit() throws IOException { + return sftpInit(SFTPEngine.MAX_SUPPORTED_VERSION); + } + + private SFTPEngine sftpInit(int version) throws IOException { + SSHClient ssh = fixture.setupConnectedDefaultClient(); + ssh.authPassword("test", "test"); + SFTPEngine sftp = new SFTPEngine(ssh).init(version); + return sftp; + } +}