Skip to content

feat(configs): add address validation for QUIC/HTTP builders#3152

Merged
hubcio merged 3 commits into
apache:masterfrom
Standing-Man:address-validation
May 12, 2026
Merged

feat(configs): add address validation for QUIC/HTTP builders#3152
hubcio merged 3 commits into
apache:masterfrom
Standing-Man:address-validation

Conversation

@Standing-Man
Copy link
Copy Markdown
Contributor

@Standing-Man Standing-Man commented Apr 22, 2026

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:

  1. Ensure the api_url can be successfully parsed by Url crate
  2. If a scheme is present, restrict it to http or https
  3. 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

@Standing-Man 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
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

❌ Patch coverage is 97.38562% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.16%. Comparing base (1e44cc5) to head (307a21e).

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     73.79%   71.16%   -2.63%     
  Complexity      943      943              
============================================
  Files          1190     1190              
  Lines        107692   102963    -4729     
  Branches      84709    79997    -4712     
============================================
- Hits          79468    73277    -6191     
- Misses        25467    26732    +1265     
- Partials       2757     2954     +197     
Components Coverage Δ
Rust Core 71.49% <97.38%> (-3.34%) ⬇️
Java SDK 60.14% <ø> (ø)
C# SDK 69.15% <ø> (-0.31%) ⬇️
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 113 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
Copy link
Copy Markdown
Contributor

hubcio commented Apr 22, 2026

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

@Standing-Man Standing-Man marked this pull request as ready for review April 22, 2026 13:34
@Standing-Man
Copy link
Copy Markdown
Contributor Author

Standing-Man commented Apr 22, 2026

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?

Comment thread core/common/src/utils/net.rs Outdated
Comment thread core/common/src/utils/net.rs
Comment thread core/common/Cargo.toml Outdated
Comment thread core/sdk/src/http/http_client.rs
Comment thread core/common/src/error/iggy_error.rs Outdated
@hubcio
Copy link
Copy Markdown
Contributor

hubcio commented May 4, 2026

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

@Standing-Man
Copy link
Copy Markdown
Contributor Author

@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 Standing-Man force-pushed the address-validation branch 2 times, most recently from b411449 to 46af920 Compare May 6, 2026 10:54
@Standing-Man
Copy link
Copy Markdown
Contributor Author

Standing-Man commented May 6, 2026

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

  1. 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
  2. Added URL validation not only in the builders, but also in the constructors.
  3. Added corresponding tests for the TCP and QUIC builders.
  4. Replaced reqwest::Url usage with url::Url.

@Standing-Man Standing-Man requested a review from hubcio May 6, 2026 11:48
@Standing-Man Standing-Man force-pushed the address-validation branch from 46af920 to 1c2efbc Compare May 6, 2026 11:49
@hubcio hubcio merged commit 9b9be19 into apache:master May 12, 2026
80 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Address validation for QuicClientConfigBuilder and HttpClientConfigBuilder

4 participants