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.

Author: tomaswolf

sshd-sftp/src/main/java/org/apache/sshd/sftp/server/FileHandle.java

@@ -134,10 +134,21 @@ public int read(byte[] data, int doff, int length, long offset) throws IOExcepti
     @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;
     }

sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java

@@ -900,7 +900,7 @@ protected int doRead(
                     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;

sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SimpleSftpClientTest.java

@@ -25,6 +25,7 @@
 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 @@ void sftpProxyCalls() throws Exception {
             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)) {