Skip to content

Commit 2246f68

Browse files
KaiCreatesclaude
andcommitted
fix: resolve 20 bugs found by performance audit
Critical fixes: - server.rs: BUG-1 — client_count was never incremented, only decremented; move count management into handle_client_handshake with fetch_add on connect and fetch_sub on disconnect - server.rs: BUG-6 — wrap every handshake with 10s tokio::time::timeout to prevent stalled/malicious connections from leaking tasks indefinitely - server.rs: BUG-8 — UDP broadcast task was not cancelled on server stop; pass CancellationToken and use select! so the UdpSocket is released before the next server start (fixes "address already in use" on UDP rebind) - main.rs: BUG-14 — handle.shutdown() was called without .await; the future was dropped immediately, leaving the TcpListener bound across restarts - main.rs: BUG-2 — _status_rx was immediately dropped, silently discarding all disconnect/error events from the client UDP task; spawn forwarding task - session.rs: BUG-5 — replace rand::thread_rng() with OsRng for session code generation; session codes are security credentials High fixes: - server.rs: BUG-7 — Mutex was held across all async UDP sends; clone client data under lock then release before sending - server.rs: BUG-16 — add MAX_CLIENTS=10 check before accepting new sessions - client.rs: BUG-9 — TCP port was hardcoded to 24800 ignoring config; pass control_port parameter through connect_to_server - client.rs: BUG-10 — UDP port was hardcoded to CLIENT_UDP_PORT constant; bind to OS-assigned port (0.0.0.0:0) and report actual port to server - client.rs: BUG-12 — TLS fallback ServerName "InputSync" panicked for IP addresses; detect IP with parse::<IpAddr>() and use ServerName::IpAddress - client.rs: BUG-21 — unbounded std::mpsc::channel for simulator could grow without bound; switch to sync_channel(512) with try_send - capture.rs: BUG-11 — seq counter was incremented before checking whether a packet would be emitted; move increment inside the Some(pkt) branch Medium fixes: - capture.rs: SMELL-3 — first_move not reset when forwarding re-enabled; track was_forwarding and set first_move=true on forwarding transitions - app.rs: BUG-15 — ctx.set_visuals() was called every frame; move once to InputSyncApp::new via cc.egui_ctx - logs_panel.rs: BUG-19 — remove non-functional Clear/Export buttons that egui_logger 0.6 does not support - settings_panel.rs: BUG-18 — dead zone w_frac/h_frac could exceed screen bounds; clamp to (1.0 - x_frac) / (1.0 - y_frac) in the DragValue range - main.rs: BUG-20 — server_addr in ClientState hardcoded port 24800 instead of config.control_port Note: BUG-13 (AltGr distinction) deferred — enigo 0.2 does not expose Key::AltGr; requires dependency upgrade tracked for v1.2. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 1df1ca6 commit 2246f68

9 files changed

Lines changed: 162 additions & 68 deletions

File tree

