feat(configs): add address validation for `QUIC/HTTP` builders by Standing-Man · Pull Request #3152 · apache/iggy

Standing-Man · 2026-04-22T11:43:18Z

Which issue does this PR close?

Closes #3075.

Rationale

To align with the build methods in the tcp/websocket builders, add validation for server_address and api_url in QuicClientConfigBuilder::build() and HttpClientConfigBuilder::build(), respectively.

What changed?

In QuicClientConfigBuilder::build(), server_address is validated using validate_server_address.

In HttpClientConfigBuilder::build(), validation is performed in three steps:

Ensure the api_url can be successfully parsed by Url crate
If a scheme is present, restrict it to http or https
Ensure the parsed api_url contains only the host and port components in its URI

Local Execution

Passed / not passed
Passed
Pre-commit hooks ran / not ran
Ran

AI Usage

codecov · 2026-04-22T11:44:43Z

Codecov Report

❌ Patch coverage is 97.38562% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.84%. Comparing base (140f3c5) to head (1c2efbc).

Files with missing lines	Patch %	Lines
core/sdk/src/clients/client_builder.rs	0.00%	1 Missing and 1 partial ⚠️
core/common/src/utils/net.rs	98.38%	1 Missing ⚠️
core/sdk/src/http/http_client.rs	90.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3152      +/-   ##
============================================
- Coverage     74.48%   71.84%   -2.64%     
  Complexity      943      943              
============================================
  Files          1188     1188              
  Lines        106530   101801    -4729     
  Branches      83560    78849    -4711     
============================================
- Hits          79350    73144    -6206     
- Misses        24433    25709    +1276     
- Partials       2747     2948     +201

Components	Coverage Δ
Rust Core	`72.39% <97.38%> (-3.35%)`	⬇️
Java SDK	`60.14% <ø> (ø)`
C# SDK	`69.09% <ø> (-0.29%)`	⬇️
Python SDK	`81.43% <ø> (ø)`
Node SDK	`91.53% <ø> (ø)`
Go SDK	`39.80% <ø> (ø)`

Files with missing lines	Coverage Δ
core/common/src/error/iggy_error.rs	`100.00% <ø> (ø)`
...guration/http_config/http_client_config_builder.rs	`82.50% <100.00%> (+21.38%)`	⬆️
...guration/quic_config/quic_client_config_builder.rs	`29.00% <100.00%> (+29.00%)`	⬆️
...figuration/tcp_config/tcp_client_config_builder.rs	`68.04% <100.00%> (ø)`
...ebsocket_config/websocket_client_config_builder.rs	`30.20% <100.00%> (+30.20%)`	⬆️
core/sdk/src/quic/quic_client.rs	`72.94% <100.00%> (+0.39%)`	⬆️
core/common/src/utils/net.rs	`99.21% <98.38%> (-0.27%)`	⬇️
core/sdk/src/http/http_client.rs	`92.08% <90.00%> (+0.16%)`	⬆️
core/sdk/src/clients/client_builder.rs	`59.88% <0.00%> (ø)`

... and 122 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

hubcio · 2026-04-22T11:45:02Z

looks good, but next time please comment under the issue before implementation and wait till we give a green light.

Standing-Man · 2026-04-22T13:47:26Z

Hi @hubcio, @spetz, @numinnex and @mmodzelewski, just a gentle ping — this PR is ready on my side. Could you please take a look when convenient?

hubcio · 2026-04-29T11:33:54Z

