fix(utils): parse and render IPv6 hosts in HostPortPair#3012
Open
CTOmari360 wants to merge 1 commit into
Open
Conversation
`HostPortPair::from_str` split the input on every `:`, so any IPv6
literal host produced more than the one or two fields it expected and
was rejected as `InvalidString`. The canonical bracketed forms
(`[2001:db8::1]` and `[2001:db8::1]:443`) and bare IPv6 literals
(`2001:db8::1`, `::1`) could not be parsed at all, even though every
IPv4 `host:port` worked. This is reachable from admin input: the
`site-explorer.bmc_proxy` setting is parsed via this `from_str`
(crates/api-core/src/handlers/api.rs), so an IPv6 BMC proxy could not
be configured.
`Display` had the matching defect: `HostAndPort("2001:db8::1", 443)`
rendered as the ambiguous `2001:db8::1:443`, which then failed to parse
back, breaking the serialize/deserialize round trip (the value is
stored back through `Serialize`).
Fix `from_str` to accept the bracketed forms and to treat a bare
string that parses as an `Ipv6Addr` as a host-only value; the host is
still stored unbracketed so existing callers that read `.host()` (and
bracket it themselves) are unaffected. Bracket an IPv6 literal host in
`Display` so the `host:port` form is unambiguous and round-trips.
IPv4 and hostname inputs are unchanged.
Extend the parsing table with bracketed/bare IPv6 cases and add a
Display round-trip test; the bracketed `[2001:db8::1]:8443` case fails
before this change.
Part of NVIDIA#2237 (IPv6 Bug Scan Bucket).
Signed-off-by: Omar Refai <omar@refai.org>
Contributor
Summary by CodeRabbit
WalkthroughIPv6 support is added to IPv6 Parsing and Display
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/utils/src/host_port_pair.rs`:
- Around line 71-96: The Redfish proxy URL builder in nv_redfish::mod::build
proxy URL logic is using the raw host from HostPortPair, so bare IPv6 literals
are rendered without brackets and produce an invalid URL. Update the URL
construction to bracket IPv6 hosts before interpolation, or switch the URL
assembly to use url::Url so host formatting is handled correctly. Use the
HostPortPair value from the existing host/port handling path to detect IPv6
literals and ensure the final https URL is valid for both IPv4/hostname and IPv6
cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0aac2bcb-8a2f-4e2e-9e45-8af1c98bae63
📒 Files selected for processing (1)
crates/utils/src/host_port_pair.rs
Comment on lines
+71
to
+96
| // A bracketed host is the canonical way to write an IPv6 literal, e.g. | ||
| // `[2001:db8::1]` or `[2001:db8::1]:443`. The host is stored without the | ||
| // brackets so callers can use it directly. The plain `split(':')` below | ||
| // can't handle this because an IPv6 address itself contains colons. | ||
| if let Some(rest) = s.strip_prefix('[') { | ||
| let (host, after) = rest.split_once(']').ok_or(InvalidString)?; | ||
| if host.is_empty() { | ||
| return Err(InvalidString); | ||
| } | ||
| return match after { | ||
| "" => Ok(HostPortPair::HostOnly(host.to_string())), | ||
| _ => { | ||
| let port = after.strip_prefix(':').ok_or(InvalidString)?; | ||
| let port = port | ||
| .parse::<u16>() | ||
| .map_err(|_| InvalidPort(port.to_string()))?; | ||
| Ok(HostPortPair::HostAndPort(host.to_string(), port)) | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| // A bare (unbracketed) IPv6 literal is a host with no port; its colons | ||
| // would otherwise be misread as host/port separators below. | ||
| if s.parse::<Ipv6Addr>().is_ok() { | ||
| return Ok(HostPortPair::HostOnly(s.to_string())); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how create_bmc / proxy_address build URLs from HostPortPair host fields.
fd -t f 'mod.rs' crates/redfish | xargs rg -nP -C4 'format!\("https://'
# Broader scan for any HostPortPair host interpolation lacking IPv6 bracketing.
rg -nP -C3 'HostPortPair::(HostAndPort|HostOnly|PortOnly)' --type=rustRepository: NVIDIA/infra-controller
Length of output: 1249
Bracket IPv6 hosts before building the Redfish proxy URL crates/redfish/src/nv_redfish/mod.rs:149-151
HostPortPair stores IPv6 literals without brackets, but this interpolation uses the raw host string, so 2001:db8::1 becomes https://2001:db8::1:443, which is not a valid URL. Add brackets here or construct the URL via url::Url.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/utils/src/host_port_pair.rs` around lines 71 - 96, The Redfish proxy
URL builder in nv_redfish::mod::build proxy URL logic is using the raw host from
HostPortPair, so bare IPv6 literals are rendered without brackets and produce an
invalid URL. Update the URL construction to bracket IPv6 hosts before
interpolation, or switch the URL assembly to use url::Url so host formatting is
handled correctly. Use the HostPortPair value from the existing host/port
handling path to detect IPv6 literals and ensure the final https URL is valid
for both IPv4/hostname and IPv6 cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
HostPortPair::from_strsplit its input on every:, so any IPv6 literal host yielded more fields than the one or two it expected and was rejected asInvalidString. None of the standard ways to write an IPv6 host could be parsed:[2001:db8::1]:443[2001:db8::1]2001:db8::1/::1…even though every IPv4
host:portparsed fine.This is reachable from admin input: the
site-explorer.bmc_proxydynamic setting is parsed through thisfrom_str(crates/api-core/src/handlers/api.rs), so an IPv6 BMC proxy could not be configured at all.Displayhad the matching defect —HostAndPort("2001:db8::1", 443)rendered as the ambiguous2001:db8::1:443, which then failed to parse back, breaking theSerialize/Deserializeround trip (the value is stored back out viaSerialize).Fix
from_strnow accepts the canonical bracketed forms and treats a bare string that parses as anIpv6Addras a host-only value. The host is still stored unbracketed, so existing callers that read.host()and bracket it themselves (e.g. the nv-redfish / bmc-proxy URL builders) are unaffected.Displaybrackets an IPv6 literal host in thehost:portform so it is unambiguous and round-trips.Part of #2237 (IPv6 Bug Scan Bucket).
Type of Change
Testing
test_proxy_address_parsingtable with bracketed/bare IPv6 cases (including malformed[2001:db8::1]:,[2001:db8::1,[]).test_ipv6_display_round_tripsassertingDisplayoutput and theDisplay→from_strround trip.Displayproduced2001:db8::1:443;from_str("[2001:db8::1]:8443")returnedErr(InvalidString)) and pass after.cargo test -p carbide-utils,cargo clippy -p carbide-utils --all-targets -- -D warnings, andcargo fmt -p carbide-utils -- --checkall green.