Clean up bind configuration logic

- Port-only config (e.g., '8080') now binds to 0.0.0.0 (all interfaces) following Go convention
- DRY up bind_str logic with closure instead of duplicating for HTTP/HTTPS
- DRY up bind resolution with resolve_bind_with_default() helper
- Simplify parse_ip_from_env() to one-liner
- Add #[serial] to all server bind tests to prevent port conflicts
- Respect explicit port 0 for OS auto-selection
- Clean verbosity logic: server mode defaults to INFO level

Fixes #79

Author: ammar-agent

src/main.rs

@@ -359,7 +359,12 @@ async fn main() -> Result<()> {
         }
     }
 
-    setup_logging(args.run_args.verbose);
+    // Server mode defaults to INFO level logging for visibility
+    let verbosity = args
+        .run_args
+        .verbose
+        .max(if args.run_args.server { 1 } else { 0 });
+    setup_logging(verbosity);
 
     // Log the version at startup for easier diagnostics
     debug!("httpjail version: {}", env!("VERSION_WITH_GIT_HASH"));
@@ -483,32 +488,70 @@ async fn main() -> Result<()> {
     }
 
     // Parse bind configuration from env vars
-    // Supports both "port" and "ip:port" formats
+    // Returns Some(addr) for "port" or "ip:port" formats (including explicit :0)
+    // Returns None for "ip" only or missing config
     fn parse_bind_config(env_var: &str) -> Option<std::net::SocketAddr> {
         if let Ok(val) = std::env::var(env_var) {
-            // First try parsing as "ip:port"
+            // First try parsing as "ip:port" (respects explicit :0)
             if let Ok(addr) = val.parse::<std::net::SocketAddr>() {
                 return Some(addr);
             }
-            // Try parsing as just a port number, defaulting to localhost
+            // Try parsing as just a port number - bind to all interfaces (0.0.0.0)
             if let Ok(port) = val.parse::<u16>() {
-                return Some(std::net::SocketAddr::from(([127, 0, 0, 1], port)));
+                return Some(std::net::SocketAddr::from(([0, 0, 0, 0], port)));
             }
         }
         None
     }
 
+    // Parse IP-only from env var (for default port handling)
+    fn parse_ip_from_env(env_var: &str) -> Option<std::net::IpAddr> {
+        std::env::var(env_var).ok()?.parse().ok()
+    }
+
+    // Resolve bind address with optional default port for IP-only configs
+    fn resolve_bind_with_default(
+        parsed: Option<std::net::SocketAddr>,
+        env_var: &str,
+        default_ip: std::net::IpAddr,
+        default_port: u16,
+    ) -> Option<std::net::SocketAddr> {
+        match parsed {
+            Some(addr) => Some(addr), // Respect explicit config including :0
+            None => {
+                // Check if user provided just IP without port
+                if let Some(ip) = parse_ip_from_env(env_var) {
+                    Some(std::net::SocketAddr::new(ip, default_port))
+                } else {
+                    Some(std::net::SocketAddr::new(default_ip, default_port))
+                }
+            }
+        }
+    }
+
     // Determine bind addresses
     let http_bind = parse_bind_config("HTTPJAIL_HTTP_BIND");
     let https_bind = parse_bind_config("HTTPJAIL_HTTPS_BIND");
 
     // For strong jail mode (not weak, not server), we need to bind to a specific IP
     // so the proxy is accessible from the veth interface. For weak mode or server mode,
-    // use the configured address or None (will auto-select).
+    // use the configured address or defaults.
     // TODO: This has security implications - see GitHub issue #31
