diff --git a/soulseek-rs-lib/src/message/server/join_room.rs b/soulseek-rs-lib/src/message/server/join_room.rs index f31071a..67e31e0 100644 --- a/soulseek-rs-lib/src/message/server/join_room.rs +++ b/soulseek-rs-lib/src/message/server/join_room.rs @@ -19,6 +19,11 @@ impl MessageHandler for JoinRoomHandler { 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 _ = sender.send(ServerMessage::RoomJoined { room, users }); @@ -29,6 +34,26 @@ impl MessageHandler for JoinRoomHandler { mod tests { use super::*; + #[test] + fn hostile_user_count_does_not_hang() { + // room="" then user_count=u32::MAX with no usernames: must return + // promptly instead of looping ~4 billion times. + let (tx, rx) = std::sync::mpsc::channel(); + let mut message = Message::new(); + message.write_raw_bytes(vec![0u8; 8]); + message.write_string(""); + message.write_int32(u32::MAX); + message.set_pointer(8); + + JoinRoomHandler.handle(&mut message, tx); + match rx.try_recv() { + Ok(ServerMessage::RoomJoined { users, .. }) => { + assert!(users.is_empty()); + } + other => panic!("unexpected: {other:?}"), + } + } + #[test] fn forwards_room_and_member_list() { let (tx, rx) = std::sync::mpsc::channel(); diff --git a/soulseek-rs-lib/src/message/server/room_list.rs b/soulseek-rs-lib/src/message/server/room_list.rs index e102578..5e779d8 100644 --- a/soulseek-rs-lib/src/message/server/room_list.rs +++ b/soulseek-rs-lib/src/message/server/room_list.rs @@ -27,11 +27,20 @@ pub fn parse_room_list(message: &mut Message) -> Vec { let name_count = message.read_int32(); let mut names = Vec::new(); for _ in 0..name_count { + // Stop once the payload can't hold another field: a bogus (possibly + // hostile) count must not spin us into an OOM allocation loop. A + // 4-byte floor also avoids read_int32 stalling on a 1-3 byte tail. + if message.get_pointer() + 4 > message.get_size() { + break; + } names.push(message.read_string()); } let count_count = message.read_int32(); let mut counts = Vec::new(); for _ in 0..count_count { + if message.get_pointer() + 4 > message.get_size() { + break; + } counts.push(message.read_int32()); } names @@ -90,6 +99,17 @@ mod tests { assert!(parse_room_list(&mut message).is_empty()); } + #[test] + fn hostile_counts_do_not_hang_or_overallocate() { + // A tiny frame claiming ~4 billion names/counts must return promptly + // (bounded by the payload) rather than looping into an OOM. + let mut message = framed(|m| { + m.write_int32(u32::MAX); + }); + let rooms = parse_room_list(&mut message); + assert!(rooms.is_empty()); + } + #[test] fn handler_forwards_parsed_rooms() { let (tx, rx) = std::sync::mpsc::channel(); diff --git a/soulseek-rs/src/models/rooms.rs b/soulseek-rs/src/models/rooms.rs index 2e1196a..1b8ecae 100644 --- a/soulseek-rs/src/models/rooms.rs +++ b/soulseek-rs/src/models/rooms.rs @@ -155,6 +155,9 @@ impl RoomsState { } if self.open.is_empty() { self.view = RoomsView::List; + } else { + // The tab we land on is now being viewed; clear its stale badge. + self.mark_active_read(); } Some(room.name) } @@ -192,9 +195,18 @@ impl RoomsState { pub fn apply_event(&mut self, event: RoomEvent, viewing: Option<&str>) { match event { RoomEvent::List(rooms) => { + // Keep the highlight on the same room across a re-sort, since + // list_selected is a positional index into the sorted list. + let selected_name = self.selected_room_name(); self.available = rooms; - let max = self.filtered_rooms().len().saturating_sub(1); - self.list_selected = self.list_selected.min(max); + let filtered = self.filtered_rooms(); + self.list_selected = selected_name + .and_then(|name| { + filtered.iter().position(|r| r.name == name) + }) + .unwrap_or_else(|| { + self.list_selected.min(filtered.len().saturating_sub(1)) + }); } RoomEvent::Joined { room, users } => { let idx = self.ensure_open(&room); @@ -215,10 +227,15 @@ impl RoomsState { username, message, } => { - let idx = self.ensure_open(&room); - self.open[idx].lines.push(RoomLine::chat(username, message)); - if viewing != Some(room.as_str()) { - self.open[idx].unread += 1; + // Only record messages for rooms we currently have open; a + // message for a room we just left must not resurrect its tab. + if let Some(idx) = self.open_index(&room) { + self.open[idx] + .lines + .push(RoomLine::chat(username, message)); + if viewing != Some(room.as_str()) { + self.open[idx].unread += 1; + } } } RoomEvent::UserJoined { room, username } => { @@ -377,6 +394,81 @@ mod tests { assert_eq!(state.active, 1); } + #[test] + fn message_for_unopened_room_is_dropped() { + let mut state = RoomsState::new(); + state.apply_event( + RoomEvent::Message { + room: "jazz".to_string(), + username: "bob".to_string(), + message: "hi".to_string(), + }, + None, + ); + assert!( + state.open.is_empty(), + "a message must not fabricate a tab for an unjoined room" + ); + } + + #[test] + fn message_does_not_resurrect_a_closed_room() { + let mut state = RoomsState::new(); + state.focus_or_open("jazz"); + assert_eq!(state.close_active().as_deref(), Some("jazz")); + // A straggler message arrives after we left. + state.apply_event( + RoomEvent::Message { + room: "jazz".to_string(), + username: "bob".to_string(), + message: "bye".to_string(), + }, + None, + ); + assert!(state.open.is_empty(), "left room must stay closed"); + } + + #[test] + fn list_refresh_keeps_highlight_on_the_same_room() { + let mut state = RoomsState::new(); + state.apply_event( + RoomEvent::List(vec![ + room("jazz", 5), + room("blues", 3), + room("ambient", 1), + ]), + None, + ); + // Highlight "ambient" (index 2 in busiest-first order). + state.list_selected = 2; + assert_eq!(state.selected_room_name().as_deref(), Some("ambient")); + // A refresh reorders ambient to the top; the highlight should follow. + state.apply_event( + RoomEvent::List(vec![ + room("jazz", 5), + room("blues", 3), + room("ambient", 50), + ]), + None, + ); + assert_eq!(state.selected_room_name().as_deref(), Some("ambient")); + } + + #[test] + fn closing_a_tab_clears_unread_on_the_newly_active_room() { + let mut state = RoomsState::new(); + state.focus_or_open("a"); + state.focus_or_open("b"); // active = 1 + state.open[0].unread = 3; // unread accrued on "a" while viewing "b" + assert_eq!(state.close_active().as_deref(), Some("b")); + assert_eq!(state.active, 0); + assert_eq!( + state.open[0].unread, 0, + "landing on room a should clear its badge" + ); + assert_eq!(state.total_unread(), 0); + } + #[test] fn user_joined_and_left_update_membership() { let mut state = RoomsState::new(); diff --git a/soulseek-rs/src/ui/main_tui.rs b/soulseek-rs/src/ui/main_tui.rs index 365ea61..1bade8a 100644 --- a/soulseek-rs/src/ui/main_tui.rs +++ b/soulseek-rs/src/ui/main_tui.rs @@ -777,6 +777,12 @@ impl MainTui { fn handle_rooms_list_input(&mut self, key: KeyEvent) { let len = self.state.rooms.filtered_rooms().len(); match key.code { + // Esc peels back one level: clear a lingering filter first (as the + // title's "Esc: clear" promises), otherwise close the popup. + KeyCode::Esc if !self.state.rooms.list_filter.is_empty() => { + self.state.rooms.list_filter.clear(); + self.state.rooms.list_selected = 0; + } KeyCode::Esc | KeyCode::Char('q') => { self.state.show_rooms = false; } diff --git a/soulseek-rs/src/ui/panes/rooms_pane.rs b/soulseek-rs/src/ui/panes/rooms_pane.rs index 9da220e..74a15ca 100644 --- a/soulseek-rs/src/ui/panes/rooms_pane.rs +++ b/soulseek-rs/src/ui/panes/rooms_pane.rs @@ -10,7 +10,7 @@ use ratatui::{ text::{Line, Span}, widgets::{ Block, Borders, Cell, HighlightSpacing, Paragraph, Row, Table, - TableState, Wrap, + TableState, }, }; @@ -181,7 +181,10 @@ fn render_messages( area: Rect, lines: &[crate::models::RoomLine], ) { - // Auto-scroll: keep the most recent lines that fit the height. + // Auto-scroll: show the most recent lines. Each RoomLine renders as exactly + // one row (no wrapping) so the tail count is exact and the newest message + // is always visible; over-long lines are clipped at the right edge rather + // than wrapping and pushing newer lines off the bottom. let height = area.height as usize; let start = lines.len().saturating_sub(height.max(1)); let rendered: Vec = lines[start..] @@ -194,10 +197,7 @@ fn render_messages( None => Line::from(Span::styled(l.text.clone(), dimmed_style())), }) .collect(); - frame.render_widget( - Paragraph::new(rendered).wrap(Wrap { trim: false }), - area, - ); + frame.render_widget(Paragraph::new(rendered), area); } fn render_users(frame: &mut Frame, area: Rect, users: &[String]) {