#36636 netaddress: make .onion suffix check case-insensitive
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36636 netaddress: make .onion suffix check case-insensitive | Ready | 4 | Strong | 3 + 7 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Makes the `.onion` suffix check case-insensitive when parsing network addresses in `CNetAddr::SetTor`. This aligns Tor address parsing with existing I2P hostname handling and prevents addresses with uppercase `.ONION` suffixes from falling through to regular DNS hostname resolution.
Problem: Tor addresses containing uppercase letters in the `.onion` suffix are not recognized as Tor addresses. Instead, they fail `SetTor` and fall through to standard DNS hostname resolution, risking privacy leaks or connection failures.
Evidence of importance:
- PR description: 'With the current code, a name like <addr>.ONION will not be parsed as a Tor address, and will instead fall through to regular hostname resolution.'
Risk of a mistake: low (p2p, tests). A flaw in netaddress parsing could misparse network addresses or fail to identify Tor destinations, but the change is tiny, localized, and well-covered by unit tests.
Changed since the previous assessment: Now includes the complete diff and test coverage (a new unit test in `src/test/net_tests.cpp` validating mixed-case Tor address parsing); implementation logic remains the same.
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- author: respond to Husteem's suggestion to use CaseInsensitiveEqual 2026-10-26
Agreement
Strong
substantive support, no open objection (AndreaDiazCorreia, winterrdog)
Thread notes: Added ACKs from Husteem and winterrdog, and recorded Husteem's non-blocking suggestion regarding CaseInsensitiveEqual.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| Husteem (none) | suggestion, approach | Use CaseInsensitiveEqual from util/strencodings.h instead of ToLower to avoid temporary string creation | open | answered | no | 2026-10-26: 'Using `CaseInsensitiveEqual(addr.substr(addr.size() - suffix.size()), suffix)` from `util/strencodings.h` avoids the temporary string entirely. It would be nice to use that here' |
Support:
- vasild (member) (ACK, 2026-10-24): (no reason given) [not substantive]
- AndreaDiazCorreia (none) (ACK, 2026-10-25) [tested: net_tests, netbase_tests, base32_tests, netaddress.cpp]: Built and ran net_tests, netbase_tests, and base32_tests successfully. Confirmed the new test fails with the parent’s netaddress.cpp.
- Husteem (none) (ACK, 2026-10-26) [read]: (no reason given) [not substantive]
- winterrdog (contributor) (Tested ACK, 2026-10-26) [tested]: successfully built and tested on: Windows 10/MSVC 19.51.36257/x86_64. changes LGTM and the intent is pragmatic
Participants:
- vasild (member, support, 1 statements 2026-10-24 to 2026-10-24): ACKed commit
- AndreaDiazCorreia (none, support, 1 statements 2026-10-25 to 2026-10-25): tested ACK; ran net, netbase, and base32 tests
- Husteem (none, support, 1 statements 2026-10-26 to 2026-10-26): ACKed; suggested using CaseInsensitiveEqual
- winterrdog (contributor, support, 1 statements 2026-10-26 to 2026-10-26): tested ACK on Windows MSVC
Files
7 lines under test/bench/ci.
- src/test/net_tests.cpp +7/-0
- src/netaddress.cpp +3/-2
Card
The short text the ranking pass compares across PRs.
Makes the suffix check case-insensitive when parsing .onion network addresses, matching existing behavior for I2P hostnames. Currently, hostnames ending in uppercase variations like .ONION fail Tor address detection and fall through to regular DNS resolution. This prevents potential DNS leaks and connection failures when configuring or parsing mixed-case Tor addresses.
Data
thread read through 2026-10-26 (4 statements), head 0574f096ab
labels: none
Categories
P2P
P3 · bug fix (ranking pass: Grouped with the other Tor/I2P address-handling fixes.)
Fixes an address parsing edge case where uppercase `.ONION` addresses fail Tor detection and fall through to standard DNS hostname resolution. The PR description notes that this prevents unexpected DNS lookups and connection failures, plugging a minor privacy leak when parsing mixed-case Tor hostnames.
Membership: Touches src/netaddress.cpp and address parsing logic for the Tor integration.
Factors: security/stability 1, bug 1, performance 0, user value 1, leverage 0