feat(peer): ask a peer what it says about itself
The client answered a UserInfoRequest but could never send one, so a user's profile — description, upload slots, queue length — was something this client served and never saw. The reply (peer code 16) is now parsed too, with the tolerances other clients carry: a picture is skipped rather than kept, a length that overruns the message is clamped, and a message that stops early (older clients, and Museek+ writing a boolean as an integer) still yields every field before it.
This commit is contained in:
@@ -1,6 +1,7 @@
|
|||||||
use crate::actor::{Actor, ActorHandle, ConnectionState};
|
use crate::actor::{Actor, ActorHandle, ConnectionState};
|
||||||
use crate::client::ClientOperation;
|
use crate::client::ClientOperation;
|
||||||
use crate::dispatcher::MessageDispatcher;
|
use crate::dispatcher::MessageDispatcher;
|
||||||
|
use crate::message::peer::UserInfoResponseHandler;
|
||||||
use crate::message::peer::{
|
use crate::message::peer::{
|
||||||
FileSearchResponse, FolderContentsRequest, FolderContentsResponseHandler,
|
FileSearchResponse, FolderContentsRequest, FolderContentsResponseHandler,
|
||||||
GetShareFileList, PeerInit, PlaceInQueueRequest, PlaceInQueueResponse,
|
GetShareFileList, PeerInit, PlaceInQueueRequest, PlaceInQueueResponse,
|
||||||
@@ -47,6 +48,8 @@ pub enum PeerMessage {
|
|||||||
ShareListRequested,
|
ShareListRequested,
|
||||||
/// A peer we are browsing sent us their shared-file listing (code 5).
|
/// A peer we are browsing sent us their shared-file listing (code 5).
|
||||||
ShareListReceived(Vec<SharedDirectory>),
|
ShareListReceived(Vec<SharedDirectory>),
|
||||||
|
/// A peer answered our `UserInfoRequest` with what it says about itself.
|
||||||
|
UserInfoReceived(crate::message::peer::PeerInfo),
|
||||||
/// A peer answered our `FolderContentsRequest` with one folder's listing.
|
/// A peer answered our `FolderContentsRequest` with one folder's listing.
|
||||||
FolderContentsReceived {
|
FolderContentsReceived {
|
||||||
token: u32,
|
token: u32,
|
||||||
@@ -207,6 +210,7 @@ impl PeerActor {
|
|||||||
handlers.register_handler(FolderContentsRequest);
|
handlers.register_handler(FolderContentsRequest);
|
||||||
handlers.register_handler(FolderContentsResponseHandler);
|
handlers.register_handler(FolderContentsResponseHandler);
|
||||||
handlers.register_handler(UserInfoRequest);
|
handlers.register_handler(UserInfoRequest);
|
||||||
|
handlers.register_handler(UserInfoResponseHandler);
|
||||||
handlers.register_handler(UploadDeniedHandler);
|
handlers.register_handler(UploadDeniedHandler);
|
||||||
handlers.register_handler(UploadFailedHandler);
|
handlers.register_handler(UploadFailedHandler);
|
||||||
handlers.register_handler(PlaceInQueueRequest);
|
handlers.register_handler(PlaceInQueueRequest);
|
||||||
@@ -317,6 +321,12 @@ impl PeerActor {
|
|||||||
directories,
|
directories,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
PeerMessage::UserInfoReceived(info) => {
|
||||||
|
self.forward(ClientOperation::PeerInfoReceived {
|
||||||
|
username: self.peer_username(),
|
||||||
|
info,
|
||||||
|
});
|
||||||
|
}
|
||||||
PeerMessage::FolderContentsReceived {
|
PeerMessage::FolderContentsReceived {
|
||||||
token,
|
token,
|
||||||
folder,
|
folder,
|
||||||
|
|||||||
@@ -340,6 +340,11 @@ pub enum ClientOperation {
|
|||||||
token: u32,
|
token: u32,
|
||||||
folder: String,
|
folder: String,
|
||||||
},
|
},
|
||||||
|
/// A peer answered our request for what it says about itself.
|
||||||
|
PeerInfoReceived {
|
||||||
|
username: String,
|
||||||
|
info: crate::message::peer::PeerInfo,
|
||||||
|
},
|
||||||
/// A peer answered our request for one folder of their shares.
|
/// A peer answered our request for one folder of their shares.
|
||||||
FolderContents {
|
FolderContents {
|
||||||
username: String,
|
username: String,
|
||||||
@@ -494,6 +499,8 @@ pub struct ClientContext {
|
|||||||
browse_results: HashMap<String, Vec<SharedDirectory>>,
|
browse_results: HashMap<String, Vec<SharedDirectory>>,
|
||||||
/// One-folder listings received from peers, keyed by peer and folder.
|
/// One-folder listings received from peers, keyed by peer and folder.
|
||||||
folder_contents: HashMap<(String, String), Vec<SharedDirectory>>,
|
folder_contents: HashMap<(String, String), Vec<SharedDirectory>>,
|
||||||
|
/// What peers said about themselves (peer code 16), keyed by peer.
|
||||||
|
peer_infos: HashMap<String, crate::message::peer::PeerInfo>,
|
||||||
pending_browses: HashMap<String, Instant>,
|
pending_browses: HashMap<String, Instant>,
|
||||||
/// Latest snapshot of the public chat-room list (from `RoomList`, code 64).
|
/// Latest snapshot of the public chat-room list (from `RoomList`, code 64).
|
||||||
room_list: Vec<RoomInfo>,
|
room_list: Vec<RoomInfo>,
|
||||||
@@ -651,6 +658,7 @@ impl ClientContext {
|
|||||||
pending_serves: HashMap::new(),
|
pending_serves: HashMap::new(),
|
||||||
browse_results: HashMap::new(),
|
browse_results: HashMap::new(),
|
||||||
folder_contents: HashMap::new(),
|
folder_contents: HashMap::new(),
|
||||||
|
peer_infos: HashMap::new(),
|
||||||
pending_browses: HashMap::new(),
|
pending_browses: HashMap::new(),
|
||||||
room_list: Vec::new(),
|
room_list: Vec::new(),
|
||||||
room_events: Vec::new(),
|
room_events: Vec::new(),
|
||||||
@@ -863,6 +871,30 @@ impl ClientContext {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Record what `username` says about itself.
|
||||||
|
pub fn store_peer_info(
|
||||||
|
&mut self,
|
||||||
|
username: String,
|
||||||
|
info: crate::message::peer::PeerInfo,
|
||||||
|
) {
|
||||||
|
self.peer_infos.insert(username, info);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// What `username` last said about itself, if anything.
|
||||||
|
#[must_use]
|
||||||
|
pub fn peer_info(
|
||||||
|
&self,
|
||||||
|
username: &str,
|
||||||
|
) -> Option<crate::message::peer::PeerInfo> {
|
||||||
|
self.peer_infos.get(username).cloned()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Drop what we hold about `username`, so a poll after a fresh request
|
||||||
|
/// cannot report the previous answer.
|
||||||
|
pub fn invalidate_peer_info(&mut self, username: &str) {
|
||||||
|
self.peer_infos.remove(username);
|
||||||
|
}
|
||||||
|
|
||||||
/// Record one folder's listing from `username`.
|
/// Record one folder's listing from `username`.
|
||||||
pub fn store_folder_contents(
|
pub fn store_folder_contents(
|
||||||
&mut self,
|
&mut self,
|
||||||
|
|||||||
@@ -919,6 +919,11 @@ impl Client {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
ClientOperation::PeerInfoReceived { username, info } => {
|
||||||
|
if let Ok(mut ctx) = client_context.write_safe() {
|
||||||
|
ctx.store_peer_info(username, info);
|
||||||
|
}
|
||||||
|
}
|
||||||
ClientOperation::FolderContents {
|
ClientOperation::FolderContents {
|
||||||
username,
|
username,
|
||||||
token,
|
token,
|
||||||
|
|||||||
@@ -166,6 +166,34 @@ impl Client {
|
|||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Ask `username` what it says about itself (peer code 15): its
|
||||||
|
/// description, upload slots and queue length — a user's profile as other
|
||||||
|
/// clients show it.
|
||||||
|
///
|
||||||
|
/// Any previous answer is dropped first, so [`Client::peer_info`] reports
|
||||||
|
/// the reply to this request rather than a stale one.
|
||||||
|
///
|
||||||
|
/// # Errors
|
||||||
|
/// [`crate::SoulseekRs::NotConnected`] when there is no server connection,
|
||||||
|
/// or [`crate::SoulseekRs::LockPoisoned`] if the client state is poisoned.
|
||||||
|
pub fn request_peer_info(&self, username: &str) -> Result<()> {
|
||||||
|
self.context.write_safe()?.invalidate_peer_info(username);
|
||||||
|
let request = crate::message::peer::build_user_info_request();
|
||||||
|
self.send_to_peer_or_queue(username, request)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// What `username` last said about itself.
|
||||||
|
#[must_use]
|
||||||
|
pub fn peer_info(
|
||||||
|
&self,
|
||||||
|
username: &str,
|
||||||
|
) -> Option<crate::message::peer::PeerInfo> {
|
||||||
|
self.context
|
||||||
|
.read_safe()
|
||||||
|
.ok()
|
||||||
|
.and_then(|ctx| ctx.peer_info(username))
|
||||||
|
}
|
||||||
|
|
||||||
/// Ask `username` for the contents of one folder of their shares (peer
|
/// Ask `username` for the contents of one folder of their shares (peer
|
||||||
/// code 36) — what "download folder" in other clients asks for, without
|
/// code 36) — what "download folder" in other clients asks for, without
|
||||||
/// pulling their whole listing.
|
/// pulling their whole listing.
|
||||||
|
|||||||
@@ -34,4 +34,7 @@ pub use transfer_request::TransferRequest;
|
|||||||
pub use transfer_response::TransferResponse;
|
pub use transfer_response::TransferResponse;
|
||||||
pub use upload_denied::UploadDeniedHandler;
|
pub use upload_denied::UploadDeniedHandler;
|
||||||
pub use upload_failed::UploadFailedHandler;
|
pub use upload_failed::UploadFailedHandler;
|
||||||
pub use user_info::{UserInfoRequest, build_user_info};
|
pub use user_info::{
|
||||||
|
PeerInfo, UserInfoRequest, UserInfoResponseHandler, build_user_info,
|
||||||
|
build_user_info_request,
|
||||||
|
};
|
||||||
|
|||||||
@@ -15,6 +15,71 @@ impl MessageHandler<PeerMessage> for UserInfoRequest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Build a `UserInfoRequest` (peer code 15, no body): ask a peer what it says
|
||||||
|
/// about itself.
|
||||||
|
#[must_use]
|
||||||
|
pub fn build_user_info_request() -> Message {
|
||||||
|
Message::new().write_int32(15).clone()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// What a peer says about itself, from its `UserInfoResponse` (code 16).
|
||||||
|
#[derive(Debug, Clone, Default, PartialEq, Eq)]
|
||||||
|
pub struct PeerInfo {
|
||||||
|
pub description: String,
|
||||||
|
/// Whether a profile picture came with it, and how many bytes it was. The
|
||||||
|
/// image itself is not kept: no caller has a use for it yet, and a peer
|
||||||
|
/// can make it arbitrarily large.
|
||||||
|
pub picture_bytes: Option<usize>,
|
||||||
|
/// Uploads the peer says it has done in total.
|
||||||
|
pub total_uploads: u32,
|
||||||
|
/// How many transfers are waiting in its queue.
|
||||||
|
pub queue_size: u32,
|
||||||
|
/// Whether it has a free upload slot right now.
|
||||||
|
pub slots_free: bool,
|
||||||
|
/// Whether it would let us queue anything at all. Absent from older
|
||||||
|
/// clients, which stop the message one field early.
|
||||||
|
pub upload_allowed: Option<u32>,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// `UserInfoResponse` (peer code 16): a peer's answer about itself.
|
||||||
|
pub struct UserInfoResponseHandler;
|
||||||
|
impl MessageHandler<PeerMessage> for UserInfoResponseHandler {
|
||||||
|
fn get_code(&self) -> u32 {
|
||||||
|
16
|
||||||
|
}
|
||||||
|
fn handle(&self, message: &mut Message, sender: Sender<PeerMessage>) {
|
||||||
|
let description = message.read_string();
|
||||||
|
let has_picture = message.read_bool();
|
||||||
|
let picture_bytes = has_picture.then(|| {
|
||||||
|
let length = message.read_int32() as usize;
|
||||||
|
let available =
|
||||||
|
message.get_size().saturating_sub(message.get_pointer());
|
||||||
|
let length = length.min(available);
|
||||||
|
let at = message.get_pointer() + length;
|
||||||
|
message.set_pointer(at);
|
||||||
|
length
|
||||||
|
});
|
||||||
|
let total_uploads = message.read_int32();
|
||||||
|
let queue_size = message.read_int32();
|
||||||
|
let slots_free = message.read_bool();
|
||||||
|
// Museek+ writes `slotsavail` as an integer, leaving three bytes of
|
||||||
|
// rubbish where the next field starts; Nicotine+ guards the read the
|
||||||
|
// same way, by only taking a field there is room for.
|
||||||
|
let upload_allowed =
|
||||||
|
(message.get_size().saturating_sub(message.get_pointer()) >= 4)
|
||||||
|
.then(|| message.read_int32());
|
||||||
|
|
||||||
|
let _ = sender.send(PeerMessage::UserInfoReceived(PeerInfo {
|
||||||
|
description,
|
||||||
|
picture_bytes,
|
||||||
|
total_uploads,
|
||||||
|
queue_size,
|
||||||
|
slots_free,
|
||||||
|
upload_allowed,
|
||||||
|
}));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Build a `UserInfoResponse` (peer code 16) with no description or picture.
|
/// Build a `UserInfoResponse` (peer code 16) with no description or picture.
|
||||||
#[must_use]
|
#[must_use]
|
||||||
pub fn build_user_info(
|
pub fn build_user_info(
|
||||||
@@ -31,3 +96,86 @@ pub fn build_user_info(
|
|||||||
.write_bool(slots_free)
|
.write_bool(slots_free)
|
||||||
.clone()
|
.clone()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
use crate::message::framed;
|
||||||
|
|
||||||
|
fn parse(build: impl FnOnce(&mut Message)) -> PeerInfo {
|
||||||
|
let (tx, rx) = std::sync::mpsc::channel();
|
||||||
|
let mut message = framed(build);
|
||||||
|
UserInfoResponseHandler.handle(&mut message, tx);
|
||||||
|
match rx.try_recv() {
|
||||||
|
Ok(PeerMessage::UserInfoReceived(info)) => info,
|
||||||
|
other => panic!("unexpected: {other:?}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_peers_answer_carries_its_slots_and_queue() {
|
||||||
|
let info = parse(|m| {
|
||||||
|
m.write_string("hello, I share flacs");
|
||||||
|
m.write_bool(false);
|
||||||
|
m.write_int32(42); // total uploads
|
||||||
|
m.write_int32(3); // queue size
|
||||||
|
m.write_bool(true); // a slot is free
|
||||||
|
m.write_int32(1); // uploads allowed
|
||||||
|
});
|
||||||
|
|
||||||
|
assert_eq!(info.description, "hello, I share flacs");
|
||||||
|
assert_eq!(info.picture_bytes, None);
|
||||||
|
assert_eq!(info.total_uploads, 42);
|
||||||
|
assert_eq!(info.queue_size, 3);
|
||||||
|
assert!(info.slots_free);
|
||||||
|
assert_eq!(info.upload_allowed, Some(1));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_picture_is_skipped_over_rather_than_kept() {
|
||||||
|
let info = parse(|m| {
|
||||||
|
m.write_string("with a picture");
|
||||||
|
m.write_bool(true);
|
||||||
|
m.write_int32(4);
|
||||||
|
m.write_raw_bytes(vec![1, 2, 3, 4]);
|
||||||
|
m.write_int32(7);
|
||||||
|
m.write_int32(0);
|
||||||
|
m.write_bool(false);
|
||||||
|
m.write_int32(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
assert_eq!(info.picture_bytes, Some(4));
|
||||||
|
assert_eq!(info.total_uploads, 7, "the fields after it still line up");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_message_that_stops_early_still_parses() {
|
||||||
|
// Older clients omit the trailing field, and Museek+ leaves rubbish
|
||||||
|
// where it would be; neither may cost us the fields before it.
|
||||||
|
let info = parse(|m| {
|
||||||
|
m.write_string("older client");
|
||||||
|
m.write_bool(false);
|
||||||
|
m.write_int32(5);
|
||||||
|
m.write_int32(1);
|
||||||
|
m.write_bool(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
assert_eq!(info.total_uploads, 5);
|
||||||
|
assert_eq!(info.queue_size, 1);
|
||||||
|
assert!(info.slots_free);
|
||||||
|
assert_eq!(info.upload_allowed, None);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_picture_longer_than_the_message_does_not_run_off_the_end() {
|
||||||
|
let info = parse(|m| {
|
||||||
|
m.write_string("hostile");
|
||||||
|
m.write_bool(true);
|
||||||
|
m.write_int32(u32::MAX);
|
||||||
|
m.write_raw_bytes(vec![9, 9]);
|
||||||
|
});
|
||||||
|
|
||||||
|
assert_eq!(info.picture_bytes, Some(2));
|
||||||
|
assert_eq!(info.total_uploads, 0, "nothing left to read reads as zero");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -5176,3 +5176,39 @@ fn a_finished_upload_teaches_us_our_own_recorded_speed() {
|
|||||||
let _ = std::fs::remove_dir_all(share_dir);
|
let _ = std::fs::remove_dir_all(share_dir);
|
||||||
let _ = std::fs::remove_dir_all(download_dir);
|
let _ = std::fs::remove_dir_all(download_dir);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_peer_tells_us_about_itself_when_asked() {
|
||||||
|
// The profile another client shows for a user: their description, upload
|
||||||
|
// slots and queue length, straight from the peer (code 15 and its reply).
|
||||||
|
let server = server_or_skip!();
|
||||||
|
|
||||||
|
let share_dir = unique_download_dir();
|
||||||
|
std::fs::write(share_dir.join("e2e_peerinfo.bin"), b"data").unwrap();
|
||||||
|
let (_sharer, asker) = sharer_and_searcher(
|
||||||
|
&server,
|
||||||
|
&share_dir,
|
||||||
|
"e2e_peerinfo_sharer",
|
||||||
|
"e2e_peerinfo_asker",
|
||||||
|
);
|
||||||
|
|
||||||
|
let deadline = Instant::now() + Duration::from_secs(20);
|
||||||
|
let mut info = None;
|
||||||
|
while Instant::now() < deadline && info.is_none() {
|
||||||
|
asker
|
||||||
|
.request_peer_info("e2e_peerinfo_sharer")
|
||||||
|
.expect("ask the peer about itself");
|
||||||
|
std::thread::sleep(Duration::from_millis(500));
|
||||||
|
info = asker.peer_info("e2e_peerinfo_sharer");
|
||||||
|
}
|
||||||
|
let info = info.expect("the peer should answer for itself");
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
info.slots_free,
|
||||||
|
"a peer with nothing queued has a free slot, got {info:?}"
|
||||||
|
);
|
||||||
|
assert_eq!(info.queue_size, 0);
|
||||||
|
assert_eq!(info.picture_bytes, None);
|
||||||
|
|
||||||
|
let _ = std::fs::remove_dir_all(share_dir);
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user