quality: PR5 — perf, quality, and dead-code cleanups

crates/tuic-client/src/error.rs

@@ -4,6 +4,11 @@ use quinn::{ConnectError, ConnectionError};
 use rustls::Error as RustlsError;
 use thiserror::Error;
 
+// NOTE: `Timeout`, `InvalidSocks5Auth`, `Socks5` are currently unconstructed in
+// the workspace. `WrongPacketSource` IS constructed (PR1 wired it into the
+// UDP-associate first-packet check). Keeping the rest as `pub` API for future
+// call sites rather than removing — they encode legitimate, named failure
+// modes the client may want to surface.
 #[derive(Debug, Error)]
 pub enum Error {
 	#[error(transparent)]

crates/tuic-client/src/forward.rs

@@ -8,12 +8,8 @@ use std::{
 };
 
 use bytes::Bytes;
-use once_cell::sync::OnceCell;
 use socket2::{Domain, Protocol, SockAddr, Socket, Type};
-use tokio::{
-	net::{TcpListener, UdpSocket},
-	sync::RwLock as AsyncRwLock,
-};
+use tokio::net::{TcpListener, UdpSocket};
 use tracing::{Instrument, debug, info, warn};
 use wind_core::{
 	AbstractOutbound,
@@ -27,48 +23,20 @@ use crate::{
 	wind_adapter,
 };
 
-// Global UDP forward session registry
-pub static UDP_SESSIONS: OnceCell<AsyncRwLock<HashMap<u16, ForwardUdpSession>>> = OnceCell::new();
 static NEXT_ASSOC_ID: AtomicU16 = AtomicU16::new(0);
 
 fn next_assoc_id() -> u16 {
 	// Use high bit set to avoid collision with SOCKS5-generated assoc IDs
 	0x8000 | (NEXT_ASSOC_ID.fetch_add(1, Ordering::Relaxed) & 0x7fff)
 }
 
-#[derive(Clone)]
-pub struct ForwardUdpSession {
-	socket: Arc<UdpSocket>,
-	src_addr: SocketAddr,
-	assoc_id: u16,
-}
-
-impl ForwardUdpSession {
-	pub fn new(socket: Arc<UdpSocket>, src_addr: SocketAddr, assoc_id: u16) -> Self {
-		Self {
-			socket,
-			src_addr,
-			assoc_id,
-		}
-	}
-
-	pub async fn send(&self, pkt: Bytes) -> Result<(), Error> {
-		if let Err(err) = self.socket.send_to(&pkt, self.src_addr).await {
-			warn!(
-				"[forward-udp] [{assoc:#06x}] failed sending packet to {dst}: {err}",
-				assoc = self.assoc_id,
-				dst = self.src_addr,
-			);
-			return Err(Error::Io(err));
-		}
-		Ok(())
-	}
-}
+// NOTE: a global `UDP_SESSIONS: HashMap<assoc_id, ForwardUdpSession>` once
+// existed here; it was being written (insert / remove) by this file but
+// never read by anyone — pure dead code that just took locks on the UDP
+// hot path. The local `sessions: HashMap<SocketAddr, UdpForwardSession>`
+// in `run_udp_forwarder` is the only routing table in use.
 
 pub async fn start(tcp: Vec<TcpForward>, udp: Vec<UdpForward>) {
-	// Init UDP session map
-	let _ = UDP_SESSIONS.set(AsyncRwLock::new(HashMap::new()));
-
 	for entry in tcp {
 		tokio::spawn(run_tcp_forwarder(entry));
 	}
@@ -85,7 +53,8 @@ async fn run_tcp_forwarder(entry: TcpForward) {
 			return;
 		}
 	};
-	warn!(
+	// Normal startup info — `warn!` here was startling on every launch.
+	info!(
 		"[forward-tcp] listening on {listen} -> {remote:?}",
 		listen = listener.local_addr().unwrap(),
 		remote = entry.remote
@@ -161,7 +130,8 @@ async fn run_udp_forwarder(entry: UdpForward) {
 		}
 	};
 	let socket = Arc::new(socket);
-	warn!(
+	// Normal startup info — `warn!` here was startling on every launch.
+	info!(
 		"[forward-udp] listening on {listen} -> {remote:?} timeout={timeout:?}",
 		listen = entry.listen,
 		remote = entry.remote,
@@ -196,14 +166,6 @@ async fn run_udp_forwarder(entry: UdpForward) {
 						let (tx_to_local, mut rx_from_out) = tokio::sync::mpsc::channel::<UdpPacket>(64);
 						let udp_stream = UdpStream { tx: tx_to_local, rx: rx_from_local };
 
-						UDP_SESSIONS
-							.get()
-							.map(|s| s.try_write().map(|mut w| {
-								w.insert(assoc_id, ForwardUdpSession::new(socket.clone(), src_addr, assoc_id));
-							}))
-							.and_then(|res| res.ok())
-							.unwrap_or(());
-
 						// Reply bridge: take packets coming back from the outbound
 						// and write them to the original src_addr.
 						tokio::spawn(async move {
@@ -253,9 +215,6 @@ async fn run_udp_forwarder(entry: UdpForward) {
 						Err(tokio::sync::mpsc::error::TrySendError::Closed(_)) => {
 							debug!("[forward-udp] [{assoc_id:#06x}] outbound closed; removing session for {src_addr}");
 							sessions.remove(&src_addr);
-							if let Some(reg) = UDP_SESSIONS.get() {
-								let _ = reg.try_write().map(|mut w| w.remove(&assoc_id));
-							}
 						}
 					}
 				}
@@ -272,9 +231,6 @@ async fn run_udp_forwarder(entry: UdpForward) {
 							"[forward-udp] [{assoc:#06x}] idle timeout; dropping session for {src_addr}",
 							assoc = s.assoc_id
 						);
-						if let Some(reg) = UDP_SESSIONS.get() {
-							let _ = reg.try_write().map(|mut w| w.remove(&s.assoc_id));
-						}
 						false
 					} else {
 						true

crates/tuic-client/src/wind_adapter.rs

@@ -36,10 +36,40 @@ impl TuicOutboundAdapter {
 		// Convert password to Arc<[u8]>
 		let password: Arc<[u8]> = relay.password.clone();
 
+		// Pick the SNI to send during TLS handshake.
+		//
+		// Defaulting to `relay.server.0` is sensible when the server field is
+		// a hostname, but if it's an IP literal we end up announcing the IP
+		// in the SNI extension — most rustls/webpki verifiers reject that as
+		// a non-hostname SNI, and even when they don't, the SNI value carries
+		// no integrity benefit. Warn loudly so operators notice the
+		// wrong configuration; require an explicit `sni` for IP-literal
+		// servers and use a placeholder otherwise so the connection still
+		// attempts to handshake (and rustls will surface the bad-SNI error
+		// in its own message).
+		let sni = match relay.sni.clone() {
+			Some(s) => s,
+			None => {
+				if relay.server.0.parse::<std::net::IpAddr>().is_ok() {
+					tracing::warn!(
+						"relay server `{}` is an IP literal but no `sni` was configured; TLS verification will likely fail. \
+						 Set `sni = \"<hostname>\"` in the relay config to fix.",
+						relay.server.0,
+					);
+					// rustls accepts "invalid" only if the verifier accepts it
+					// — for the skip-verify path this is harmless; for the
+					// real path the connection will fail with a clear error.
+					"invalid.sni.placeholder".to_string()
+				} else {
+					relay.server.0.clone()
+				}
+			}
+		};
+
 		// Create wind-tuic outbound options
 		let opts = TuicOutboundOpts {
 			peer_addr: server_addr,
-			sni: relay.sni.unwrap_or_else(|| relay.server.0.clone()),
+			sni,
 			auth: (relay.uuid, password),
 			zero_rtt_handshake: relay.zero_rtt_handshake,
 			heartbeat: relay.heartbeat,

crates/tuic-server/src/acl.rs

@@ -27,15 +27,28 @@ pub struct AclRule {
 	pub hijack: Option<String>,
 }
 
-fn format_optional_parts(ports: &Option<AclPorts>, hijack: &Option<String>) -> String {
-	let mut result = String::new();
-	if let Some(p) = ports {
-		result.push_str(&format!(" {}", p));
-	}
-	if let Some(h) = hijack {
-		result.push_str(&format!(" {}", h));
+// Used by the derive(Display) macro on `AclRule`. The macro expands to a
+// `write!` call, so anything implementing `Display` works — return type was
+// `String` purely for the previous `format!`-based implementation.
+struct OptionalParts<'a> {
+	ports: &'a Option<AclPorts>,
+	hijack: &'a Option<String>,
+}
+
+impl<'a> std::fmt::Display for OptionalParts<'a> {
+	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+		if let Some(p) = self.ports {
+			write!(f, " {p}")?;
+		}
+		if let Some(h) = self.hijack {
+			write!(f, " {h}")?;
+		}
+		Ok(())
 	}
-	result
+}
+
+fn format_optional_parts<'a>(ports: &'a Option<AclPorts>, hijack: &'a Option<String>) -> OptionalParts<'a> {
+	OptionalParts { ports, hijack }
 }
 
 /// Represents different types of addresses in ACL rules
@@ -86,10 +99,14 @@ pub struct AclPortEntry {
 	pub port_spec: AclPortSpec,
 }
 
-fn format_protocol(protocol: &Option<AclProtocol>) -> String {
+fn format_protocol(protocol: &Option<AclProtocol>) -> &'static str {
+	// Allocation-free: each combination maps to a fixed string literal. The
+	// derive(Display) macro just needs `Display`, which `&str` already
+	// implements, so we can return the borrowed literal directly.
 	match protocol {
-		Some(p) => format!("{}/", p),
-		None => String::new(),
+		Some(AclProtocol::Tcp) => "tcp/",
+		Some(AclProtocol::Udp) => "udp/",
+		None => "",
 	}
 }
 
@@ -2171,4 +2188,27 @@ addr = "private"
 		let rules = acl_to_rules(std::slice::from_ref(&acl));
 		assert!(rules.is_empty(), "malformed IP must drop the rule");
 	}
+
+	// ------------------------------------------------------------------
+	// PR5-K regression: `format_protocol` / `format_optional_parts` Display
+	// output must still match the parser-accepted spelling.
+	// ------------------------------------------------------------------
+
+	#[test]
+	fn pr5_format_protocol_zero_alloc_output() {
+		assert_eq!(super::format_protocol(&Some(super::AclProtocol::Tcp)), "tcp/");
+		assert_eq!(super::format_protocol(&Some(super::AclProtocol::Udp)), "udp/");
+		assert_eq!(super::format_protocol(&None), "");
+	}
+
+	#[test]
+	fn pr5_format_optional_parts_display() {
+		// Empty case — no ports + no hijack ⇒ empty Display.
+		let s = super::format_optional_parts(&None, &None).to_string();
+		assert_eq!(s, "");
+
+		// Hijack only.
+		let s = super::format_optional_parts(&None, &Some("10.0.0.1".into())).to_string();
+		assert_eq!(s, " 10.0.0.1");
+	}
 }

crates/tuic-server/src/config.rs

@@ -1671,8 +1671,11 @@ mod tests {
 		// Note: This test doesn't actually set env vars, just tests the structure
 		let env_state = EnvState::from_system();
 
-		// Should not panic and return a valid EnvState
-		assert!(env_state.tuic_config_format.is_none() || env_state.tuic_config_format.is_some());
+		// `from_system` must not panic. There is no value we can usefully
+		// assert about `tuic_config_format` here without setting up env
+		// vars first, so just keep `env_state` alive to confirm it
+		// constructs.
+		let _ = env_state;
 	}
 
 	#[tokio::test]

crates/tuic-server/src/error.rs

@@ -5,6 +5,10 @@ use rustls::Error as RustlsError;
 use thiserror::Error;
 use uuid::Uuid;
 
+// NOTE: many variants are currently unconstructed inside the workspace but are
+// `pub` API — downstream binaries / future call sites may construct them, so
+// we keep them all. If they remain unconstructed for several releases, prune
+// then.
 #[derive(Debug, Error)]
 pub enum Error {
 	#[error(transparent)]

crates/tuic-server/src/tls.rs

@@ -235,7 +235,6 @@ mod tests {
 			SanType::IpAddress("127.0.0.1".parse()?),
 		];
 		let key_pair = KeyPair::generate()?;
-		key_pair.serialize_der();
 
 		let cert = params.self_signed(&key_pair)?;