From 9ce0a851dc78a971375ceaa78ffa46ef01a09eb9 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Thu, 13 Aug 2026 12:45:57 +0200 Subject: [PATCH] fix(peer): handle UploadDenied so refused downloads fail fast Peer message code 50 had no handler, so the dispatcher dropped it and a download refused by the peer stayed Queued until the caller's timeout. The new handler reads the filename and reason and routes the denial into the same failure path as UploadFailed. A reason of "Queued" is status, not a failure: some clients send it for a download that is still held in their queue, and failing it would kill healthy downloads after a reconnect, so that one is only logged. Fixes #29 --- soulseek-rs-lib/src/actor/peer_actor.rs | 3 +- soulseek-rs-lib/src/message/peer/mod.rs | 2 + .../src/message/peer/upload_denied.rs | 56 +++++++++++++++++++ soulseek-rs-lib/tests/e2e.rs | 12 ++++ 4 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 soulseek-rs-lib/src/message/peer/upload_denied.rs diff --git a/soulseek-rs-lib/src/actor/peer_actor.rs b/soulseek-rs-lib/src/actor/peer_actor.rs index d46dd38..821af9f 100644 --- a/soulseek-rs-lib/src/actor/peer_actor.rs +++ b/soulseek-rs-lib/src/actor/peer_actor.rs @@ -5,7 +5,7 @@ use crate::message::peer::{ FileSearchResponse, GetShareFileList, PeerInit, PlaceInQueueRequest, PlaceInQueueResponse, QueueUploadHandler, SharedDirectory, SharedFileListResponseHandler, TransferRequest, TransferResponse, - UploadFailedHandler, + UploadDeniedHandler, UploadFailedHandler, }; use crate::message::server::MessageFactory; use crate::message::{Handlers, Message, MessageReader, MessageType}; @@ -178,6 +178,7 @@ impl PeerActor { handlers.register_handler(TransferRequest); handlers.register_handler(TransferResponse); handlers.register_handler(GetShareFileList); + handlers.register_handler(UploadDeniedHandler); handlers.register_handler(UploadFailedHandler); handlers.register_handler(PlaceInQueueRequest); handlers.register_handler(PlaceInQueueResponse); diff --git a/soulseek-rs-lib/src/message/peer/mod.rs b/soulseek-rs-lib/src/message/peer/mod.rs index 60f9bfe..0f245d9 100644 --- a/soulseek-rs-lib/src/message/peer/mod.rs +++ b/soulseek-rs-lib/src/message/peer/mod.rs @@ -7,6 +7,7 @@ mod queue_upload; mod shared_file_list; mod transfer_request; mod transfer_response; +mod upload_denied; mod upload_failed; // Re-export handlers @@ -24,4 +25,5 @@ pub use shared_file_list::{ }; pub use transfer_request::TransferRequest; pub use transfer_response::TransferResponse; +pub use upload_denied::UploadDeniedHandler; pub use upload_failed::UploadFailedHandler; diff --git a/soulseek-rs-lib/src/message/peer/upload_denied.rs b/soulseek-rs-lib/src/message/peer/upload_denied.rs new file mode 100644 index 0000000..3ee5d94 --- /dev/null +++ b/soulseek-rs-lib/src/message/peer/upload_denied.rs @@ -0,0 +1,56 @@ +use crate::info; +use crate::{ + message::{Message, MessageHandler}, + peer::PeerMessage, +}; +use std::sync::mpsc::Sender; + +pub struct UploadDeniedHandler; +impl MessageHandler for UploadDeniedHandler { + fn get_code(&self) -> u8 { + 50 + } + fn handle(&self, message: &mut Message, sender: Sender) { + let filename = message.read_string(); + let reason = message.read_string(); + info!("Upload denied for {}: {}", filename, reason); + if reason == "Queued" { + return; + } + let _ = sender.send(PeerMessage::UploadFailed(String::new(), filename)); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::message::framed; + + #[test] + fn a_queued_reason_is_not_a_failure() { + let (tx, rx) = std::sync::mpsc::channel(); + let mut message = framed(|m| { + m.write_string("@@share\\gone.mp3").write_string("Queued"); + }); + + UploadDeniedHandler.handle(&mut message, tx); + assert!(rx.try_recv().is_err()); + } + + #[test] + fn the_denied_filename_is_forwarded() { + let (tx, rx) = std::sync::mpsc::channel(); + let mut message = framed(|m| { + m.write_string("@@share\\gone.mp3") + .write_string("File not shared."); + }); + + UploadDeniedHandler.handle(&mut message, tx); + match rx.try_recv() { + Ok(PeerMessage::UploadFailed(_, filename)) => { + assert_eq!(filename, "@@share\\gone.mp3"); + } + other => panic!("unexpected: {other:?}"), + } + } +} diff --git a/soulseek-rs-lib/tests/e2e.rs b/soulseek-rs-lib/tests/e2e.rs index 64c7339..ab48b13 100644 --- a/soulseek-rs-lib/tests/e2e.rs +++ b/soulseek-rs-lib/tests/e2e.rs @@ -859,6 +859,18 @@ fn an_upload_failed_reply_fails_the_download_quickly() { }); } +#[test] +fn an_upload_denied_reply_fails_the_download_quickly() { + refusal_fails_the_download_quickly("e2e_denying_peer", |filename| { + let mut refusal = Message::new(); + refusal + .write_int32(50) + .write_string(filename) + .write_string("File not shared."); + refusal + }); +} + #[test] fn an_interrupted_download_resumes_from_its_partial_file() { let server = server_or_skip!();