This is an automated email from the ASF dual-hosted git repository.
twolf pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
The following commit(s) were added to refs/heads/master by this push:
new 3a272e9de SSHD-1348: SFTP - gracefully handle zero-length SSH_FXP_READs
3a272e9de is described below
commit 3a272e9de06db39e654444549ab5f3097c224356
Author: Thomas Wolf <[email protected]>
AuthorDate: Sat Dec 20 14:42:52 2025 +0100
SSHD-1348: SFTP - gracefully handle zero-length SSH_FXP_READs
Fix inconsistency between checks for > 0 and >= 0, and ensure a zero-
length read at or after EOF returns EOF, i.e., a SSH_FXP_STATUS reply
with SSH_FX_EOF. Zero-length reads before EOF return a zero-length
SSH_FXP_DATA reply.
---
.../org/apache/sshd/sftp/server/FileHandle.java | 15 +++++++++++++--
.../org/apache/sshd/sftp/server/SftpSubsystem.java | 2 +-
.../sshd/sftp/client/SimpleSftpClientTest.java | 22 ++++++++++++++++++++++
3 files changed, 36 insertions(+), 3 deletions(-)
diff --git
a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/FileHandle.java
b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/FileHandle.java
index 258d22497..e87b46102 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/FileHandle.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/FileHandle.java
@@ -134,10 +134,21 @@ public class FileHandle extends Handle {
@SuppressWarnings("resource")
public int read(byte[] data, int doff, int length, long offset,
AtomicReference<Boolean> eof) throws IOException {
SeekableByteChannel channel = getFileChannel();
+ if (length == 0 && offset >= channel.size()) {
+ // Zero-length read at EOF. SFTP v3: draft RFC v2 says: "If [...]
EOF is encountered before reading any
+ // data, the server will respond with SSH_FXP_STATUS." (section
6.4). Later versions of the draft are
+ // silent on this issue.
+ //
+ // See
https://datatracker.ietf.org/doc/html/draft-ietf-secsh-filexfer-02#section-6.4
+ if (eof != null) {
+ eof.set(true);
+ }
+ return -1;
+ }
channel = channel.position(offset);
int l = channel.read(ByteBuffer.wrap(data, doff, length));
- if (l > 0 && eof != null && l < length) {
- eof.set(channel.position() >= channel.size());
+ if (eof != null) {
+ eof.set(l < 0 || (l < length && channel.position() >=
channel.size()));
}
return l;
}
diff --git
a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java
b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java
index f619c7783..afe040c10 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java
@@ -900,7 +900,7 @@ public class SftpSubsystem
session, id, Handle.safe(handle), h, offset, length);
}
- ValidateUtils.checkTrue(length > 0L, "Invalid read length: %d",
length);
+ ValidateUtils.checkTrue(length >= 0, "Invalid read length: %d",
length);
FileHandle fh = validateHandle(handle, h, FileHandle.class);
SftpEventListener listener = getSftpEventListenerProxy();
int readLen;
diff --git
a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SimpleSftpClientTest.java
b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SimpleSftpClientTest.java
index f7ce68087..4d2d421f2 100644
---
a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SimpleSftpClientTest.java
+++
b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SimpleSftpClientTest.java
@@ -25,6 +25,7 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Collections;
import java.util.EnumSet;
+import java.util.concurrent.atomic.AtomicReference;
import org.apache.sshd.common.file.FileSystemFactory;
import org.apache.sshd.common.file.virtualfs.VirtualFileSystemFactory;
@@ -115,6 +116,27 @@ public class SimpleSftpClientTest extends
BaseSimpleClientTestSupport {
byte[] local = Files.readAllBytes(clientFile);
assertArrayEquals(written, local, "Mismatched remote written
data");
+ // Test zero reads
+ try (SftpClient.CloseableHandle h = sftp.open(remoteFilePath,
SftpClient.OpenMode.Read)) {
+ AtomicReference<Boolean> eof = new AtomicReference<>();
+ byte[] buf = {};
+ int bytesRead = sftp.read(h, 0, buf, eof);
+ assertEquals(0, bytesRead);
+ assertFalse(eof.get() != null && eof.get().booleanValue());
+ bytesRead = sftp.read(h, written.length, buf, eof);
+ assertEquals(-1, bytesRead);
+ assertTrue(eof.get() != null && eof.get().booleanValue());
+ bytesRead = sftp.read(h, 1, buf, eof);
+ assertEquals(0, bytesRead);
+ assertFalse(eof.get() != null && eof.get().booleanValue());
+ bytesRead = sftp.read(h, written.length + 10, buf, eof);
+ assertEquals(-1, bytesRead);
+ assertTrue(eof.get() != null && eof.get().booleanValue());
+ bytesRead = sftp.read(h, written.length - 1, buf, eof);
+ assertEquals(0, bytesRead);
+ assertFalse(eof.get() != null && eof.get().booleanValue());
+ }
+
try (SftpClient.CloseableHandle h = sftp.openDir(remoteFileDir)) {
boolean matchFound = false;
for (SftpClient.DirEntry entry : sftp.listDir(h)) {
