From 96988f9f9e805f09419e400ddfc066ff6488eb82 Mon Sep 17 00:00:00 2001
From: michel <michel@re-invention.nl>
Date: Sat, 18 Jul 2026 14:22:32 +0200
Subject: [PATCH] fix: detect truncated downloads instead of saving them as
 Completed

read_download_stream broke out of its read loop on peer EOF (Ok(0)) without
checking whether the promised number of bytes had actually arrived, then saved
the partial buffer and the caller reported DownloadStatus::Completed. A peer
that aborts mid-transfer thus produced a truncated file marked complete. The
buffer was also never trimmed, so trailing coalesced bytes were written past
the file's real size.

finalize_download_buffer now errors with IncompleteDownload when fewer bytes
than expected arrived (caller maps this to Failed) and trims any overshoot to
the exact expected size. The listener's incoming-file path now also marks the
download Failed on error rather than only logging, so it can no longer stay
stuck Queued/InProgress forever.
---
 soulseek-rs-lib/src/peer/download_peer.rs | 62 ++++++++++++++++++++++-
 soulseek-rs-lib/src/peer/listen.rs        | 14 +++++
 2 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/soulseek-rs-lib/src/peer/download_peer.rs b/soulseek-rs-lib/src/peer/download_peer.rs
index 2b2623d..bc6b161 100644
--- a/soulseek-rs-lib/src/peer/download_peer.rs
+++ b/soulseek-rs-lib/src/peer/download_peer.rs
@@ -31,6 +31,7 @@ pub enum DownloadError {
     PathResolutionError(String),
     InvalidTokenBytes,
     LockPoisoned,
+    IncompleteDownload { received: usize, expected: usize },
 }
 
 impl std::fmt::Display for DownloadError {
@@ -57,6 +58,10 @@ impl std::fmt::Display for DownloadError {
                 write!(f, "Invalid token bytes received")
             }
             Self::LockPoisoned => write!(f, "Lock poisoned"),
+            Self::IncompleteDownload { received, expected } => write!(
+                f,
+                "Incomplete download: received {received} of {expected} bytes"
+            ),
         }
     }
 }
@@ -361,7 +366,12 @@ impl DownloadPeer {
         let download =
             download.ok_or(DownloadError::DownloadInfoMissing(self.token))?;
 
-        Ok((processor.buffer, download))
+        let buffer = Self::finalize_download_buffer(
+            processor.buffer,
+            download.size as usize,
+        )?;
+
+        Ok((buffer, download))
     }
 
     fn send_download_status(
@@ -432,6 +442,25 @@ impl DownloadPeer {
             .map(String::from)
     }
 
+    /// Validate and normalize a fully-read download buffer against the size the
+    /// peer promised in the transfer request. A peer that closes the connection
+    /// early yields fewer bytes than expected — that must be reported as a
+    /// failure, not silently saved as a truncated "completed" file. A peer that
+    /// sends trailing bytes past the expected size has them trimmed off.
+    fn finalize_download_buffer(
+        mut buffer: Vec<u8>,
+        expected_size: usize,
+    ) -> Result<Vec<u8>, DownloadError> {
+        if buffer.len() < expected_size {
+            return Err(DownloadError::IncompleteDownload {
+                received: buffer.len(),
+                expected: expected_size,
+            });
+        }
+        buffer.truncate(expected_size);
+        Ok(buffer)
+    }
+
     fn save_downloaded_file(
         &self,
         path: &str,
@@ -498,7 +527,36 @@ impl DownloadPeer {
 
 #[cfg(test)]
 mod tests {
-    use super::{DownloadPeer, FileManager};
+    use super::{DownloadError, DownloadPeer, FileManager};
+
+    #[test]
+    fn finalize_rejects_truncated_download() {
+        // Peer closed early: 5 of 10 promised bytes. Must be a failure so the
+        // partial file is never reported as Completed.
+        let result = DownloadPeer::finalize_download_buffer(vec![1, 2, 3, 4, 5], 10);
+        assert!(matches!(
+            result,
+            Err(DownloadError::IncompleteDownload {
+                received: 5,
+                expected: 10
+            })
+        ));
+    }
+
+    #[test]
+    fn finalize_trims_overshoot_to_expected_size() {
+        // Peer sent 12 bytes for a 10-byte file (trailing bytes coalesced in).
+        let result =
+            DownloadPeer::finalize_download_buffer((0..12).collect(), 10);
+        assert_eq!(result.unwrap(), (0..10).collect::<Vec<u8>>());
+    }
+
+    #[test]
+    fn finalize_accepts_exact_size() {
+        let result =
+            DownloadPeer::finalize_download_buffer((0..10).collect(), 10);
+        assert_eq!(result.unwrap(), (0..10).collect::<Vec<u8>>());
+    }
 
     #[test]
     fn test_establish_connection_invalid_address() {
diff --git a/soulseek-rs-lib/src/peer/listen.rs b/soulseek-rs-lib/src/peer/listen.rs
index ab26426..af63fd2 100644
--- a/soulseek-rs-lib/src/peer/listen.rs
+++ b/soulseek-rs-lib/src/peer/listen.rs
@@ -158,6 +158,7 @@ fn handle_file_connection(
         peer_ip,
         peer_port,
     );
+    let failure_token = download.as_ref().map(|d| d.token);
 
     let download_peer = DownloadPeer::new(
         format!("{}:direct", peer.username),
@@ -194,6 +195,19 @@ fn handle_file_connection(
                 "Failed to download file from {}:{} (token: {}) - Error: {}",
                 peer.host, peer.port, token, e
             );
+            // A failed incoming transfer (e.g. a truncated/incomplete download)
+            // must not leave the download stuck as Queued/InProgress forever.
+            if let Some(failure_token) = failure_token {
+                match context.client_context.write_safe() {
+                    Ok(mut ctx) => ctx.update_download_with_status(
+                        failure_token,
+                        DownloadStatus::Failed,
+                    ),
+                    Err(e) => {
+                        error!("[listener] handle_file_connection fail write: {}", e)
+                    }
+                }
+            }
         }
     }
 }