src-tauri/src/app.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,15 @@ pub struct InputSyncApp {
2525

2626
impl InputSyncApp {
2727
pub fn new(
28-
_cc: &eframe::CreationContext<'_>,
28+
cc: &eframe::CreationContext<'_>,
2929
cmd_tx: mpsc::UnboundedSender<UiCommand>,
3030
net_rx: mpsc::UnboundedReceiver<NetEvent>,
3131
shared: SharedState,
3232
config: ServerConfig,
3333
data_dir: PathBuf,
3434
) -> Self {
35+
// Set dark theme once at startup instead of every frame.
36+
cc.egui_ctx.set_visuals(egui::Visuals::dark());
3537
Self {
3638
cmd_tx,
3739
net_rx,
@@ -82,9 +84,6 @@ impl eframe::App for InputSyncApp {
8284
};
8385
ctx.request_repaint_after(repaint_delay);
8486

85-
// Apply dark theme
86-
ctx.set_visuals(egui::Visuals::dark());
87-
8887
egui::CentralPanel::default().show(ctx, |ui| {
8988
crate::ui::show(&mut self.ui, ui, &self.status, &self.cmd_tx, &self.data_dir);
9089
});

src-tauri/src/core/session.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1+
use rand::rngs::OsRng;
12
use rand::Rng;
23

34
const CODE_CHARSET: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
45
const CODE_LENGTH: usize = 6;
56

6-
/// Generate a cryptographically random 6-character alphanumeric session code
7+
/// Generate a cryptographically random 6-character alphanumeric session code.
8+
/// Uses OsRng (OS entropy source) — a security credential must not use thread_rng.
79
pub fn generate_session_code() -> String {
8-
let mut rng = rand::thread_rng();
10+
let mut rng = OsRng;
911
(0..CODE_LENGTH)
1012
.map(|_| {
1113
let idx = rng.gen_range(0..CODE_CHARSET.len());

src-tauri/src/input/capture.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ pub fn start_capture(
150150
let mut last_y: f64 = 0.0;
151151
let mut first_move = true;
152152
let mut local_seq: u32 = 0;
153+
let mut was_forwarding = false;
153154

154155
let callback = move |event: Event| {
155156
if stop_rx.try_recv().is_ok() {
@@ -168,11 +169,19 @@ pub fn start_capture(
168169
check_edge_trigger(*x, *y, &config, screen_size, &forwarding);
169170
}
170171

171-
if !forwarding.load(Ordering::Relaxed) {
172-
return;
172+
let is_forwarding = forwarding.load(Ordering::Relaxed);
173+
174+
// Reset mouse baseline when forwarding is re-enabled to avoid a
175+
// large delta jump on the client from the cursor position change
176+
// that occurred while forwarding was off.
177+
if is_forwarding && !was_forwarding {
178+
first_move = true;
173179
}
180+
was_forwarding = is_forwarding;
174181

175-
local_seq = local_seq.wrapping_add(1);
182+
if !is_forwarding {
183+
return;
184+
}
176185

177186
let packet: Option<InputPacket> = match &event.event_type {
178187
EventType::MouseMove { x, y } => {
@@ -233,7 +242,11 @@ pub fn start_capture(
233242
}
234243
};
235244

245+
// Only advance seq when a packet is actually emitted.
246+
// Advancing it for dropped events (first_move, zero delta, zero wheel)
247+
// would create spurious counter gaps on the receiver side.
236248
if let Some(pkt) = packet {
249+
local_seq = local_seq.wrapping_add(1);
237250
if event_tx.try_send(pkt).is_err() {
238251
tracing::trace!("Input queue full — packet dropped");
239252
}

src-tauri/src/input/simulation.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ fn keycode_to_enigo(kc: KeyCode) -> Option<Key> {
123123
F7 => Some(Key::F7), F8 => Some(Key::F8), F9 => Some(Key::F9),
124124
F10 => Some(Key::F10), F11 => Some(Key::F11), F12 => Some(Key::F12),
125125
// Modifiers
126+
// Note: enigo 0.2 does not expose Key::AltGr/RControl/RShift separately,
127+
// so left and right variants map to the same logical key. AltGr distinction
128+
// requires upgrading to a newer enigo release.
126129
LeftCtrl | RightCtrl => Some(Key::Control),
127130
LeftShift | RightShift => Some(Key::Shift),
128131
LeftAlt | RightAlt => Some(Key::Alt),

src-tauri/src/main.rs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ async fn async_main(
164164
Err(e) => {
165165
let _ = net_tx
166166
.send(NetEvent::Error(format!("Capture error: {}", e)));
167-
handle.shutdown();
167+
handle.shutdown().await;
168168
continue;
169169
}
170170
};
@@ -212,15 +212,16 @@ async fn async_main(
212212
}
213213

214214
let ssl_enabled = locked.config.ssl_enabled;
215+
let control_port = locked.config.control_port;
215216
let tls_connector = if ssl_enabled {
216217
Some(crate::network::tls::make_tls_connector())
217218
} else {
218219
None
219220
};
220221

221-
let (status_tx, _status_rx) = mpsc::unbounded_channel::<String>();
222+
let (status_tx, mut status_rx) = mpsc::unbounded_channel::<String>();
222223
let handle =
223-
match connect_to_server(&ip, &code, status_tx, tls_connector).await {
224+
match connect_to_server(&ip, &code, control_port, status_tx, tls_connector).await {
224225
Ok(h) => h,
225226
Err(e) => {
226227
let _ = net_tx
@@ -231,11 +232,24 @@ async fn async_main(
231232

232233
locked.client = Some(ClientState {
233234
handle,
234-
server_addr: format!("{}:24800", ip),
235+
server_addr: format!("{}:{}", ip, control_port),
235236
latency_ms: None,
236237
last_error: None,
237238
});
238239

240+
// Forward disconnect events from the client UDP task to the UI.
241+
// Without this the receiver is dropped and all status sends silently fail.
242+
let net_tx_status = net_tx.clone();
243+
let shared_status = shared.clone();
244+
tokio::spawn(async move {
245+
while let Some(msg) = status_rx.recv().await {
246+
if msg.starts_with("disconnected") || msg.starts_with("simulator_error") {
247+
shared_status.lock().await.client = None;
248+
let _ = net_tx_status.send(NetEvent::Disconnected);
249+
}
250+
}
251+
});
252+
239253
tracing::info!("Connected to {}", ip);
240254
let _ = net_tx.send(NetEvent::Connected);
241255
let _ = net_tx.send(NetEvent::StatusUpdate(locked.status()));

src-tauri/src/network/client.rs

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@ use crate::core::protocol::{
1212
};
1313
use crate::input::simulation::InputSimulator;
1414

15-
pub const CLIENT_UDP_PORT: u16 = 24802;
1615
const COUNTER_WINDOW: u64 = 64;
16+
/// Bounded simulator channel capacity — drops old packets when the
17+
/// simulator thread falls behind, preventing unbounded heap growth.
18+
const SIM_CHANNEL_CAP: usize = 512;
1719

1820
pub struct ClientHandle {
1921
shutdown_tx: oneshot::Sender<()>,
@@ -28,16 +30,28 @@ impl ClientHandle {
2830
pub async fn connect_to_server(
2931
server_host: &str,
3032
session_code: &str,
33+
control_port: u16,
3134
status_tx: mpsc::UnboundedSender<String>,
3235
tls_connector: Option<tokio_rustls::TlsConnector>,
3336
) -> Result<ClientHandle> {
34-
let server_tcp_addr = format!("{}:24800", server_host);
37+
let server_tcp_addr = format!("{}:{}", server_host, control_port);
3538
let tcp_stream = TcpStream::connect(&server_tcp_addr).await?;
3639
tracing::info!("TCP connected to {}", server_tcp_addr);
3740

3841
if let Some(connector) = tls_connector {
39-
let server_name = rustls::pki_types::ServerName::try_from(server_host.to_string())
40-
.unwrap_or_else(|_| rustls::pki_types::ServerName::try_from("InputSync").unwrap());
42+
// Handle both IP addresses and DNS hostnames.
43+
// AcceptAnyCert ignores the server name for validation, but rustls
44+
// still requires a syntactically valid ServerName.
45+
let server_name = server_host
46+
.parse::<std::net::IpAddr>()
47+
.map(|ip| rustls::pki_types::ServerName::IpAddress(ip.into()))
48+
.unwrap_or_else(|_| {
49+
rustls::pki_types::ServerName::try_from(server_host.to_string())
50+
.unwrap_or_else(|_| {
51+
rustls::pki_types::ServerName::try_from("inputsync.local")
52+
.expect("inputsync.local is a valid DNS name")
53+
})
54+
});
4155
let tls_stream = connector.connect(server_name, tcp_stream).await?;
4256
tracing::info!("TLS handshake complete");
4357
do_handshake_and_run(tls_stream, session_code, status_tx).await
@@ -94,8 +108,12 @@ where
94108
stream.write_u8(HS_NONCE_EXCHANGE).await?;
95109
stream.write_all(&client_base_nonce).await?;
96110

97-
// Step 5: Send UDP listen port
98-
stream.write_u16(CLIENT_UDP_PORT).await?;
111+
// Step 5: Bind UDP to OS-assigned port (port 0) to avoid hardcoded port
112+
// conflicts when two clients run on the same machine. Report actual port
113+
// to the server so it knows where to send events.
114+
let udp_socket = Arc::new(UdpSocket::bind("0.0.0.0:0").await?);
115+
let actual_udp_port = udp_socket.local_addr()?.port();
116+
stream.write_u16(actual_udp_port).await?;
99117
stream.flush().await?;
100118

101119
let combined_nonce = combine_nonces(&server_base_nonce, &client_base_nonce);
@@ -106,15 +124,15 @@ where
106124
bail!("Server did not acknowledge: 0x{:02X}", ack);
107125
}
108126

109-
tracing::info!("Handshake complete — session established.");
127+
tracing::info!("Handshake complete — session established. UDP port: {}", actual_udp_port);
110128
let _ = status_tx.send("connected".to_string());
111129

112-
// ── UDP receive socket ───────────────────────────────────────────────
113-
let udp_socket = Arc::new(UdpSocket::bind(format!("0.0.0.0:{}", CLIENT_UDP_PORT)).await?);
114130
let (shutdown_tx, mut shutdown_rx) = oneshot::channel::<()>();
115131

116132
// ── Simulator thread ─────────────────────────────────────────────────
117-
let (sim_tx, sim_rx) = std::sync::mpsc::channel::<Vec<u8>>();
133+
// Bounded sync_channel prevents heap growth when simulator falls behind.
134+
// The async UDP task uses try_send to avoid blocking the executor.
135+
let (sim_tx, sim_rx) = std::sync::mpsc::sync_channel::<Vec<u8>>(SIM_CHANNEL_CAP);
118136
let status_sim = status_tx.clone();
119137
std::thread::Builder::new()
120138
.name("inputsync-simulator".into())
@@ -181,7 +199,10 @@ where
181199
matched - current
182200
);
183201
}
184-
let _ = sim_tx.send(plain);
202+
// try_send: discard packet if simulator is backed up
203+
if sim_tx.try_send(plain).is_err() {
204+
tracing::trace!("Simulator queue full — packet dropped");
205+
}
185206
}
186207
None => {
187208
tracing::warn!(

0 commit comments

Comments
 (0)