Added SFTP file transfer resume support on both PUT and GET. (#775)

* Added SFTP file transfer resume support on both PUT and GET. Internally SFTPFileTransfer has a few sanity checks to fall back to full replacement even if the resume flag is set. 

SCP file transfers have not been changed to support this at this time.

* Added JUnit tests for issue-700

* Throw SCPException when attempting to resume SCP transfers.

* Licensing

* Small bug resuming a completed file was restarting since the bytes were equal.

* Enhanced test cases to validate the expected bytes transferred for each scenario are the actual bytes transferred.

* Removed author info which was pre-filled from company IDE template

* Added "fall through" comment for switch

* Changed the API for requesting a resume from a boolean flag with some internal decisions to be a user-specified long byte offset. This is cleaner but puts the onus on the caller to know exactly what they're asking for in their circumstance, which is ultimately better for a library like sshj.

* Reverted some now-unnecessary changes to SFTPFileTransfer.Uploader.prepareFile()

* Fix gradle exclude path for test files

Co-authored-by: Jeroen van Erp <jeroen@hierynomus.com>
This commit is contained in:
Brent Tyler
2022-05-27 06:05:41 -05:00
committed by GitHub
parent d7e402c557
commit 3de0302c84
12 changed files with 398 additions and 27 deletions

View File

@@ -65,7 +65,12 @@ license {
mapping {
java = 'SLASHSTAR_STYLE'
}
excludes(['**/djb/Curve25519.java', '**/sshj/common/Base64.java', '**/com/hierynomus/sshj/userauth/keyprovider/bcrypt/*.java'])
excludes([
'**/djb/Curve25519.java',
'**/sshj/common/Base64.java',
'**/com/hierynomus/sshj/userauth/keyprovider/bcrypt/*.java',
'**/files/test_file_*.txt',
])
}
if (!JavaVersion.current().isJava9Compatible()) {

View File

@@ -261,7 +261,16 @@ public class RemoteFile
private boolean eof;
public ReadAheadRemoteFileInputStream(int maxUnconfirmedReads) {
this(maxUnconfirmedReads, 0L, -1L);
this(maxUnconfirmedReads, 0L);
}
/**
*
* @param maxUnconfirmedReads Maximum number of unconfirmed requests to send
* @param fileOffset Initial offset in file to read from
*/
public ReadAheadRemoteFileInputStream(int maxUnconfirmedReads, long fileOffset) {
this(maxUnconfirmedReads, fileOffset, -1L);
}
/**

View File

@@ -232,21 +232,41 @@ public class SFTPClient
throws IOException {
xfer.download(source, dest);
}
public void get(String source, String dest, long byteOffset)
throws IOException {
xfer.download(source, dest, byteOffset);
}
public void put(String source, String dest)
throws IOException {
xfer.upload(source, dest);
}
public void put(String source, String dest, long byteOffset)
throws IOException {
xfer.upload(source, dest, byteOffset);
}
public void get(String source, LocalDestFile dest)
throws IOException {
xfer.download(source, dest);
}
public void get(String source, LocalDestFile dest, long byteOffset)
throws IOException {
xfer.download(source, dest, byteOffset);
}
public void put(LocalSourceFile source, String dest)
throws IOException {
xfer.upload(source, dest);
}
public void put(LocalSourceFile source, String dest, long byteOffset)
throws IOException {
xfer.upload(source, dest, byteOffset);
}
@Override
public void close()

View File

@@ -50,25 +50,47 @@ public class SFTPFileTransfer
@Override
public void upload(String source, String dest)
throws IOException {
upload(new FileSystemFile(source), dest);
upload(source, dest, 0);
}
@Override
public void upload(String source, String dest, long byteOffset)
throws IOException {
upload(new FileSystemFile(source), dest, byteOffset);
}
@Override
public void download(String source, String dest)
throws IOException {
download(source, new FileSystemFile(dest));
download(source, dest, 0);
}
@Override
public void download(String source, String dest, long byteOffset)
throws IOException {
download(source, new FileSystemFile(dest), byteOffset);
}
@Override
public void upload(LocalSourceFile localFile, String remotePath) throws IOException {
new Uploader(localFile, remotePath).upload(getTransferListener());
upload(localFile, remotePath, 0);
}
@Override
public void upload(LocalSourceFile localFile, String remotePath, long byteOffset) throws IOException {
new Uploader(localFile, remotePath).upload(getTransferListener(), byteOffset);
}
@Override
public void download(String source, LocalDestFile dest) throws IOException {
download(source, dest, 0);
}
@Override
public void download(String source, LocalDestFile dest, long byteOffset) throws IOException {
final PathComponents pathComponents = engine.getPathHelper().getComponents(source);
final FileAttributes attributes = engine.stat(source);
new Downloader().download(getTransferListener(), new RemoteResourceInfo(pathComponents, attributes), dest);
new Downloader().download(getTransferListener(), new RemoteResourceInfo(pathComponents, attributes), dest, byteOffset);
}
public void setUploadFilter(LocalFileFilter uploadFilter) {
@@ -92,7 +114,8 @@ public class SFTPFileTransfer
@SuppressWarnings("PMD.MissingBreakInSwitch")
private void download(final TransferListener listener,
final RemoteResourceInfo remote,
final LocalDestFile local) throws IOException {
final LocalDestFile local,
final long byteOffset) throws IOException {
final LocalDestFile adjustedFile;
switch (remote.getAttributes().getType()) {
case DIRECTORY:
@@ -101,8 +124,9 @@ public class SFTPFileTransfer
case UNKNOWN:
log.warn("Server did not supply information about the type of file at `{}` " +
"-- assuming it is a regular file!", remote.getPath());
// fall through
case REGULAR:
adjustedFile = downloadFile(listener.file(remote.getName(), remote.getAttributes().getSize()), remote, local);
adjustedFile = downloadFile(listener.file(remote.getName(), remote.getAttributes().getSize()), remote, local, byteOffset);
break;
default:
throw new IOException(remote + " is not a regular file or directory");
@@ -119,7 +143,7 @@ public class SFTPFileTransfer
final RemoteDirectory rd = engine.openDir(remote.getPath());
try {
for (RemoteResourceInfo rri : rd.scan(getDownloadFilter()))
download(listener, rri, adjusted.getChild(rri.getName()));
download(listener, rri, adjusted.getChild(rri.getName()), 0); // not supporting individual byte offsets for these files
} finally {
rd.close();
}
@@ -128,13 +152,15 @@ public class SFTPFileTransfer
private LocalDestFile downloadFile(final StreamCopier.Listener listener,
final RemoteResourceInfo remote,
final LocalDestFile local)
final LocalDestFile local,
final long byteOffset)
throws IOException {
final LocalDestFile adjusted = local.getTargetFile(remote.getName());
final RemoteFile rf = engine.open(remote.getPath());
try {
final RemoteFile.ReadAheadRemoteFileInputStream rfis = rf.new ReadAheadRemoteFileInputStream(16);
final OutputStream os = adjusted.getOutputStream();
log.debug("Attempting to download {} with offset={}", remote.getPath(), byteOffset);
final RemoteFile.ReadAheadRemoteFileInputStream rfis = rf.new ReadAheadRemoteFileInputStream(16, byteOffset);
final OutputStream os = adjusted.getOutputStream(byteOffset != 0);
try {
new StreamCopier(rfis, os, engine.getLoggerFactory())
.bufSize(engine.getSubsystem().getLocalMaxPacketSize())
@@ -173,17 +199,17 @@ public class SFTPFileTransfer
this.remote = remote;
}
private void upload(final TransferListener listener) throws IOException {
private void upload(final TransferListener listener, long byteOffset) throws IOException {
if (source.isDirectory()) {
makeDirIfNotExists(remote); // Ensure that the directory exists
uploadDir(listener.directory(source.getName()), source, remote);
setAttributes(source, remote);
} else if (source.isFile() && isDirectory(remote)) {
String adjustedRemote = engine.getPathHelper().adjustForParent(this.remote, source.getName());
uploadFile(listener.file(source.getName(), source.getLength()), source, adjustedRemote);
uploadFile(listener.file(source.getName(), source.getLength()), source, adjustedRemote, byteOffset);
setAttributes(source, adjustedRemote);
} else if (source.isFile()) {
uploadFile(listener.file(source.getName(), source.getLength()), source, remote);
uploadFile(listener.file(source.getName(), source.getLength()), source, remote, byteOffset);
setAttributes(source, remote);
} else {
throw new IOException(source + " is not a file or directory");
@@ -192,13 +218,14 @@ public class SFTPFileTransfer
private void upload(final TransferListener listener,
final LocalSourceFile local,
final String remote)
final String remote,
final long byteOffset)
throws IOException {
final String adjustedPath;
if (local.isDirectory()) {
adjustedPath = uploadDir(listener.directory(local.getName()), local, remote);
} else if (local.isFile()) {
adjustedPath = uploadFile(listener.file(local.getName(), local.getLength()), local, remote);
adjustedPath = uploadFile(listener.file(local.getName(), local.getLength()), local, remote, byteOffset);
} else {
throw new IOException(local + " is not a file or directory");
}
@@ -217,22 +244,34 @@ public class SFTPFileTransfer
throws IOException {
makeDirIfNotExists(remote);
for (LocalSourceFile f : local.getChildren(getUploadFilter()))
upload(listener, f, engine.getPathHelper().adjustForParent(remote, f.getName()));
upload(listener, f, engine.getPathHelper().adjustForParent(remote, f.getName()), 0); // not supporting individual byte offsets for these files
return remote;
}
private String uploadFile(final StreamCopier.Listener listener,
final LocalSourceFile local,
final String remote)
final String remote,
final long byteOffset)
throws IOException {
final String adjusted = prepareFile(local, remote);
final String adjusted = prepareFile(local, remote, byteOffset);
RemoteFile rf = null;
InputStream fis = null;
RemoteFile.RemoteFileOutputStream rfos = null;
EnumSet<OpenMode> modes;
try {
rf = engine.open(adjusted, EnumSet.of(OpenMode.WRITE, OpenMode.CREAT, OpenMode.TRUNC));
if (byteOffset == 0) {
// Starting at the beginning, overwrite/create
modes = EnumSet.of(OpenMode.WRITE, OpenMode.CREAT, OpenMode.TRUNC);
} else {
// Starting at some offset, append
modes = EnumSet.of(OpenMode.WRITE, OpenMode.APPEND);
}
log.debug("Attempting to upload {} with offset={}", local.getName(), byteOffset);
rf = engine.open(adjusted, modes);
fis = local.getInputStream();
rfos = rf.new RemoteFileOutputStream(0, 16);
fis.skip(byteOffset);
rfos = rf.new RemoteFileOutputStream(byteOffset, 16);
new StreamCopier(fis, rfos, engine.getLoggerFactory())
.bufSize(engine.getSubsystem().getRemoteMaxPacketSize() - rf.getOutgoingPacketOverhead())
.keepFlushing(false)
@@ -294,7 +333,7 @@ public class SFTPFileTransfer
}
}
private String prepareFile(final LocalSourceFile local, final String remote)
private String prepareFile(final LocalSourceFile local, final String remote, final long byteOffset)
throws IOException {
final FileAttributes attrs;
try {
@@ -309,7 +348,7 @@ public class SFTPFileTransfer
if (attrs.getMode().getType() == FileMode.Type.DIRECTORY) {
throw new IOException("Trying to upload file " + local.getName() + " to path " + remote + " but that is a directory");
} else {
log.debug("probeFile: {} is a {} file that will be replaced", remote, attrs.getMode().getType());
log.debug("probeFile: {} is a {} file that will be {}", remote, attrs.getMode().getType(), byteOffset > 0 ? "resumed" : "replaced");
return remote;
}
}

View File

@@ -71,7 +71,13 @@ public class FileSystemFile
@Override
public OutputStream getOutputStream()
throws IOException {
return new FileOutputStream(file);
return getOutputStream(false);
}
@Override
public OutputStream getOutputStream(boolean append)
throws IOException {
return new FileOutputStream(file, append);
}
@Override

View File

@@ -31,6 +31,19 @@ public interface FileTransfer {
void upload(String localPath, String remotePath)
throws IOException;
/**
* This is meant to delegate to {@link #upload(LocalSourceFile, String)} with the {@code localPath} wrapped as e.g.
* a {@link FileSystemFile}. Appends to existing if {@code byteOffset} &gt; 0.
*
* @param localPath
* @param remotePath
* @param byteOffset
*
* @throws IOException
*/
void upload(String localPath, String remotePath, long byteOffset)
throws IOException;
/**
* This is meant to delegate to {@link #download(String, LocalDestFile)} with the {@code localPath} wrapped as e.g.
* a {@link FileSystemFile}.
@@ -43,6 +56,19 @@ public interface FileTransfer {
void download(String remotePath, String localPath)
throws IOException;
/**
* This is meant to delegate to {@link #download(String, LocalDestFile)} with the {@code localPath} wrapped as e.g.
* a {@link FileSystemFile}. Appends to existing if {@code byteOffset} &gt; 0.
*
* @param localPath
* @param remotePath
* @param byteOffset
*
* @throws IOException
*/
void download(String remotePath, String localPath, long byteOffset)
throws IOException;
/**
* Upload {@code localFile} to {@code remotePath}.
*
@@ -54,6 +80,18 @@ public interface FileTransfer {
void upload(LocalSourceFile localFile, String remotePath)
throws IOException;
/**
* Upload {@code localFile} to {@code remotePath}. Appends to existing if {@code byteOffset} &gt; 0.
*
* @param localFile
* @param remotePath
* @param byteOffset
*
* @throws IOException
*/
void upload(LocalSourceFile localFile, String remotePath, long byteOffset)
throws IOException;
/**
* Download {@code remotePath} to {@code localFile}.
*
@@ -65,6 +103,18 @@ public interface FileTransfer {
void download(String remotePath, LocalDestFile localFile)
throws IOException;
/**
* Download {@code remotePath} to {@code localFile}. Appends to existing if {@code byteOffset} &gt; 0.
*
* @param localFile
* @param remotePath
* @param byteOffset
*
* @throws IOException
*/
void download(String remotePath, LocalDestFile localFile, long byteOffset)
throws IOException;
TransferListener getTransferListener();
void setTransferListener(TransferListener listener);

View File

@@ -20,8 +20,13 @@ import java.io.OutputStream;
public interface LocalDestFile {
long getLength();
OutputStream getOutputStream()
throws IOException;
OutputStream getOutputStream(boolean append)
throws IOException;
/** @return A child file/directory of this directory with given {@code name}. */
LocalDestFile getChild(String name);

View File

@@ -52,24 +52,49 @@ public class SCPFileTransfer
@Override
public void upload(String localPath, String remotePath)
throws IOException {
newSCPUploadClient().copy(new FileSystemFile(localPath), remotePath);
upload(localPath, remotePath, 0);
}
@Override
public void upload(String localFile, String remotePath, long byteOffset)
throws IOException {
upload(new FileSystemFile(localFile), remotePath, byteOffset);
}
@Override
public void download(String remotePath, String localPath)
throws IOException {
download(remotePath, new FileSystemFile(localPath));
download(remotePath, localPath, 0);
}
@Override
public void download(String remotePath, String localPath, long byteOffset) throws IOException {
download(remotePath, new FileSystemFile(localPath), byteOffset);
}
@Override
public void download(String remotePath, LocalDestFile localFile)
throws IOException {
download(remotePath, localFile, 0);
}
@Override
public void download(String remotePath, LocalDestFile localFile, long byteOffset)
throws IOException {
checkByteOffsetSupport(byteOffset);
newSCPDownloadClient().copy(remotePath, localFile);
}
@Override
public void upload(LocalSourceFile localFile, String remotePath)
throws IOException {
upload(localFile, remotePath, 0);
}
@Override
public void upload(LocalSourceFile localFile, String remotePath, long byteOffset)
throws IOException {
checkByteOffsetSupport(byteOffset);
newSCPUploadClient().copy(localFile, remotePath);
}
@@ -79,4 +104,11 @@ public class SCPFileTransfer
}
return this;
}
private void checkByteOffsetSupport(long byteOffset) throws IOException {
// TODO - implement byte offsets on SCP, if possible.
if (byteOffset > 0) {
throw new SCPException("Byte offset on SCP file transfers is not supported.");
}
}
}