+        _ => return Err(IggyError::InvalidApiUrl(addr.to_string())),
+    }
+
+    if api_url.host_str().is_none() || api_url.port().is_none() {


port().is_none() rejects valid urls with default-scheme ports. the url crate normalizes default ports during parse - Url::parse("http://example.com:80").port() returns None (per https://docs.rs/url/2.5.8/url/struct.Url.html#method.port: "default port numbers are never reflected by the serialization"). same for https://...:443. fix: switch to port_or_known_default().is_none() after the scheme allowlist. tests below don't cover :80 or :443 - please add them.

hubcio · 2026-04-29T11:33:54Z

+        || api_url.query().is_some()
+        || api_url.fragment().is_some()
+    {
+        return Err(IggyError::InvalidApiUrl(addr.to_string()));


port 0 inconsistency with validate_server_address. http://host:0 parses to Some(0) and passes here, but validate_server_address rejects port 0 (line 79, and [::1]:0 test at line 225). either both reject or both accept - align the policy.

hubcio · 2026-04-29T11:33:54Z

 once_cell = { workspace = true }
 papaya = { workspace = true }
 rcgen = { workspace = true }
+reqwest = { workspace = true }


pulling full reqwest into the foundation crate just to use reqwest::Url. reqwest's lib.rs is pub use url::Url;, and the workspace already declares url = "2.5.8" in root Cargo.toml. swap this to url = { workspace = true } and import use url::Url; in net.rs:21. avoids dragging hyper / tokio-tls / h2 transitively into every downstream consumer of iggy_common.

hubcio · 2026-04-29T11:33:55Z

-            return Err(IggyError::CannotParseUrl);
-        }
-        let api_url = api_url.unwrap();
+        let api_url = Url::parse(&config.api_url).map_err(|_| IggyError::CannotParseUrl)?;


validate_api_url only runs via HttpClientConfigBuilder::build. HttpClient::new, HttpClient::create, and HttpClient::from_connection_string all skip it - only Url::parse runs. direct HttpClientConfig { api_url: ..., ..Default::default() } literal also bypasses. validation contract leaks. move validate_api_url(&config.api_url)? here in create (the real construction boundary) so every entry path enforces it. same gap on quic side via QuicClient::create.

hubcio · 2026-04-29T11:33:55Z

    InvalidIpAddress(String, String) = 35,
    #[error("Http error {0}")]
    HttpError(String) = 36,
+    #[error("Invalid Api Url: {0}")]


"Invalid Api Url: {0}" doesn't match the acronym style used at line 86 ("Invalid IP address: {0}:{1}"). prefer "Invalid API URL: {0}".

hubcio · 2026-04-29T11:33:55Z

+        validate_api_url(&self.config.api_url)?;
+
+        Ok(self.config)
    }


no tests for this build(). tcp builder has a full table at tcp_client_config_builder.rs:124-189. add at minimum: default-builds-ok, trim semantics (e.g. " http://x:1 "), and one error case via invalid api_url. same gap in quic builder.

hubcio · 2026-04-29T11:33:55Z

+        validate_server_address(&self.config.server_address)?;
+
+        Ok(self.config)
    }


no tests for this build(). mirror the table from tcp_client_config_builder.rs:124-189 - default-builds-ok, trim semantics, and one error case via invalid server_address.

hubcio · 2026-04-29T11:33:55Z

-    pub fn build(self) -> QuicClientConfig {
-        self.config
+    pub fn build(mut self) -> Result<QuicClientConfig, IggyError> {
+        self.config.server_address = self.config.server_address.trim().to_owned();


uses .to_owned() while tcp/websocket builders use .to_string() (tcp_client_config_builder.rs:106, websocket_client_config_builder.rs:142). functionally identical for &str, but pick one across all four builders. http builder also uses .to_owned() at line 56.

hubcio · 2026-05-04T09:29:59Z

@Standing-Man do you have plans to continue? this is good change, just needs a little bit more polishing

Standing-Man · 2026-05-04T11:02:34Z

@Standing-Man do you have plans to continue? this is good change, just needs a little bit more polishing

@hubcio, I’ll continue polishing this PR — I’ve been a bit busy recently.

Standing-Man · 2026-05-06T11:48:40Z

Hi @hubcio, thank you very much for your detailed review. I’ve addressed the following issues in the latest commit:

Strengthened validate_api_url behavior
- Switched the port check to port_or_known_default() so default ports are treated as valid
- Added explicit rejection for :0 (api_url.port() == Some(0))
- Kept strict rejection for userinfo/path/query/fragment components
Added URL validation not only in the builders, but also in the constructors.
Added corresponding tests for the TCP and QUIC builders.
Replaced reqwest::Url usage with url::Url.

Signed-off-by: StandingMan <jmtangcs@gmail.com>

Standing-Man changed the title ~~feat(config): add address validation for QUIC/HTTP builders~~ feat(configs): add address validation for QUIC/HTTP builders Apr 22, 2026

Standing-Man force-pushed the address-validation branch from b4efaa8 to 751b415 Compare April 22, 2026 13:30

Standing-Man marked this pull request as ready for review April 22, 2026 13:34

hubcio requested changes Apr 29, 2026

View reviewed changes

Standing-Man force-pushed the address-validation branch 2 times, most recently from b411449 to 46af920 Compare May 6, 2026 10:54

Standing-Man requested a review from hubcio May 6, 2026 11:48

Standing-Man added 2 commits May 6, 2026 19:49

feat(config): add address validation for QUIC/HTTP builders

c887bac

Signed-off-by: StandingMan <jmtangcs@gmail.com>

feat(config): address review feedback

1c2efbc

Signed-off-by: StandingMan <jmtangcs@gmail.com>

Standing-Man force-pushed the address-validation branch from 46af920 to 1c2efbc Compare May 6, 2026 11:49

Conversation

Standing-Man commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale

What changed?

Local Execution

AI Usage

Uh oh!

codecov Bot commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

hubcio commented Apr 22, 2026

Uh oh!

Standing-Man commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hubcio commented May 4, 2026

Uh oh!

Standing-Man commented May 4, 2026

Uh oh!

Standing-Man commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Standing-Man commented Apr 22, 2026 •

edited

Loading

codecov Bot commented Apr 22, 2026 •

edited

Loading

Standing-Man commented Apr 22, 2026 •

edited

Loading

Standing-Man commented May 6, 2026 •

edited

Loading