Repository navigation
fix(iroh): handle IPv6 literal relay URLs in QAD probes - #4527
Merged
flub merged 11 commits intoOct 2, 2026
Merged
Conversation
6 tasks done
flub
approved these changes
Sep 29, 2026
flub
left a comment
flub
left a comment
Collaborator
There was a problem hiding this comment.
Hey, thanks for this PR and for bearing with. It has been a busy few weeks on this side. One small nit, let me know if you'd like to address it. It would be kinda neat if we did, but it is not absolutely required.
| } | ||
|
|
||
| #[cfg(not(wasm_browser))] | ||
| fn relay_tls_server_name(relay: &RelayConfig) -> Option<String> { |
Collaborator
There was a problem hiding this comment.
To make this extra neat it could return an std::borrow::Cow<str>, so that the common case does not need an allocation. But also this isn't on a hot path so probably doesn't matter that much.
Collaborator
|
ah, and it seems like the tests will need some tweaks for windows. |
Contributor
Author
|
@flub Thanks for the review! I’ve updated the PR as suggested. |
flub
enabled auto-merge
October 2, 2026 13:47
gsnaiper
pushed a commit
to gsnaiper/iroh
that referenced
this pull request
Oct 8, 2026
…4527) ## Description Fixes n0-computer#4526 QAD probes pass the relay URL's `host_str()` to QUIC as the TLS server name. For IPv6 literals this includes brackets, causing `invalid server name` before certificate verification. Extract the name from the parsed URL host instead, and use it in both QAD probe paths. Certificate verification and QAD port selection remain unchanged. The regression uses a local IPv6 QAD server with a trusted certificate for `::1`. A connection using the bare IP succeeds; probing through the IPv6 relay URL fails before the fix and succeeds afterwards. The relay URL and QAD configuration use different ports. A unit test also covers domain, IPv4 and IPv6 names. Validation on macOS arm64: ```sh cargo test --locked -p iroh --lib net_report:: # 9 passed cargo clippy --locked -p iroh --lib --tests -- -D warnings cargo +nightly make format-check ``` ## API Changes None. ## Notes & open questions None ## Change checklist - [x] Self-review. - [x] Documentation updates following the [style guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text), if relevant. - [x] Tests if relevant. - [x] All API changes documented. - [x] This PR was created by a human that thought critically about the proposed change and wrote an as clear and concise description as they could. - [x] This PR isn't slop, and is carefully crafted to do have the intented effect.
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
2 participants
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
Fixes #4526
QAD probes pass the relay URL's
host_str()to QUIC as the TLS server name. For IPv6 literals this includes brackets, causinginvalid server namebefore certificate verification.Extract the name from the parsed URL host instead, and use it in both QAD probe paths. Certificate verification and QAD port selection remain unchanged.
The regression uses a local IPv6 QAD server with a trusted certificate for
::1. A connection using the bare IP succeeds; probing through the IPv6 relay URL fails before the fix and succeeds afterwards. The relay URL and QAD configuration use different ports. A unit test also covers domain, IPv4 and IPv6 names.Validation on macOS arm64:
API Changes
None.
Notes & open questions
None
Change checklist
proposed change and wrote an as clear and concise description as
they could.
intented effect.