diff --git a/Cargo.lock b/Cargo.lock index 1ffac29f2f..81b5eb856b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6551,6 +6551,7 @@ dependencies = [ "tungstenite 0.29.0", "twox-hash", "ulid", + "url", "uuid", ] diff --git a/core/common/Cargo.toml b/core/common/Cargo.toml index fd2a0708c9..cb5aabe613 100644 --- a/core/common/Cargo.toml +++ b/core/common/Cargo.toml @@ -66,8 +66,8 @@ 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 79e1c81231..6962e82532 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/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 029eaf6b81..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 @@ -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,44 @@ 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) + } +} + +#[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 c6ee96e792..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 @@ -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,44 @@ 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) + } +} + +#[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 1720c23963..f89e5afa89 100644 --- a/core/common/src/utils/net.rs +++ b/core/common/src/utils/net.rs @@ -19,6 +19,7 @@ use crate::IggyError; use std::net::{Ipv4Addr, Ipv6Addr}; +use url::Url; /// Validates that `addr` is syntactically a valid `host:port` string. /// Does NOT perform DNS resolution. @@ -122,6 +123,46 @@ 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_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())); + } + + 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 +348,64 @@ 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()); + 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()); + } + + #[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_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()); + assert!(validate_api_url("https://:443").is_err()); + } + + #[test] + 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] + 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..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,11 +266,8 @@ 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(); + 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()) .with(TracingMiddleware::::new()) @@ -537,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()); + } }