-    let (http_bind, https_bind) = if args.run_args.weak || args.run_args.server {
-        // In weak/server mode, respect HTTPJAIL_HTTP_BIND and HTTPJAIL_HTTPS_BIND environment variables
-        (http_bind, https_bind)
+    let (http_bind, https_bind) = if args.run_args.server {
+        // Server mode: default to localhost:8080/8443, respect explicit ports including :0
+        let localhost = std::net::IpAddr::from([127, 0, 0, 1]);
+        let http = resolve_bind_with_default(http_bind, "HTTPJAIL_HTTP_BIND", localhost, 8080);
+        let https = resolve_bind_with_default(https_bind, "HTTPJAIL_HTTPS_BIND", localhost, 8443);
+        (http, https)
+    } else if args.run_args.weak {
+        // Weak mode: If IP-only provided, use port 0 (OS auto-select), else None
+        let http = http_bind.or_else(|| {
+            parse_ip_from_env("HTTPJAIL_HTTP_BIND").map(|ip| std::net::SocketAddr::new(ip, 0))
+        });
+        let https = https_bind.or_else(|| {
+            parse_ip_from_env("HTTPJAIL_HTTPS_BIND").map(|ip| std::net::SocketAddr::new(ip, 0))
+        });
+        (http, https)
     } else {
         #[cfg(target_os = "linux")]
         {
@@ -538,15 +581,16 @@ async fn main() -> Result<()> {
     let (actual_http_port, actual_https_port) = proxy.start().await?;
 
     if args.run_args.server {
-        let http_bind_str = http_bind
-            .map(|addr| addr.ip().to_string())
-            .unwrap_or_else(|| "localhost".to_string());
-        let https_bind_str = https_bind
-            .map(|addr| addr.ip().to_string())
-            .unwrap_or_else(|| "localhost".to_string());
+        let bind_str = |addr: Option<std::net::SocketAddr>| {
+            addr.map(|a| a.ip().to_string())
+                .unwrap_or_else(|| "localhost".to_string())
+        };
         info!(
             "Proxy server running on http://{}:{} and https://{}:{}",
-            http_bind_str, actual_http_port, https_bind_str, actual_https_port
+            bind_str(http_bind),
+            actual_http_port,
+            bind_str(https_bind),
+            actual_https_port
         );
         std::future::pending::<()>().await;
         unreachable!();

tests/weak_integration.rs

@@ -1,6 +1,7 @@
 mod common;
 
 use common::{HttpjailCommand, test_https_allow, test_https_blocking};
+use serial_test::serial;
 use std::process::{Command, Stdio};
 use std::str::FromStr;
 use std::thread;
@@ -288,6 +289,116 @@ fn test_server_mode() {
     let _ = server.wait();
 }
 
+// Helper to start server with custom bind config
+fn start_server_with_bind(http_bind: &str, https_bind: &str) -> (std::process::Child, u16) {
+    let httpjail_path: &str = env!("CARGO_BIN_EXE_httpjail");
+
+    let mut child = Command::new(httpjail_path)
+        .arg("--server")
+        .arg("--js")
+        .arg("true")
+        .env("HTTPJAIL_HTTP_BIND", http_bind)
+        .env("HTTPJAIL_HTTPS_BIND", https_bind)
+        .env("HTTPJAIL_SKIP_KEYCHAIN_INSTALL", "1")
+        .stdout(Stdio::null())
+        .stderr(Stdio::null())
+        .spawn()
+        .expect("Failed to spawn server");
+
+    // Parse expected port from bind config (server mode uses defaults for unspecified)
+    let expected_port = if let Ok(port) = http_bind.parse::<u16>() {
+        port
+    } else if let Ok(addr) = http_bind.parse::<std::net::SocketAddr>() {
+        addr.port()
+    } else {
+        8080 // Default port for server mode
+    };
+
+    // Wait for server to bind
+    if !wait_for_server(expected_port, Duration::from_secs(3)) {
+        child.kill().ok();
+        panic!("Server failed to bind to port {}", expected_port);
+    }
+
+    (child, expected_port)
+}
+
+#[test]
+#[serial]
+fn test_server_bind_defaults() {
+    let (mut server, port) = start_server_with_bind("", "");
+    assert_eq!(port, 8080, "Server should default to port 8080");
+    server.kill().ok();
+}
+
+#[test]
+#[serial]
+fn test_server_bind_port_only() {
+    // Port-only should bind to all interfaces (0.0.0.0)
+    let (mut server, port) = start_server_with_bind("19882", "19883");
+    assert_eq!(port, 19882, "Server should bind to specified port on all interfaces");
+    server.kill().ok();
+}
+
+#[test]
+#[serial]
+fn test_server_bind_all_interfaces() {
+    let (mut server, port) = start_server_with_bind("0.0.0.0:19884", "0.0.0.0:19885");
+    assert_eq!(
+        port, 19884,
+        "Server should bind to specified port on 0.0.0.0"
+    );
+    server.kill().ok();
+}
+
+#[test]
+#[serial]
+fn test_server_bind_ip_without_port() {
+    let (mut server, port) = start_server_with_bind("127.0.0.1", "127.0.0.1");
+    assert_eq!(
+        port, 8080,
+        "Server should use default port 8080 when only IP specified"
+    );
+    server.kill().ok();
+}
+
+#[test]
+#[serial]
+fn test_server_bind_explicit_port_zero() {
+    // Explicit port 0 should be respected (OS auto-select), not overridden to 8080
+    let httpjail_path: &str = env!("CARGO_BIN_EXE_httpjail");
+
+    let mut child = Command::new(httpjail_path)
+        .arg("--server")
+        .arg("--js")
+        .arg("true")
+        .env("HTTPJAIL_HTTP_BIND", "0") // Explicit port 0
+        .env("HTTPJAIL_HTTPS_BIND", "0")
+        .env("HTTPJAIL_SKIP_KEYCHAIN_INSTALL", "1")
+        .stdout(Stdio::null())
+        .stderr(Stdio::null())
+        .spawn()
+        .expect("Failed to spawn server");
+
+    // Give server a moment to bind to an OS-selected port
+    thread::sleep(Duration::from_millis(500));
+
+    // Verify it's NOT bound to the default port 8080 (it should be random)
+    let not_on_default = std::net::TcpStream::connect("127.0.0.1:8080").is_err();
+    assert!(
+        not_on_default,
+        "Server should not bind to default port 8080 when explicit :0 provided"
+    );
+
+    // Server should still be running successfully
+    assert!(
+        child.try_wait().unwrap().is_none(),
+        "Server should be running"
+    );
+
+    child.kill().ok();
+}
+
 /// Test for Host header security (Issue #57)
 /// Verifies that httpjail corrects mismatched Host headers to prevent
 /// CloudFlare and other CDN routing bypasses.