[SSHD-1288] SFTP: fix reading files that are being written
If data is appended while a file is read via SFTP, SftpInputStreamAsync
would enter an infinite loop if requestOffset >= fileSize + bufferSize.
Fix this and issue only sequential read requests once we detect that
we're beyond the expected EOF.
diff --git a/CHANGES.md b/CHANGES.md
index 842a961..a4d57fd 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -22,4 +22,5 @@
* [SSHD-1281](https://issues.apache.org/jira/browse/SSHD-1281) ClientSession.auth().verify() is terminated with timeout
* [SSHD-1285](https://issues.apache.org/jira/browse/SSHD-1285) 2.9.0 release broken on Java 8
+* [SSHD-1288](https://issues.apache.org/jira/browse/SSHD-1288) SFTP: fix reading files that are being written
* [SSHD-1289](https://issues.apache.org/jira/browse/SSHD-1289) Deadlock during session exit
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/SftpInputStreamAsync.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/SftpInputStreamAsync.java
index 12ea4e5..d846c66 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/SftpInputStreamAsync.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/SftpInputStreamAsync.java
@@ -228,8 +228,11 @@
Session session = client.getSession();
byte[] id = handle.getIdentifier();
boolean traceEnabled = log.isTraceEnabled();
- while (pendingReads.size() < Math.max(1, windowSize / bufferSize)
- && (fileSize <= 0 || requestOffset < fileSize + bufferSize)) {
+ if (fileSize > 0 && requestOffset > fileSize && !pendingReads.isEmpty()) {
+ // We'd be issuing requests for reading beyond the expected EOF. Do that only one by one.
+ return;
+ }
+ while (pendingReads.size() < Math.max(1, windowSize / bufferSize)) {
Buffer buf = session.createBuffer(SshConstants.SSH_MSG_CHANNEL_DATA,
23 /* sftp packet */ + 16 + id.length);
buf.rpos(23);
@@ -244,6 +247,9 @@
}
pendingReads.add(ack);
requestOffset += bufferSize;
+ if (fileSize > 0 && requestOffset > fileSize) {
+ break;
+ }
}
}
diff --git a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java
index f247ed5..4351040 100644
--- a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java
+++ b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java
@@ -244,6 +244,51 @@
}
}
+ @Test // see SSHD-1288
+ public void testReadWriteDownload() throws Exception {
+ Assume.assumeTrue("Not sure appending to a file opened for reading works on Windows",
+ OsUtils.isUNIX() || OsUtils.isOSX());
+ Path targetPath = detectTargetFolder();
+ Path parentPath = targetPath.getParent();
+ Path lclSftp = CommonTestSupportUtils.resolve(targetPath, SftpConstants.SFTP_SUBSYSTEM_NAME, getClass().getSimpleName(),
+ getCurrentTestName());
+ Path testFile = assertHierarchyTargetFolderExists(lclSftp).resolve("file.bin");
+ byte[] expected = new byte[1024 * 1024];
+
+ Factory<? extends Random> factory = sshd.getRandomFactory();
+ Random rnd = factory.create();
+ rnd.fill(expected);
+ Files.write(testFile, expected);
+
+ String file = CommonTestSupportUtils.resolveRelativeRemotePath(parentPath, testFile);
+ try (SftpClient sftp = createSingleSessionClient()) {
+ byte[] actual;
+ try (InputStream in = sftp.read(file); ByteArrayOutputStream buf = new ByteArrayOutputStream(2 * expected.length)) {
+ Files.write(testFile, expected, StandardOpenOption.WRITE, StandardOpenOption.APPEND);
+ byte[] data = new byte[4096];
+ for (int n = 0; n >= 0;) {
+ n = in.read(data, 0, data.length);
+ if (n > 0) {
+ buf.write(data, 0, n);
+ }
+ }
+ actual = buf.toByteArray();
+ }
+ assertEquals("Short read", 2 * expected.length, actual.length);
+ for (int i = 0, j = 0; i < actual.length; i++, j++) {
+ if (j >= expected.length) {
+ j = 0;
+ }
+ byte expByte = expected[j];
+ byte actByte = actual[i];
+ if (expByte != actByte) {
+ fail("Mismatched values at index=" + i + ": expected=0x" + Integer.toHexString(expByte & 0xFF)
+ + ", actual=0x" + Integer.toHexString(actByte & 0xFF));
+ }
+ }
+ }
+ }
+
@Test // see extra fix for SSHD-538
public void testNavigateBeyondRootFolder() throws Exception {
Path rootLocation = Paths.get(OsUtils.isUNIX() ? "/" : "C:\\");