View File

@@ -39,4 +39,8 @@ public class FileUtil {
IOUtils.closeQuietly(fileInputStream);
}
}
public static boolean compareFileContents(File f1, File f2) throws IOException {
return readFromFile(f1).equals(readFromFile(f2));
}
}

View File

@@ -0,0 +1,185 @@
/*
* 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.SshFixture;
import com.hierynomus.sshj.test.util.FileUtil;
import java.io.File;
import java.io.IOException;
import net.schmizz.sshj.SSHClient;
import net.schmizz.sshj.common.StreamCopier;
import net.schmizz.sshj.xfer.TransferListener;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
public class SFTPFileTransferTest {
public static final String TARGET_FILE_NAME = "target.txt";
File targetDir;
File targetFile;
File sourceFile;
File partialFile;
SSHClient sshClient;
SFTPFileTransfer xfer;
ByteCounter listener;
@Rule
public SshFixture fixture = new SshFixture();
@Rule
public TemporaryFolder tempFolder = new TemporaryFolder();
@Before
public void init() throws IOException {
targetDir = tempFolder.newFolder();
targetFile = new File(targetDir, TARGET_FILE_NAME);
sourceFile = new File("src/test/resources/files/test_file_full.txt");
partialFile = new File("src/test/resources/files/test_file_partial.txt");
sshClient = fixture.setupConnectedDefaultClient();
sshClient.authPassword("test", "test");
xfer = sshClient.newSFTPClient().getFileTransfer();
xfer.setTransferListener(listener = new ByteCounter());
}
@After
public void cleanup() {
if (targetFile.exists()) {
targetFile.delete();
}
if (targetDir.exists()) {
targetDir.delete();
}
}
private void performDownload(long byteOffset) throws IOException {
assertTrue(listener.getBytesTransferred() == 0);
long expectedBytes = 0;
// Using the resume param this way to call the different entry points into the FileTransfer interface
if (byteOffset > 0) {
expectedBytes = sourceFile.length() - targetFile.length(); // only the difference between what is there and what should be
xfer.download(sourceFile.getAbsolutePath(), targetFile.getAbsolutePath(), byteOffset);
} else {
expectedBytes = sourceFile.length(); // the entire source file should be transferred
xfer.download(sourceFile.getAbsolutePath(), targetFile.getAbsolutePath());
}
assertTrue(FileUtil.compareFileContents(sourceFile, targetFile));
assertTrue(listener.getBytesTransferred() == expectedBytes);
}
private void performUpload(long byteOffset) throws IOException {
assertTrue(listener.getBytesTransferred() == 0);
long expectedBytes = 0;
// Using the resume param this way to call the different entry points into the FileTransfer interface
if (byteOffset > 0) {
expectedBytes = sourceFile.length() - targetFile.length(); // only the difference between what is there and what should be
xfer.upload(sourceFile.getAbsolutePath(), targetFile.getAbsolutePath(), byteOffset);
} else {
expectedBytes = sourceFile.length(); // the entire source file should be transferred
xfer.upload(sourceFile.getAbsolutePath(), targetFile.getAbsolutePath());
}
assertTrue(FileUtil.compareFileContents(sourceFile, targetFile));
assertTrue(listener.getBytesTransferred() == expectedBytes);
}
@Test
public void testDownload() throws IOException {
performDownload(0);
}
@Test
public void testDownloadResumePartial() throws IOException {
FileUtil.writeToFile(targetFile, FileUtil.readFromFile(partialFile));
assertFalse(FileUtil.compareFileContents(sourceFile, targetFile));
performDownload(targetFile.length());
}
@Test
public void testDownloadResumeNothing() throws IOException {
assertFalse(targetFile.exists());
performDownload(targetFile.length());
}
@Test
public void testDownloadResumePreviouslyCompleted() throws IOException {
FileUtil.writeToFile(targetFile, FileUtil.readFromFile(sourceFile));
assertTrue(FileUtil.compareFileContents(sourceFile, targetFile));
performDownload(targetFile.length());
}
@Test
public void testUpload() throws IOException {
performUpload(0);
}
@Test
public void testUploadResumePartial() throws IOException {
FileUtil.writeToFile(targetFile, FileUtil.readFromFile(partialFile));
assertFalse(FileUtil.compareFileContents(sourceFile, targetFile));
performUpload(targetFile.length());
}
@Test
public void testUploadResumeNothing() throws IOException {
assertFalse(targetFile.exists());
performUpload(targetFile.length());
}
@Test
public void testUploadResumePreviouslyCompleted() throws IOException {
FileUtil.writeToFile(targetFile, FileUtil.readFromFile(sourceFile));
assertTrue(FileUtil.compareFileContents(sourceFile, targetFile));
performUpload(targetFile.length());
}
public class ByteCounter implements TransferListener, StreamCopier.Listener {
long bytesTransferred;
public long getBytesTransferred() {
return bytesTransferred;
}
@Override
public TransferListener directory(String name) {
return this;
}
@Override
public StreamCopier.Listener file(String name, long size) {
return this;
}
@Override
public void reportProgress(long transferred) throws IOException {
bytesTransferred = transferred;
}
}
}

View File

@@ -0,0 +1,10 @@
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*(),.;'[]/?
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*(),.;'[]/?
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*(),.;'[]/?
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*(),.;'[]/?
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*(),.;'[]/?
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*(),.;'[]/?
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*(),.;'[]/?
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*(),.;'[]/?
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*(),.;'[]/?
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*(),.;'[]/?

View File

@@ -0,0 +1,6 @@
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*(),.;'[]/?
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*(),.;'[]/?
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*(),.;'[]/?
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*(),.;'[]/?
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*(),.;'[]/?
abcdefghijklmnopqrstuvwxyzABCDEF