Skip to content

Commit

Permalink
adds fallback to posix-rename@openssh.com extension if possible and c… (
Browse files Browse the repository at this point in the history
#827)

* adds fallback to posix-rename@openssh.com extension if possible and communicates possible problems with flags to the developer

* Adds '{}' around if/else statements

* adds basic tests for file rename

* fix comments

* fixes indentation

* adds helper methods to make existing sftp rename tests more concise

* adds basic test for atomic rewrite

* adds possibility to request a specific client version (e.g. for testing purposes)

* adds testcases for SFTP rename flags fallback behaviour

* refactoring to make SFTPEngine.init(int requestedVersion) protected

---------

Co-authored-by: Florian Klemenz <florian.klemenz@fau.de>
Co-authored-by: Jeroen van Erp <jeroen@hierynomus.com>
  • Loading branch information
3 people authored Oct 23, 2023
1 parent 542bb35 commit 4774721
Show file tree
Hide file tree
Showing 2 changed files with 321 additions and 9 deletions.
93 changes: 84 additions & 9 deletions src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,23 @@ public String canonicalize(String path)

public SFTPEngine init()
throws IOException {
transmit(new SFTPPacket<Request>(PacketType.INIT).putUInt32(MAX_SUPPORTED_VERSION));
return init(MAX_SUPPORTED_VERSION);
}

/**
* 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)");

if (requestedVersion < MAX_SUPPORTED_VERSION)
log.debug("Client version {} is smaller than MAX_SUPPORTED_VERSION {}", requestedVersion, MAX_SUPPORTED_VERSION);

transmit(new SFTPPacket<Request>(PacketType.INIT).putUInt32(requestedVersion));

final SFTPPacket<Response> response = reader.readPacket();

Expand All @@ -91,7 +107,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)
Expand Down Expand Up @@ -234,16 +250,75 @@ public FileAttributes lstat(String path)

public void rename(String oldPath, String newPath, Set<RenameFlags> flags)
throws IOException {
if (operativeVersion < 1)
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();
}
}
// 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);
}

Expand Down
237 changes: 237 additions & 0 deletions src/test/java/net/schmizz/sshj/sftp/RemoteFileRenameTest.java
Original file line number Diff line number Diff line change
@@ -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<RenameFlags> 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<RenameFlags> 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<RenameFlags> 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<RenameFlags> 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<RenameFlags> 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<RenameFlags> 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;
}
}

0 comments on commit 4774721

Please sign in to comment.