Uh oh!
There was an error while loading. Please reload this page.
Conversation
9e8690a to
1f3357bCompareThere was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // 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(([0, 0, 0, 0], port))); |
There was a problem hiding this comment.
When the bind environment variables contain only a port, the new parser builds a SocketAddr with 0.0.0.0, so HTTPJAIL_HTTP_BIND=8080 now listens on every interface. Previously such configuration bound to loopback, and the commit summary still claims that the port-only format defaults to localhost. This change silently broadens the listener’s exposure and can unintentionally publish the proxy to the whole network. It would be safer to keep the default IP at 127.0.0.1 and only bind to 0.0.0.0 when the user explicitly supplies an address.
Useful? React with 👍 / 👎.
Fixes#79 The server mode was ignoring the IP addresses specified in the HTTPJAIL_HTTP_BIND and HTTPJAIL_HTTPS_BIND environment variables, always binding to localhost instead. This commit refactors the proxy binding logic to use std::net::SocketAddr throughout, which provides a cleaner API that supports both IPv4 and IPv6 while combining IP and port into a single type. Changes: - Refactored ProxyServer struct to use Option<SocketAddr> for http_bind and https_bind instead of separate IP and port fields - Created unified bind_listener() function that handles both IPv4 and IPv6 - Removed redundant bind_ipv4_listener() function - Simplified parse_bind_config() to return Option<SocketAddr> directly - Fixed strong jail mode to properly bind to computed jail IP with port 0 for auto-selection when no port is specified The fix maintains backward compatibility and passes all tests: - 45/45 unit tests pass - 23/23 integration tests pass - Clippy passes with -D warnings Supported formats for bind addresses: - "ip:port" (e.g., "0.0.0.0:8080", "127.0.0.1:8080") - "[ipv6]:port" (e.g., "[::1]:8080") - "port" (defaults to localhost)
- 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
Previously this test passed port-only (e.g., '19876') which now binds to 0.0.0.0 (all interfaces) per Go convention. The test expects localhost binding, so explicitly specify '127.0.0.1:PORT' format.
This commit adds support for the Go-style :port bind syntax (e.g., :80, :8080) which binds to all interfaces (0.0.0.0) on the specified port. Changes: - Updated parse_bind_config() to handle :port format - Added test_server_bind_colon_prefix_port test - Updated start_server_with_bind() helper to parse :port format Now supports all bind formats: - "80" -> 0.0.0.0:80 - ":80" -> 0.0.0.0:80 (new) - "127.0.0.1:80" -> 127.0.0.1:80 - "127.0.0.1" -> 127.0.0.1:8080 (server mode default)
Uh oh!
There was an error while loading. Please reload this page.
Fixes#79
Summary
The server mode was ignoring the IP addresses specified in the
HTTPJAIL_HTTP_BINDandHTTPJAIL_HTTPS_BINDenvironment variables, always binding to localhost instead.This PR refactors the proxy binding logic to use
std::net::SocketAddrthroughout, which provides a cleaner API that supports both IPv4 and IPv6 while combining IP and port into a single type.Changes
ProxyServerstruct to useOption<SocketAddr>for http_bind and https_bind instead of separate IP and port fieldsbind_listener()function that handles both IPv4 and IPv6bind_ipv4_listener()functionparse_bind_config()to returnOption<SocketAddr>directlyTesting
✅ All tests passing:
-D warningscargo fmt✅ Functional verification:
0.0.0.0when configuredSupported Bind Address Formats
"ip:port"(e.g.,"0.0.0.0:8080","127.0.0.1:8080")"[ipv6]:port"(e.g.,"[::1]:8080")"port"(defaults to localhost)Backward Compatibility
The fix maintains full backward compatibility while resolving the reported issue.