Reject interface-scoped addresses; prefer IPv4 in mDNS resolve - #2
Merged
Merged
Conversation
…esolve NWEndpoint's resolved remote address carries a %<interfaceName> scope whenever a path is tied to a specific local interface — routine on any multi-interface host, for both IPv4 and IPv6, not just a link-local-IPv6 edge case. That scoped string isn't a valid URL host, and every URL-based transport built on this library (acumen-shelly's HTTP/WebSocket transports) crashed the whole process outright when it reached one, rather than failing the one resolve. Confirmed against real hardware: a Mac with two active interfaces resolved a perfectly reachable Shelly device to "192.168.173.104%en1". NetworkAddress(endpoint:) now rejects any scoped address instead of handing it out, which surfaces as an ordinary resolveFailed the setup flow already treats as "fall back to the .local hostname" — resolved fresh and unscoped at actual connection time. Unscoped IPv6 is now properly bracketed per RFC 3986. resolve() also tries an IPv4-only constrained connection first (bounded by a 2s timeout, since a no-viable-path NWConnection commonly parks in .waiting rather than .failed) before falling back to an unconstrained attempt.
Mirrors the MDNSApple fix in the same PR, for the hand-rolled Linux backend: a resolved AAAA record formatted as a bare, unbracketed colon-separated IPv6 literal, which is not a valid URL host either. handleResponse also resolved on whichever record type it encountered first in a response with no preference, unlike MDNSApple's IPv4-first behavior. Extracted the pure formatting/ordering logic into LinuxServiceDiscovery.resolvableHosts(from:) — A records always ordered before AAAA regardless of packet order, AAAA formatted bracketed — so both fixes are unit-testable without a live socket.
Rejecting a scoped address outright (the previous commit) discarded its port along with it. That port came from the very same successful SRV resolution as the rejected host — mDNS *browsing* alone never carries a port at all (ServiceRecord.port is 0 until something resolves it) — so a caller falling back to a bare "<name>.local" guess loses it, producing "http://foo.local:0/rpc": a URL that builds fine via URLComponents but fails at request-submission time (NSURLErrorBadURL), just moving the same "not usable" problem from the host to the port. Confirmed against real hardware immediately after the previous fix shipped. NetworkAddress(endpoint:) now takes an optional fallbackHostname, substituted for a scoped host while keeping its correctly-resolved port; AppleServiceDiscovery.resolve passes "<record.name>.local".
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
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.
Summary
NetworkAddress(endpoint:)(MDNSApple) used to hand back whatever raw stringIPv4Address/IPv6Addressinterpolation produced — including, for an interface-scoped address, a%<interfaceName>suffix thatURLComponentscannot parse. This is routine on any multi-interface host (Wi-Fi + Ethernet, a VPN, a bridge) for both IPv4 and IPv6, not just a link-local-IPv6 edge case. Every URL-based transport built on this library crashed the whole process outright the first time it reached one, instead of failing just the one resolve.192.168.173.104%en1, which crashedacumen-shelly's HTTP transport via apreconditionFailurein its URL-construction getter.MDNSLinux) had an analogous, independently-arrived-at bug: AAAA-derived hosts were formatted as bare, unbracketed colon-separated literals (also not a valid URL host), andhandleResponseresolved on whichever record type it saw first with no IPv4 preference."<name>.local"guess lost it, producinghttp://foo.local:0/rpc: builds fine viaURLComponents, fails at request-submission time (NSURLErrorBadURL). Now fixed by substituting the fallback hostname while preserving the resolved port, instead of rejecting outright.Changes
NetworkAddress(endpoint:fallbackHostname:)substitutesfallbackHostnamefor a scoped.ipv4/.ipv6address while keeping its resolved port; with no fallback given (the default), a scoped address is still rejected (nil) as before. Unscoped IPv6 is bracketed per RFC 3986.AppleServiceDiscovery.resolve(_:)passes"<record.name>.local"as the fallback, and attempts an IPv4-constrained connection first (bounded by a 2s timeout — a no-viable-pathNWConnectioncommonly parks in.waitingrather than.failed), falling back to an unconstrained attempt.LinuxServiceDiscovery's A/AAAA handling extracted into a pureresolvableHosts(from:): A records always ordered before AAAA regardless of packet order, AAAA formatted bracketed.Test plan
swift build— cleanswift test— 30/30 passing (14 new: 9 inNetworkAddressTests, 5 inLinuxServiceDiscoveryResolveTests).namepass-through, non-.hostPortendpoints, AAAA bracketing, A-before-AAAA ordering regardless of packet order, malformed byte-count records skipped — plus a direct regression check that every accepted host actually builds a validURLComponents().url