#36636 netaddress: make .onion suffix check case-insensitive

ReviewabilityReviewsAgreementSize
#36636 jpk68 netaddress: make .onion suffix check case-insensitiveReady4Strong3 + 7 tests
  • Prevent Tor addresses with uppercase suffixes from falling through to standard DNS resolution
  • Avoids potential privacy leaks and connection failures when handling mixed-case onion addresses
  • Ready for review
  • Strong support with no open objections
  • Verified by running net tests confirming failure on unpatched code (AndreaDiazCorreia)
  • Verified by testing the build on Windows MSVC (winterrdog)
  • Nonblocking suggestion to use CaseInsensitiveEqual to avoid a temporary string (Husteem)
  • Approval without stated reasons (vasild)
  • 3 lines added or modified outside tests
  • 7 lines added or modified in tests
  • 2 lines removed in total
  • 2 files, 1 commits
P2P#44 of 71
P3 · bug fix
  • P3 because it fixes an edge case where uppercase onion addresses fall back to regular DNS resolution
  • Plugs a minor privacy leak and prevents connection failures for mixed-case Tor destinations
  • Ranking pass: Grouped with the other Tor/I2P address-handling fixes.
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:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
Husteem (none)suggestion, approachUse CaseInsensitiveEqual from util/strencodings.h instead of ToLower to avoid temporary string creationopenansweredno2026-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

dossier JSON · extract JSON · model ledger, generated 2026-10-27T01:28, confidence high, input hash f88f04ece7df7d54
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