From c887bacd33c9d476104fcbd8710ee9391bb8155f Mon Sep 17 00:00:00 2001 From: StandingMan Date: Wed, 22 Apr 2026 21:29:25 +0800 Subject: [PATCH 1/2] feat(config): add address validation for QUIC/HTTP builders Signed-off-by: StandingMan --- Cargo.lock | 1 + core/common/Cargo.toml | 2 +- core/common/src/error/iggy_error.rs | 2 + .../http_config/http_client_config_builder.rs | 9 +- .../quic_config/quic_client_config_builder.rs | 9 +- core/common/src/utils/net.rs | 89 +++++++++++++++++++ core/sdk/src/clients/client_builder.rs | 4 +- core/sdk/src/http/http_client.rs | 6 +- 8 files changed, 108 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 201f97dcfb..a0d4172eeb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6558,6 +6558,7 @@ dependencies = [ "once_cell", "papaya", "rcgen", + "reqwest 0.13.2", "ring", "rustls", "secrecy", diff --git a/core/common/Cargo.toml b/core/common/Cargo.toml index fd2a0708c9..0d3704fbf6 100644 --- a/core/common/Cargo.toml +++ b/core/common/Cargo.toml @@ -53,6 +53,7 @@ moka = { workspace = true } once_cell = { workspace = true } papaya = { workspace = true } rcgen = { workspace = true } +reqwest = { workspace = true } ring = { workspace = true } rustls = { workspace = true } secrecy = { workspace = true } @@ -67,7 +68,6 @@ tungstenite = { workspace = true } twox-hash = { workspace = true } ulid = { workspace = true } uuid = { workspace = true } - [target.'cfg(unix)'.dependencies] nix = { workspace = true } diff --git a/core/common/src/error/iggy_error.rs b/core/common/src/error/iggy_error.rs index 79e1c81231..b9844a1b01 100644 --- a/core/common/src/error/iggy_error.rs +++ b/core/common/src/error/iggy_error.rs @@ -86,6 +86,8 @@ pub enum IggyError { InvalidIpAddress(String, String) = 35, #[error("Http error {0}")] HttpError(String) = 36, + #[error("Invalid Api Url: {0}")] + InvalidApiUrl(String) = 37, #[error("Unauthenticated")] Unauthenticated = 40, #[error("Unauthorized")] diff --git a/core/common/src/types/configuration/http_config/http_client_config_builder.rs b/core/common/src/types/configuration/http_config/http_client_config_builder.rs index 029eaf6b81..771afea7de 100644 --- a/core/common/src/types/configuration/http_config/http_client_config_builder.rs +++ b/core/common/src/types/configuration/http_config/http_client_config_builder.rs @@ -16,7 +16,7 @@ * under the License. */ -use crate::HttpClientConfig; +use crate::{HttpClientConfig, IggyError, utils::net::validate_api_url}; /// The builder for the `HttpClientConfig` configuration. /// Allows configuring the HTTP client with custom settings or using defaults: @@ -52,7 +52,10 @@ impl HttpClientConfigBuilder { } /// Builds the `HttpClientConfig` instance. - pub fn build(self) -> HttpClientConfig { - self.config + pub fn build(mut self) -> Result { + self.config.api_url = self.config.api_url.trim().to_owned(); + validate_api_url(&self.config.api_url)?; + + Ok(self.config) } } diff --git a/core/common/src/types/configuration/quic_config/quic_client_config_builder.rs b/core/common/src/types/configuration/quic_config/quic_client_config_builder.rs index c6ee96e792..05152cf87a 100644 --- a/core/common/src/types/configuration/quic_config/quic_client_config_builder.rs +++ b/core/common/src/types/configuration/quic_config/quic_client_config_builder.rs @@ -16,7 +16,7 @@ * under the License. */ -use crate::{AutoLogin, IggyDuration, QuicClientConfig}; +use crate::{AutoLogin, IggyDuration, IggyError, QuicClientConfig, validate_server_address}; /// Builder for the QUIC client configuration. /// @@ -154,7 +154,10 @@ impl QuicClientConfigBuilder { } /// Finalizes the builder and returns the `QuicClientConfig`. - pub fn build(self) -> QuicClientConfig { - self.config + pub fn build(mut self) -> Result { + self.config.server_address = self.config.server_address.trim().to_owned(); + validate_server_address(&self.config.server_address)?; + + Ok(self.config) } } diff --git a/core/common/src/utils/net.rs b/core/common/src/utils/net.rs index 1720c23963..b8eff74e4a 100644 --- a/core/common/src/utils/net.rs +++ b/core/common/src/utils/net.rs @@ -18,6 +18,7 @@ */ use crate::IggyError; +use reqwest::Url; use std::net::{Ipv4Addr, Ipv6Addr}; /// Validates that `addr` is syntactically a valid `host:port` string. @@ -122,6 +123,42 @@ fn is_valid_host(host: &str) -> bool { is_valid_hostname(host) } +/// Validates that `addr` is a strict HTTP(S) API base URL in the format +/// `scheme://host:port`. +/// +/// Accepted formats: +/// - `http://hostname:port` / `https://hostname:port` +/// - `http://ipv4:port` / `https://ipv4:port` +/// - `http://[ipv6]:port` / `https://[ipv6]:port` +/// +/// Rejected formats: +/// - Schemes other than `http` and `https` +/// - Missing host or missing explicit port +/// - URLs with additional components beyond `scheme://host:port`, +/// such as userinfo (`user:pass@`), path, query string, or fragment. +pub fn validate_api_url(addr: &str) -> Result<(), IggyError> { + let api_url = Url::parse(addr).map_err(|_| IggyError::CannotParseUrl)?; + match api_url.scheme() { + "http" | "https" => {} + _ => return Err(IggyError::InvalidApiUrl(addr.to_string())), + } + + if api_url.host_str().is_none() || api_url.port().is_none() { + return Err(IggyError::InvalidApiUrl(addr.to_string())); + } + + if !api_url.username().is_empty() + || api_url.password().is_some() + || api_url.path() != "/" + || api_url.query().is_some() + || api_url.fragment().is_some() + { + return Err(IggyError::InvalidApiUrl(addr.to_string())); + } + + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -307,4 +344,56 @@ mod tests { assert!(validate_server_address("256.1.1.1:8090").is_err()); assert!(validate_server_address("192.168.1:8090").is_err()); } + + #[test] + fn validate_api_url_accepts_http_with_host_and_port() { + assert!(validate_api_url("http://127.0.0.1:3000").is_ok()); + assert!(validate_api_url("http://localhost:8080").is_ok()); + } + + #[test] + fn validate_api_url_accepts_https_with_host_and_port() { + assert!(validate_api_url("https://example.com:8443").is_ok()); + assert!(validate_api_url("https://api.example.com:8443").is_ok()); + } + + #[test] + fn validate_api_url_accepts_ipv6_host_with_port() { + assert!(validate_api_url("http://[::1]:3000").is_ok()); + assert!(validate_api_url("https://[2001:db8::1]:9443").is_ok()); + } + + #[test] + fn validate_api_url_rejects_non_http_schemes() { + assert!(validate_api_url("ftp://example.com:21").is_err()); + assert!(validate_api_url("ws://example.com:8080").is_err()); + } + + #[test] + fn validate_api_url_rejects_missing_host() { + assert!(validate_api_url("http://:3000").is_err()); + assert!(validate_api_url("https://:443").is_err()); + } + + #[test] + fn validate_api_url_rejects_missing_port() { + assert!(validate_api_url("http://example.com").is_err()); + assert!(validate_api_url("https://127.0.0.1").is_err()); + } + + #[test] + fn validate_api_url_rejects_additional_url_parts() { + assert!(validate_api_url("http://user@example.com:3000").is_err()); + assert!(validate_api_url("http://user:pass@example.com:3000").is_err()); + assert!(validate_api_url("http://example.com:3000/api").is_err()); + assert!(validate_api_url("http://example.com:3000?foo=bar").is_err()); + assert!(validate_api_url("http://example.com:3000#section").is_err()); + } + + #[test] + fn validate_api_url_rejects_non_url_values() { + assert!(validate_api_url("localhost:3000").is_err()); + assert!(validate_api_url("/api/v1").is_err()); + assert!(validate_api_url("").is_err()); + } } diff --git a/core/sdk/src/clients/client_builder.rs b/core/sdk/src/clients/client_builder.rs index 4f07c75abd..abad2be01c 100644 --- a/core/sdk/src/clients/client_builder.rs +++ b/core/sdk/src/clients/client_builder.rs @@ -280,7 +280,7 @@ impl QuicClientBuilder { /// Builds the parent `IggyClient` with QUIC configuration. pub fn build(self) -> Result { - let client = QuicClient::create(Arc::new(self.config.build()))?; + let client = QuicClient::create(Arc::new(self.config.build()?))?; let client = self .parent_builder .with_client(ClientWrapper::Quic(client)) @@ -316,7 +316,7 @@ impl HttpClientBuilder { /// Builds the parent `IggyClient` with HTTP configuration. pub fn build(self) -> Result { - let client = HttpClient::create(Arc::new(self.config.build()))?; + let client = HttpClient::create(Arc::new(self.config.build()?))?; let client = self .parent_builder .with_client(ClientWrapper::Http(client)) diff --git a/core/sdk/src/http/http_client.rs b/core/sdk/src/http/http_client.rs index bdc093dcfe..8020da1b3c 100644 --- a/core/sdk/src/http/http_client.rs +++ b/core/sdk/src/http/http_client.rs @@ -266,11 +266,7 @@ impl HttpClient { /// Create a new HTTP client for interacting with the Iggy API using the provided configuration. pub fn create(config: Arc) -> Result { - let api_url = Url::parse(&config.api_url); - if api_url.is_err() { - return Err(IggyError::CannotParseUrl); - } - let api_url = api_url.unwrap(); + let api_url = Url::parse(&config.api_url).map_err(|_| IggyError::CannotParseUrl)?; let retry_policy = ExponentialBackoff::builder().build_with_max_retries(config.retries); let client = ClientBuilder::new(reqwest::Client::new()) .with(TracingMiddleware::::new()) From 1c2efbc1f5f17572b05466e4e06d7b295cd38d33 Mon Sep 17 00:00:00 2001 From: StandingMan Date: Wed, 6 May 2026 18:51:41 +0800 Subject: [PATCH 2/2] feat(config): address review feedback Signed-off-by: StandingMan --- Cargo.lock | 2 +- core/common/Cargo.toml | 2 +- core/common/src/error/iggy_error.rs | 2 +- core/common/src/lib.rs | 1 + .../http_config/http_client_config_builder.rs | 34 ++++++++++++++++++ .../quic_config/quic_client_config_builder.rs | 34 ++++++++++++++++++ .../tcp_config/tcp_client_config_builder.rs | 2 +- .../websocket_client_config_builder.rs | 36 ++++++++++++++++++- core/common/src/utils/net.rs | 22 +++++++++--- core/sdk/src/http/http_client.rs | 14 +++++++- core/sdk/src/quic/quic_client.rs | 15 +++++++- 11 files changed, 152 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a0d4172eeb..ef58d2e51f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6558,7 +6558,6 @@ dependencies = [ "once_cell", "papaya", "rcgen", - "reqwest 0.13.2", "ring", "rustls", "secrecy", @@ -6573,6 +6572,7 @@ dependencies = [ "tungstenite", "twox-hash", "ulid", + "url", "uuid", ] diff --git a/core/common/Cargo.toml b/core/common/Cargo.toml index 0d3704fbf6..cb5aabe613 100644 --- a/core/common/Cargo.toml +++ b/core/common/Cargo.toml @@ -53,7 +53,6 @@ moka = { workspace = true } once_cell = { workspace = true } papaya = { workspace = true } rcgen = { workspace = true } -reqwest = { workspace = true } ring = { workspace = true } rustls = { workspace = true } secrecy = { workspace = true } @@ -67,6 +66,7 @@ tracing = { workspace = true } tungstenite = { workspace = true } twox-hash = { workspace = true } ulid = { workspace = true } +url = { workspace = true } uuid = { workspace = true } [target.'cfg(unix)'.dependencies] nix = { workspace = true } diff --git a/core/common/src/error/iggy_error.rs b/core/common/src/error/iggy_error.rs index b9844a1b01..6962e82532 100644 --- a/core/common/src/error/iggy_error.rs +++ b/core/common/src/error/iggy_error.rs @@ -86,7 +86,7 @@ pub enum IggyError { InvalidIpAddress(String, String) = 35, #[error("Http error {0}")] HttpError(String) = 36, - #[error("Invalid Api Url: {0}")] + #[error("Invalid API URL: {0}")] InvalidApiUrl(String) = 37, #[error("Unauthenticated")] Unauthenticated = 40, diff --git a/core/common/src/lib.rs b/core/common/src/lib.rs index e25a997333..0ab795b63e 100644 --- a/core/common/src/lib.rs +++ b/core/common/src/lib.rs @@ -126,6 +126,7 @@ pub use utils::crypto::*; pub use utils::duration::{IggyDuration, SEC_IN_MICRO}; pub use utils::expiry::IggyExpiry; pub use utils::hash::*; +pub use utils::net::validate_api_url; pub use utils::net::validate_server_address; pub use utils::personal_access_token_expiry::PersonalAccessTokenExpiry; pub use utils::random_id; diff --git a/core/common/src/types/configuration/http_config/http_client_config_builder.rs b/core/common/src/types/configuration/http_config/http_client_config_builder.rs index 771afea7de..72336bb0ce 100644 --- a/core/common/src/types/configuration/http_config/http_client_config_builder.rs +++ b/core/common/src/types/configuration/http_config/http_client_config_builder.rs @@ -59,3 +59,37 @@ impl HttpClientConfigBuilder { Ok(self.config) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn build_should_trim_and_validate_api_url() { + let config = HttpClientConfigBuilder::default() + .with_api_url(" http://127.0.0.1:3000 ".to_string()) + .build() + .expect("expected valid HTTP API URL"); + + assert_eq!(config.api_url, "http://127.0.0.1:3000"); + } + + #[test] + fn build_should_trim_whitespace_before_validation() { + let config = HttpClientConfigBuilder::default() + .with_api_url("\n\thttp://localhost:8080 \t".to_string()) + .build() + .expect("expected build() to trim before validation"); + + assert_eq!(config.api_url, "http://localhost:8080"); + } + + #[test] + fn build_should_fail_for_invalid_api_url() { + let result = HttpClientConfigBuilder::default() + .with_api_url("http://127.0.0.1:0".to_string()) + .build(); + + assert!(result.is_err()); + } +} diff --git a/core/common/src/types/configuration/quic_config/quic_client_config_builder.rs b/core/common/src/types/configuration/quic_config/quic_client_config_builder.rs index 05152cf87a..7707a58a83 100644 --- a/core/common/src/types/configuration/quic_config/quic_client_config_builder.rs +++ b/core/common/src/types/configuration/quic_config/quic_client_config_builder.rs @@ -161,3 +161,37 @@ impl QuicClientConfigBuilder { Ok(self.config) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn build_should_trim_and_validate_server_address() { + let config = QuicClientConfigBuilder::default() + .with_server_address(" 127.0.0.1:8080 ".to_string()) + .build() + .expect("expected valid QUIC server address"); + + assert_eq!(config.server_address, "127.0.0.1:8080"); + } + + #[test] + fn build_should_trim_whitespace_before_validation() { + let config = QuicClientConfigBuilder::default() + .with_server_address("\n\tlocalhost:8080 \t".to_string()) + .build() + .expect("expected build() to trim before validation"); + + assert_eq!(config.server_address, "localhost:8080"); + } + + #[test] + fn build_should_fail_for_invalid_server_address() { + let result = QuicClientConfigBuilder::default() + .with_server_address("127.0.0.1".to_string()) + .build(); + + assert!(result.is_err()); + } +} diff --git a/core/common/src/types/configuration/tcp_config/tcp_client_config_builder.rs b/core/common/src/types/configuration/tcp_config/tcp_client_config_builder.rs index 6c74acaf51..6f665a5777 100644 --- a/core/common/src/types/configuration/tcp_config/tcp_client_config_builder.rs +++ b/core/common/src/types/configuration/tcp_config/tcp_client_config_builder.rs @@ -103,7 +103,7 @@ impl TcpClientConfigBuilder { /// Builds the TCP client configuration. pub fn build(mut self) -> Result { - self.config.server_address = self.config.server_address.trim().to_string(); + self.config.server_address = self.config.server_address.trim().to_owned(); validate_server_address(&self.config.server_address)?; Ok(self.config) diff --git a/core/common/src/types/configuration/websocket_config/websocket_client_config_builder.rs b/core/common/src/types/configuration/websocket_config/websocket_client_config_builder.rs index 62bfe03e5b..7a3133ba4d 100644 --- a/core/common/src/types/configuration/websocket_config/websocket_client_config_builder.rs +++ b/core/common/src/types/configuration/websocket_config/websocket_client_config_builder.rs @@ -139,9 +139,43 @@ impl WebSocketClientConfigBuilder { /// Builds the WebSocket client configuration. pub fn build(mut self) -> Result { - self.config.server_address = self.config.server_address.trim().to_string(); + self.config.server_address = self.config.server_address.trim().to_owned(); validate_server_address(&self.config.server_address)?; Ok(self.config) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn build_should_trim_and_validate_server_address() { + let config = WebSocketClientConfigBuilder::default() + .with_server_address(" 127.0.0.1:8092 ".to_string()) + .build() + .expect("expected valid WebSocket server address"); + + assert_eq!(config.server_address, "127.0.0.1:8092"); + } + + #[test] + fn build_should_trim_whitespace_before_validation() { + let config = WebSocketClientConfigBuilder::default() + .with_server_address("\n\tlocalhost:8092 \t".to_string()) + .build() + .expect("expected build() to trim before validation"); + + assert_eq!(config.server_address, "localhost:8092"); + } + + #[test] + fn build_should_fail_for_invalid_server_address() { + let result = WebSocketClientConfigBuilder::default() + .with_server_address("127.0.0.1".to_string()) + .build(); + + assert!(result.is_err()); + } +} diff --git a/core/common/src/utils/net.rs b/core/common/src/utils/net.rs index b8eff74e4a..f89e5afa89 100644 --- a/core/common/src/utils/net.rs +++ b/core/common/src/utils/net.rs @@ -18,8 +18,8 @@ */ use crate::IggyError; -use reqwest::Url; use std::net::{Ipv4Addr, Ipv6Addr}; +use url::Url; /// Validates that `addr` is syntactically a valid `host:port` string. /// Does NOT perform DNS resolution. @@ -143,7 +143,11 @@ pub fn validate_api_url(addr: &str) -> Result<(), IggyError> { _ => return Err(IggyError::InvalidApiUrl(addr.to_string())), } - if api_url.host_str().is_none() || api_url.port().is_none() { + if api_url.host_str().is_none() || api_url.port_or_known_default().is_none() { + return Err(IggyError::InvalidApiUrl(addr.to_string())); + } + + if api_url.port() == Some(0) { return Err(IggyError::InvalidApiUrl(addr.to_string())); } @@ -349,10 +353,12 @@ mod tests { fn validate_api_url_accepts_http_with_host_and_port() { assert!(validate_api_url("http://127.0.0.1:3000").is_ok()); assert!(validate_api_url("http://localhost:8080").is_ok()); + assert!(validate_api_url("http://example.com:80").is_ok()); } #[test] fn validate_api_url_accepts_https_with_host_and_port() { + assert!(validate_api_url("https://example.com:443").is_ok()); assert!(validate_api_url("https://example.com:8443").is_ok()); assert!(validate_api_url("https://api.example.com:8443").is_ok()); } @@ -369,6 +375,12 @@ mod tests { assert!(validate_api_url("ws://example.com:8080").is_err()); } + #[test] + fn validate_api_url_rejects_port_zero() { + assert!(validate_api_url("http://example.com:0").is_err()); + assert!(validate_api_url("https://127.0.0.1:0").is_err()); + } + #[test] fn validate_api_url_rejects_missing_host() { assert!(validate_api_url("http://:3000").is_err()); @@ -376,9 +388,9 @@ mod tests { } #[test] - fn validate_api_url_rejects_missing_port() { - assert!(validate_api_url("http://example.com").is_err()); - assert!(validate_api_url("https://127.0.0.1").is_err()); + fn validate_api_url_accepts_implicit_default_port() { + assert!(validate_api_url("http://example.com").is_ok()); + assert!(validate_api_url("https://127.0.0.1").is_ok()); } #[test] diff --git a/core/sdk/src/http/http_client.rs b/core/sdk/src/http/http_client.rs index 8020da1b3c..40f2d26d7b 100644 --- a/core/sdk/src/http/http_client.rs +++ b/core/sdk/src/http/http_client.rs @@ -23,7 +23,7 @@ use async_trait::async_trait; use iggy_common::locking::{IggyRwLock, IggyRwLockFn}; use iggy_common::{ ConnectionString, ConnectionStringUtils, DiagnosticEvent, HttpConnectionStringOptions, - IdentityInfo, TransportProtocol, + IdentityInfo, TransportProtocol, validate_api_url, }; use reqwest::{Response, StatusCode, Url}; use reqwest_middleware::{ClientBuilder, ClientWithMiddleware}; @@ -266,6 +266,7 @@ impl HttpClient { /// Create a new HTTP client for interacting with the Iggy API using the provided configuration. pub fn create(config: Arc) -> Result { + validate_api_url(&config.api_url)?; let api_url = Url::parse(&config.api_url).map_err(|_| IggyError::CannotParseUrl)?; let retry_policy = ExponentialBackoff::builder().build_with_max_retries(config.retries); let client = ClientBuilder::new(reqwest::Client::new()) @@ -533,4 +534,15 @@ mod tests { IggyDuration::from_str("5s").unwrap() ); } + + #[test] + fn should_fail_create_with_invalid_api_url_even_without_builder() { + let config = Arc::new(HttpClientConfig { + api_url: "http://127.0.0.1:0".to_string(), + ..Default::default() + }); + + let http_client = HttpClient::create(config); + assert!(http_client.is_err()); + } } diff --git a/core/sdk/src/quic/quic_client.rs b/core/sdk/src/quic/quic_client.rs index 9a10dac2f9..0824e962a4 100644 --- a/core/sdk/src/quic/quic_client.rs +++ b/core/sdk/src/quic/quic_client.rs @@ -27,7 +27,7 @@ use async_trait::async_trait; use bytes::Bytes; use iggy_common::{ ClientState, ConnectionString, ConnectionStringUtils, Credentials, DiagnosticEvent, - QuicConnectionStringOptions, TransportProtocol, + QuicConnectionStringOptions, TransportProtocol, validate_server_address, }; use quinn::crypto::rustls::QuicClientConfig as QuinnQuicClientConfig; use quinn::{ClientConfig, Connection, Endpoint, IdleTimeout, RecvStream, VarInt}; @@ -164,6 +164,8 @@ impl QuicClient { /// Create a new QUIC client for the provided configuration. pub fn create(config: Arc) -> Result { + validate_server_address(&config.server_address)?; + let resolved_addr = config .server_address .to_socket_addrs() @@ -963,4 +965,15 @@ mod tests { let client = client.unwrap(); assert_eq!(client.config.server_address, "localhost:1234"); } + + #[test] + fn should_fail_create_with_invalid_server_address_even_without_builder() { + let config = Arc::new(QuicClientConfig { + server_address: "127.0.0.1".to_string(), + ..Default::default() + }); + + let client = QuicClient::create(config); + assert!(client.is_err()); + } }