From b0a885ea4076f0664bec7638b6ef2d2adbf5df98 Mon Sep 17 00:00:00 2001 From: "glm-5.1" Date: Mon, 8 Jun 2026 05:30:23 +0000 Subject: [PATCH] fix(config): replace panics in parse_proxy_config with proper Result errors parse_proxy_config was using expect()/unwrap()/panic!() which would crash the process on malformed proxy config strings instead of returning a descriptive error. Now returns ConfigError::ProxyConfigInvalid with the specific issue (bad scheme, bad address). Added tests for invalid scheme, invalid address, and end-to-end from_serve_options. --- .../alknet-core/src/config/static_config.rs | 116 ++++++++++++++---- crates/alknet-core/src/error.rs | 9 ++ tasks/cleanup/panic-free-static-config.md | 4 +- 3 files changed, 101 insertions(+), 28 deletions(-) diff --git a/crates/alknet-core/src/config/static_config.rs b/crates/alknet-core/src/config/static_config.rs index 69398b9..55235cf 100644 --- a/crates/alknet-core/src/config/static_config.rs +++ b/crates/alknet-core/src/config/static_config.rs @@ -57,7 +57,7 @@ impl StaticConfig { let dynamic = crate::config::DynamicConfig::new(auth_policy); - let proxy_config = parse_proxy_config(opts.proxy.as_deref()); + let proxy_config = parse_proxy_config(opts.proxy.as_deref())?; let listeners = if let Some(listeners) = opts.listeners { listeners @@ -100,30 +100,40 @@ impl StaticConfig { } } -fn parse_proxy_config(proxy: Option<&str>) -> Option { - proxy.map(|url| { - if url.starts_with("socks5://") { - let addr: SocketAddr = url - .strip_prefix("socks5://") - .unwrap() - .parse() - .expect("invalid socks5 proxy address"); - ProxyConfig { - mode: ProxyMode::Socks5(addr), +fn parse_proxy_config( + proxy: Option<&str>, +) -> Result, crate::error::ConfigError> { + match proxy { + None => Ok(None), + Some(url) => { + if let Some(rest) = url.strip_prefix("socks5://") { + let addr: SocketAddr = rest.parse().map_err(|e| { + crate::error::ConfigError::ProxyConfigInvalid { + message: format!("invalid socks5 proxy address '{}': {}", rest, e), + } + })?; + Ok(Some(ProxyConfig { + mode: ProxyMode::Socks5(addr), + })) + } else if let Some(rest) = url.strip_prefix("http://") { + let addr: SocketAddr = rest.parse().map_err(|e| { + crate::error::ConfigError::ProxyConfigInvalid { + message: format!( + "invalid http connect proxy address '{}': {}", + rest, e + ), + } + })?; + Ok(Some(ProxyConfig { + mode: ProxyMode::HttpConnect(addr), + })) + } else { + Err(crate::error::ConfigError::ProxyConfigInvalid { + message: format!("unsupported proxy URL scheme: {}", url), + }) } - } else if url.starts_with("http://") { - let addr: SocketAddr = url - .strip_prefix("http://") - .unwrap() - .parse() - .expect("invalid http connect proxy address"); - ProxyConfig { - mode: ProxyMode::HttpConnect(addr), - } - } else { - panic!("unsupported proxy URL scheme: {url}"); } - }) + } } #[cfg(test)] @@ -147,7 +157,7 @@ mod tests { #[test] fn parse_proxy_config_socks5() { - let config = parse_proxy_config(Some("socks5://127.0.0.1:9050")); + let config = parse_proxy_config(Some("socks5://127.0.0.1:9050")).unwrap(); assert!(config.is_some()); match config.unwrap().mode { ProxyMode::Socks5(addr) => { @@ -159,7 +169,7 @@ mod tests { #[test] fn parse_proxy_config_http() { - let config = parse_proxy_config(Some("http://127.0.0.1:8080")); + let config = parse_proxy_config(Some("http://127.0.0.1:8080")).unwrap(); assert!(config.is_some()); match config.unwrap().mode { ProxyMode::HttpConnect(addr) => { @@ -171,7 +181,31 @@ mod tests { #[test] fn parse_proxy_config_none() { - assert!(parse_proxy_config(None).is_none()); + assert!(parse_proxy_config(None).unwrap().is_none()); + } + + #[test] + fn parse_proxy_config_invalid_scheme() { + let result = parse_proxy_config(Some("ftp://127.0.0.1:9050")); + assert!(result.is_err()); + match result.unwrap_err() { + crate::error::ConfigError::ProxyConfigInvalid { message } => { + assert!(message.contains("unsupported proxy URL scheme")); + } + e => panic!("expected ProxyConfigInvalid, got {:?}", e), + } + } + + #[test] + fn parse_proxy_config_invalid_address() { + let result = parse_proxy_config(Some("socks5://not-an-address")); + assert!(result.is_err()); + match result.unwrap_err() { + crate::error::ConfigError::ProxyConfigInvalid { message } => { + assert!(message.contains("invalid socks5 proxy address")); + } + e => panic!("expected ProxyConfigInvalid, got {:?}", e), + } } #[test] @@ -206,4 +240,34 @@ mod tests { TransportKind::Tcp ); } + + #[test] + fn static_config_from_serve_options_invalid_proxy_returns_err() { + let opts = ServeOptions::new(make_key_source()) + .authorized_keys(make_authorized_keys_source()) + .proxy("ftp://bad-scheme"); + let result = StaticConfig::from_serve_options(opts); + assert!(result.is_err()); + match result.unwrap_err() { + crate::error::ConfigError::ProxyConfigInvalid { message } => { + assert!(message.contains("unsupported proxy URL scheme")); + } + e => panic!("expected ProxyConfigInvalid, got {:?}", e), + } + } + + #[test] + fn static_config_from_serve_options_malformed_proxy_address_returns_err() { + let opts = ServeOptions::new(make_key_source()) + .authorized_keys(make_authorized_keys_source()) + .proxy("socks5://not-a-valid-addr"); + let result = StaticConfig::from_serve_options(opts); + assert!(result.is_err()); + match result.unwrap_err() { + crate::error::ConfigError::ProxyConfigInvalid { message } => { + assert!(message.contains("invalid socks5 proxy address")); + } + e => panic!("expected ProxyConfigInvalid, got {:?}", e), + } + } } diff --git a/crates/alknet-core/src/error.rs b/crates/alknet-core/src/error.rs index 8c5b04f..0d7471e 100644 --- a/crates/alknet-core/src/error.rs +++ b/crates/alknet-core/src/error.rs @@ -67,6 +67,8 @@ pub enum ConfigError { }, #[error("incompatible options")] IncompatibleOptions, + #[error("invalid proxy config: {message}")] + ProxyConfigInvalid { message: String }, } #[derive(Debug, thiserror::Error)] @@ -173,6 +175,13 @@ mod tests { ConfigError::IncompatibleOptions.to_string(), "incompatible options" ); + assert_eq!( + ConfigError::ProxyConfigInvalid { + message: "bad proxy".to_string() + } + .to_string(), + "invalid proxy config: bad proxy" + ); } #[test] diff --git a/tasks/cleanup/panic-free-static-config.md b/tasks/cleanup/panic-free-static-config.md index 88b746f..3db9ab4 100644 --- a/tasks/cleanup/panic-free-static-config.md +++ b/tasks/cleanup/panic-free-static-config.md @@ -1,7 +1,7 @@ --- id: cleanup/panic-free-static-config name: Replace panic/expect/unwrap with Result-based error handling in StaticConfig -status: pending +status: completed depends_on: - review/phase1-core-modifications scope: narrow @@ -40,4 +40,4 @@ Since `StaticConfig::from_serve_options()` already returns `Result<..., ConfigEr ## Summary -> To be filled on completion \ No newline at end of file +> Replaced all panic!/expect()/unwrap() in parse_proxy_config with Result-based error handling. Added ConfigError::ProxyConfigInvalid variant. Invalid proxy scheme or malformed address now returns clear errors instead of panicking. Added 4 new tests covering invalid scheme, invalid address, and from_serve_options error propagation. \ No newline at end of file