feat(lib): per-member room statistics from the JoinRoom reply
The JoinRoom (code 14) reply carries, after the membership list, four parallel vectors describing each member: status, share statistics, free upload slots and country. The handler stopped after the usernames, so all of it was discarded. It is now parsed and exposed as Client::room_member_stats. Issue #48 asked for RoomStats (57) and RoomListStats (55). Those codes are UserInterests and MyRecommendations in the Soulseek protocol; there is no RoomStats message on the wire. Per-room user counts already arrive with RoomList (64), which the client handles, and the per-member statistics are part of the JoinRoom reply -- so that is where this adds them. - join_room.rs: parse the stat vectors, each bounded by the payload so a hostile count cannot hang or overallocate - types::RoomUserStats; every field looked up positionally so a server sending short vectors (or omitting countries) does not shift the roster - ServerMessage/ClientOperation::RoomMemberStats and the context snapshot behind Client::room_member_stats, replaced on each rejoin
This commit is contained in:
@@ -27,7 +27,8 @@ use crate::message::{Message, MessageReader};
|
||||
use crate::peer::ConnectionType;
|
||||
use crate::peer::Peer;
|
||||
use crate::types::{
|
||||
ClientVersion, RoomEvent, RoomInfo, SessionLoss, SessionWatch,
|
||||
ClientVersion, RoomEvent, RoomInfo, RoomUserStats, SessionLoss,
|
||||
SessionWatch,
|
||||
};
|
||||
use crate::utils::lock::RwLockExt;
|
||||
|
||||
@@ -213,6 +214,11 @@ pub enum ServerMessage {
|
||||
room: String,
|
||||
users: Vec<String>,
|
||||
},
|
||||
/// Per-member statistics carried by the same `JoinRoom` (code 14) reply.
|
||||
RoomMemberStats {
|
||||
room: String,
|
||||
stats: Vec<RoomUserStats>,
|
||||
},
|
||||
RoomLeft {
|
||||
room: String,
|
||||
},
|
||||
@@ -480,6 +486,12 @@ impl ServerActor {
|
||||
ServerMessage::RoomJoined { room, users } => {
|
||||
self.forward_room_event(RoomEvent::Joined { room, users });
|
||||
}
|
||||
ServerMessage::RoomMemberStats { room, stats } => {
|
||||
self.forward_to_client(ClientOperation::RoomMemberStats {
|
||||
room,
|
||||
stats,
|
||||
});
|
||||
}
|
||||
ServerMessage::RoomLeft { room } => {
|
||||
self.forward_room_event(RoomEvent::Left { room });
|
||||
}
|
||||
|
||||
@@ -5,7 +5,8 @@ use crate::actor::server_actor::{
|
||||
use crate::download_store::{DownloadStore, collect_failed_tokens};
|
||||
use crate::types::{
|
||||
ClientVersion, DownloadMetadata, DownloadStatus, RoomEvent, RoomInfo,
|
||||
SessionLoss, SessionWatch, UserInfo, UserPresence, UserStats, UserStatus,
|
||||
RoomUserStats, SessionLoss, SessionWatch, UserInfo, UserPresence,
|
||||
UserStats, UserStatus,
|
||||
};
|
||||
use crate::utils::logger;
|
||||
use crate::{
|
||||
@@ -278,6 +279,11 @@ pub enum ClientOperation {
|
||||
/// Something happened in the chat-room subsystem (list refreshed, a room
|
||||
/// joined/left, a message said, a member joined/left).
|
||||
RoomEvent(RoomEvent),
|
||||
/// Per-member statistics from a `JoinRoom` reply.
|
||||
RoomMemberStats {
|
||||
room: String,
|
||||
stats: Vec<RoomUserStats>,
|
||||
},
|
||||
/// The server announced how many seconds must pass between wishlist
|
||||
/// searches.
|
||||
WishlistInterval(u32),
|
||||
@@ -325,6 +331,9 @@ pub struct ClientContext {
|
||||
/// Who is in each room we have joined, kept current from the membership
|
||||
/// the server sends on join plus the later join/leave events.
|
||||
room_members: HashMap<String, Vec<String>>,
|
||||
/// Per-member statistics for each joined room, from the stat vectors the
|
||||
/// server sends alongside the membership list.
|
||||
room_member_stats: HashMap<String, Vec<RoomUserStats>>,
|
||||
/// What the server has told us about other users, merged across the
|
||||
/// separate status and statistics replies.
|
||||
user_info: HashMap<String, UserInfo>,
|
||||
@@ -415,6 +424,7 @@ impl ClientContext {
|
||||
room_list: Vec::new(),
|
||||
room_events: Vec::new(),
|
||||
room_members: HashMap::new(),
|
||||
room_member_stats: HashMap::new(),
|
||||
user_info: HashMap::new(),
|
||||
wishlist_interval: None,
|
||||
privileged_users: HashSet::new(),
|
||||
@@ -515,6 +525,29 @@ impl ClientContext {
|
||||
self.room_members.get(room).cloned().unwrap_or_default()
|
||||
}
|
||||
|
||||
/// Record the per-member statistics of a room we just joined, replacing
|
||||
/// any previous snapshot for it.
|
||||
pub fn apply_room_member_stats(
|
||||
&mut self,
|
||||
room: String,
|
||||
stats: Vec<RoomUserStats>,
|
||||
) {
|
||||
self.room_member_stats.insert(room, stats);
|
||||
}
|
||||
|
||||
/// What the server reported about the members of `room`, sorted by name,
|
||||
/// or empty for a room we have not joined.
|
||||
#[must_use]
|
||||
pub fn room_member_stats(&self, room: &str) -> Vec<RoomUserStats> {
|
||||
let mut stats = self
|
||||
.room_member_stats
|
||||
.get(room)
|
||||
.cloned()
|
||||
.unwrap_or_default();
|
||||
stats.sort_by(|a, b| a.username.cmp(&b.username));
|
||||
stats
|
||||
}
|
||||
|
||||
/// The latest snapshot of the public chat-room list.
|
||||
#[must_use]
|
||||
pub fn room_list(&self) -> Vec<RoomInfo> {
|
||||
|
||||
@@ -479,6 +479,16 @@ impl Client {
|
||||
Err(e) => error!("[client] RoomEvent write: {}", e),
|
||||
}
|
||||
}
|
||||
ClientOperation::RoomMemberStats { room, stats } => {
|
||||
match client_context.write_safe() {
|
||||
Ok(mut ctx) => {
|
||||
ctx.apply_room_member_stats(room, stats);
|
||||
}
|
||||
Err(e) => {
|
||||
error!("[client] RoomMemberStats write: {}", e);
|
||||
}
|
||||
}
|
||||
}
|
||||
ClientOperation::WishlistInterval(seconds) => {
|
||||
if let Ok(mut ctx) = client_context.write_safe() {
|
||||
ctx.wishlist_interval = Some(seconds);
|
||||
|
||||
@@ -2,7 +2,7 @@ use super::{
|
||||
Client, PeerMessage, Result, RoomEvent, RoomInfo, RwLockExt, ServerMessage,
|
||||
SharedDirectory, SoulseekRs, UserMessage, error,
|
||||
};
|
||||
use crate::types::UserInfo;
|
||||
use crate::types::{RoomUserStats, UserInfo};
|
||||
|
||||
impl Client {
|
||||
/// Send a private message to another user via the server.
|
||||
@@ -156,6 +156,24 @@ impl Client {
|
||||
}
|
||||
}
|
||||
|
||||
/// What the server reported about the members of `room` when we joined:
|
||||
/// their status, share statistics, free upload slots and country.
|
||||
///
|
||||
/// Populated from the same reply that fills [`Client::room_members`], so
|
||||
/// it lands shortly after joining. Unlike the roster it is not updated by
|
||||
/// later join and leave events -- it is the snapshot taken at join time.
|
||||
/// Returns an empty list for a room this client has not joined.
|
||||
#[must_use]
|
||||
pub fn room_member_stats(&self, room: &str) -> Vec<RoomUserStats> {
|
||||
match self.context.read_safe() {
|
||||
Ok(ctx) => ctx.room_member_stats(room),
|
||||
Err(e) => {
|
||||
error!("[client] room_member_stats: {}", e);
|
||||
Vec::new()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Remove and return all chat-room events received since the last call.
|
||||
#[must_use]
|
||||
pub fn take_room_events(&self) -> Vec<RoomEvent> {
|
||||
|
||||
@@ -622,3 +622,53 @@ fn a_replayed_transfer_response_does_not_start_a_second_transfer() {
|
||||
"a replayed TransferResponse must not dial the peer again"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn room_member_stats_are_returned_sorted_and_scoped_to_their_room() {
|
||||
use crate::types::{RoomUserStats, UserStatus};
|
||||
let stat = |username: &str| RoomUserStats {
|
||||
username: username.to_string(),
|
||||
status: UserStatus::Online,
|
||||
average_speed: 1,
|
||||
shared_files: 2,
|
||||
shared_folders: 3,
|
||||
slots_free: 1,
|
||||
country: None,
|
||||
};
|
||||
|
||||
let mut context = ClientContext::new();
|
||||
context.apply_room_member_stats(
|
||||
"jazz".to_string(),
|
||||
vec![stat("carol"), stat("alice")],
|
||||
);
|
||||
|
||||
let names: Vec<String> = context
|
||||
.room_member_stats("jazz")
|
||||
.into_iter()
|
||||
.map(|s| s.username)
|
||||
.collect();
|
||||
assert_eq!(names, vec!["alice", "carol"]);
|
||||
assert!(context.room_member_stats("unjoined").is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejoining_replaces_the_previous_member_stats() {
|
||||
use crate::types::{RoomUserStats, UserStatus};
|
||||
let stat = |username: &str| RoomUserStats {
|
||||
username: username.to_string(),
|
||||
status: UserStatus::Online,
|
||||
average_speed: 1,
|
||||
shared_files: 2,
|
||||
shared_folders: 3,
|
||||
slots_free: 0,
|
||||
country: None,
|
||||
};
|
||||
|
||||
let mut context = ClientContext::new();
|
||||
context.apply_room_member_stats("jazz".to_string(), vec![stat("alice")]);
|
||||
context.apply_room_member_stats("jazz".to_string(), vec![stat("bob")]);
|
||||
|
||||
let stats = context.room_member_stats("jazz");
|
||||
assert_eq!(stats.len(), 1, "a rejoin supersedes the old snapshot");
|
||||
assert_eq!(stats[0].username, "bob");
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use crate::{
|
||||
actor::server_actor::ServerMessage,
|
||||
message::{Message, MessageHandler},
|
||||
types::{RoomUserStats, UserStatus},
|
||||
};
|
||||
use std::sync::mpsc::Sender;
|
||||
|
||||
@@ -12,24 +13,104 @@ impl MessageHandler<ServerMessage> for JoinRoomHandler {
|
||||
}
|
||||
|
||||
fn handle(&self, message: &mut Message, sender: Sender<ServerMessage>) {
|
||||
// JoinRoom (code 14): room name, then a vector of member usernames.
|
||||
// Per-user stat vectors follow but are not needed here, so we stop
|
||||
// after reading the names.
|
||||
// JoinRoom (code 14): room name, the member usernames, then the
|
||||
// parallel per-member stat vectors.
|
||||
let room = message.read_string();
|
||||
let user_count = message.read_int32();
|
||||
let mut users = Vec::new();
|
||||
for _ in 0..user_count {
|
||||
// Guard against a hostile user_count outrunning the payload; each
|
||||
// username is a length-prefixed string (>= 4 bytes).
|
||||
if message.get_pointer() + 4 > message.get_size() {
|
||||
break;
|
||||
}
|
||||
users.push(message.read_string());
|
||||
let users = read_strings(message);
|
||||
let stats = parse_member_stats(message, &users);
|
||||
let _ = sender.send(ServerMessage::RoomJoined {
|
||||
room: room.clone(),
|
||||
users,
|
||||
});
|
||||
if !stats.is_empty() {
|
||||
let _ = sender.send(ServerMessage::RoomMemberStats { room, stats });
|
||||
}
|
||||
let _ = sender.send(ServerMessage::RoomJoined { room, users });
|
||||
}
|
||||
}
|
||||
|
||||
/// Read a length-prefixed vector of strings, stopping early if the count
|
||||
/// outruns the payload: a bogus (possibly hostile) count must not spin us
|
||||
/// into an OOM allocation loop. A 4-byte floor is the smallest a
|
||||
/// length-prefixed string can be.
|
||||
fn read_strings(message: &mut Message) -> Vec<String> {
|
||||
let count = message.read_int32();
|
||||
let mut values = Vec::new();
|
||||
for _ in 0..count {
|
||||
if message.get_pointer() + 4 > message.get_size() {
|
||||
break;
|
||||
}
|
||||
values.push(message.read_string());
|
||||
}
|
||||
values
|
||||
}
|
||||
|
||||
/// Read a length-prefixed vector of 32-bit values, bounded the same way.
|
||||
fn read_int32s(message: &mut Message) -> Vec<u32> {
|
||||
let count = message.read_int32();
|
||||
let mut values = Vec::new();
|
||||
for _ in 0..count {
|
||||
if message.get_pointer() + 4 > message.get_size() {
|
||||
break;
|
||||
}
|
||||
values.push(message.read_int32());
|
||||
}
|
||||
values
|
||||
}
|
||||
|
||||
/// Parse the per-member statistics that follow the membership list of a
|
||||
/// `JoinRoom` (code 14) reply: statuses, then a record of share statistics per
|
||||
/// member, then free upload slots, then country codes.
|
||||
///
|
||||
/// Each vector is independently length-prefixed and a server may send fewer
|
||||
/// entries than there are members (or, on an older server, omit the country
|
||||
/// vector entirely), so every field is looked up positionally and defaults
|
||||
/// when absent rather than shifting the whole roster.
|
||||
#[must_use]
|
||||
fn parse_member_stats(
|
||||
message: &mut Message,
|
||||
users: &[String],
|
||||
) -> Vec<RoomUserStats> {
|
||||
let statuses = read_int32s(message);
|
||||
|
||||
let stat_count = message.read_int32();
|
||||
let mut shares = Vec::new();
|
||||
for _ in 0..stat_count {
|
||||
// A share record is 20 bytes: speed, an obsolete 64-bit upload
|
||||
// number, files and folders.
|
||||
if message.get_pointer() + 20 > message.get_size() {
|
||||
break;
|
||||
}
|
||||
let average_speed = message.read_int32();
|
||||
let _upload_number = message.read_int64(); // obsolete
|
||||
let shared_files = message.read_int32();
|
||||
let shared_folders = message.read_int32();
|
||||
shares.push((average_speed, shared_files, shared_folders));
|
||||
}
|
||||
|
||||
let slots_free = read_int32s(message);
|
||||
let countries = read_strings(message);
|
||||
|
||||
users
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, username)| {
|
||||
let (average_speed, shared_files, shared_folders) =
|
||||
shares.get(i).copied().unwrap_or((0, 0, 0));
|
||||
RoomUserStats {
|
||||
username: username.clone(),
|
||||
status: UserStatus::from_code(
|
||||
statuses.get(i).copied().unwrap_or(0),
|
||||
),
|
||||
average_speed,
|
||||
shared_files,
|
||||
shared_folders,
|
||||
slots_free: slots_free.get(i).copied().unwrap_or(0),
|
||||
country: countries.get(i).cloned().filter(|c| !c.is_empty()),
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -53,6 +134,158 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// A full JoinRoom payload: room, members, then the four parallel stat
|
||||
/// vectors the server sends for them.
|
||||
fn join_payload() -> Message {
|
||||
framed(|m| {
|
||||
m.write_string("nicotine")
|
||||
.write_int32(2)
|
||||
.write_string("alice")
|
||||
.write_string("bob")
|
||||
// statuses
|
||||
.write_int32(2)
|
||||
.write_int32(2)
|
||||
.write_int32(1)
|
||||
// share records
|
||||
.write_int32(2)
|
||||
.write_int32(1024)
|
||||
.write_int64(0)
|
||||
.write_int32(4321)
|
||||
.write_int32(77)
|
||||
.write_int32(50)
|
||||
.write_int64(0)
|
||||
.write_int32(10)
|
||||
.write_int32(2)
|
||||
// free slots
|
||||
.write_int32(2)
|
||||
.write_int32(1)
|
||||
.write_int32(0)
|
||||
// countries
|
||||
.write_int32(2)
|
||||
.write_string("NL")
|
||||
.write_string("");
|
||||
})
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn member_stats_are_parsed_alongside_the_roster() {
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
JoinRoomHandler.handle(&mut join_payload(), tx);
|
||||
|
||||
assert!(matches!(
|
||||
rx.try_recv(),
|
||||
Ok(ServerMessage::RoomJoined { .. })
|
||||
));
|
||||
match rx.try_recv() {
|
||||
Ok(ServerMessage::RoomMemberStats { room, stats }) => {
|
||||
assert_eq!(room, "nicotine");
|
||||
assert_eq!(
|
||||
stats,
|
||||
vec![
|
||||
RoomUserStats {
|
||||
username: "alice".to_string(),
|
||||
status: UserStatus::Online,
|
||||
average_speed: 1024,
|
||||
shared_files: 4321,
|
||||
shared_folders: 77,
|
||||
slots_free: 1,
|
||||
country: Some("NL".to_string()),
|
||||
},
|
||||
RoomUserStats {
|
||||
username: "bob".to_string(),
|
||||
status: UserStatus::Away,
|
||||
average_speed: 50,
|
||||
shared_files: 10,
|
||||
shared_folders: 2,
|
||||
slots_free: 0,
|
||||
country: None,
|
||||
},
|
||||
]
|
||||
);
|
||||
}
|
||||
other => panic!("unexpected: {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_server_that_omits_the_stat_vectors_still_yields_a_roster() {
|
||||
// Nothing after the membership list: every stat reads as a default
|
||||
// rather than shifting the roster or dropping members.
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
let mut message = framed(|m| {
|
||||
m.write_string("jazz").write_int32(1).write_string("alice");
|
||||
});
|
||||
|
||||
JoinRoomHandler.handle(&mut message, tx);
|
||||
assert!(matches!(
|
||||
rx.try_recv(),
|
||||
Ok(ServerMessage::RoomJoined { .. })
|
||||
));
|
||||
match rx.try_recv() {
|
||||
Ok(ServerMessage::RoomMemberStats { stats, .. }) => {
|
||||
assert_eq!(stats.len(), 1);
|
||||
assert_eq!(stats[0].username, "alice");
|
||||
assert_eq!(stats[0].average_speed, 0);
|
||||
assert_eq!(stats[0].country, None);
|
||||
}
|
||||
other => panic!("unexpected: {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn short_stat_vectors_do_not_shift_the_roster() {
|
||||
// Stats for the first member only: the second must still be reported,
|
||||
// with defaults, rather than inheriting the first member's figures.
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
let mut message = framed(|m| {
|
||||
m.write_string("jazz")
|
||||
.write_int32(2)
|
||||
.write_string("alice")
|
||||
.write_string("bob")
|
||||
.write_int32(1)
|
||||
.write_int32(2)
|
||||
.write_int32(1)
|
||||
.write_int32(1024)
|
||||
.write_int64(0)
|
||||
.write_int32(9)
|
||||
.write_int32(1)
|
||||
.write_int32(0)
|
||||
.write_int32(0);
|
||||
});
|
||||
|
||||
JoinRoomHandler.handle(&mut message, tx);
|
||||
let _ = rx.try_recv();
|
||||
match rx.try_recv() {
|
||||
Ok(ServerMessage::RoomMemberStats { stats, .. }) => {
|
||||
assert_eq!(stats.len(), 2);
|
||||
assert_eq!(stats[0].shared_files, 9);
|
||||
assert_eq!(stats[1].username, "bob");
|
||||
assert_eq!(stats[1].shared_files, 0);
|
||||
assert_eq!(stats[1].status, UserStatus::Offline);
|
||||
}
|
||||
other => panic!("unexpected: {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hostile_stat_counts_do_not_hang_or_overallocate() {
|
||||
// Vectors claiming ~4 billion entries in a tiny frame must return
|
||||
// promptly, bounded by the payload.
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
let mut message = framed(|m| {
|
||||
m.write_string("jazz")
|
||||
.write_int32(1)
|
||||
.write_string("alice")
|
||||
.write_int32(u32::MAX);
|
||||
});
|
||||
|
||||
JoinRoomHandler.handle(&mut message, tx);
|
||||
assert!(matches!(
|
||||
rx.try_recv(),
|
||||
Ok(ServerMessage::RoomJoined { .. })
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn forwards_room_and_member_list() {
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
|
||||
@@ -328,6 +328,25 @@ impl UserInfo {
|
||||
}
|
||||
}
|
||||
|
||||
/// What the server reports about one member of a room we joined, from the
|
||||
/// per-member stat vectors of a `JoinRoom` (code 14) reply.
|
||||
///
|
||||
/// A server may send shorter stat vectors than the membership list (older
|
||||
/// servers omit country codes entirely), so absent fields read as zero and an
|
||||
/// unreported country is `None` rather than an empty string.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct RoomUserStats {
|
||||
pub username: String,
|
||||
pub status: UserStatus,
|
||||
pub average_speed: u32,
|
||||
pub shared_files: u32,
|
||||
pub shared_folders: u32,
|
||||
/// Upload slots the member has free; zero means they are busy.
|
||||
pub slots_free: u32,
|
||||
/// Two-letter country code, when the server reports one.
|
||||
pub country: Option<String>,
|
||||
}
|
||||
|
||||
/// A public chat room advertised by the server (`RoomList`, code 64).
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct RoomInfo {
|
||||
|
||||
Reference in New Issue
Block a user