diff --git a/Sources/MDNSApple/AppleServiceDiscovery.swift b/Sources/MDNSApple/AppleServiceDiscovery.swift index e0e32a9..7772e37 100644 --- a/Sources/MDNSApple/AppleServiceDiscovery.swift +++ b/Sources/MDNSApple/AppleServiceDiscovery.swift @@ -219,47 +219,116 @@ public final class AppleServiceDiscovery: ServiceDiscovery, @unchecked Sendable } } + /// How long an IPv4-constrained resolve attempt gets before falling back + /// to an unconstrained one. Needed because an `NWConnection` with no + /// viable path for its constraint (e.g. an IPv4-only connection to a + /// device with no IPv4 address) commonly parks in `.waiting` rather than + /// transitioning to `.failed` — without a deadline, ``resolve(_:)`` + /// would simply hang for an IPv6-only device instead of falling back. + static let ipv4PreferenceTimeout: Duration = .seconds(2) + public func resolve(_ record: ServiceRecord) async throws -> NetworkAddress { - // Create a connection to the browsed service to trigger resolution let endpoint = NWEndpoint.service( name: record.name, type: record.serviceType.rawValue, domain: "local.", interface: nil ) - let connection = NWConnection(to: endpoint, using: .udp) - return try await withCheckedThrowingContinuation { (cont: CheckedContinuation) in - nonisolated(unsafe) var resumed = false - connection.stateUpdateHandler = { [weak self] state in - guard let self else { return } - guard !resumed else { return } - switch state { - case .ready: - if let remoteEndpoint = connection.currentPath?.remoteEndpoint, - let address = NetworkAddress(endpoint: remoteEndpoint) { - resumed = true - cont.resume(returning: address) - } else { - resumed = true - cont.resume(throwing: ServiceDiscoveryError.resolveFailed(record.name)) + // Prefer IPv4: most LAN smart-home devices this library targets + // (Shelly, Hue, Matter accessories) are IPv4-first in practice, and + // an unscoped IPv6 candidate isn't guaranteed to exist or work end + // to end even when mDNS opportunistically announces one. If no + // IPv4-constrained attempt succeeds in time, fall through to an + // unconstrained attempt — whatever `currentPath.remoteEndpoint` + // resolves to. Either attempt can still yield a scoped address; see + // `resolveOnce`'s `fallbackHostname` use for why that's not a + // failure. + let ipv4Parameters = NWParameters.udp + if let ipOptions = ipv4Parameters.defaultProtocolStack.internetProtocol as? NWProtocolIP.Options { + ipOptions.version = .v4 + } + if let address = try? await resolveOnce( + endpoint: endpoint, + parameters: ipv4Parameters, + recordName: record.name, + timeout: Self.ipv4PreferenceTimeout + ) { + return address + } + + return try await resolveOnce(endpoint: endpoint, parameters: .udp, recordName: record.name, timeout: nil) + } + + /// One resolve attempt over one `NWConnection`, optionally bounded by + /// `timeout` (used only for the IPv4-preference attempt — an + /// unconstrained fallback attempt has nothing further to fall back to, + /// so it runs until `NWConnection` itself reports `.ready`/`.failed`). + private func resolveOnce( + endpoint: NWEndpoint, + parameters: NWParameters, + recordName: String, + timeout: Duration? + ) async throws -> NetworkAddress { + let connection = NWConnection(to: endpoint, using: parameters) + + return try await withThrowingTaskGroup(of: NetworkAddress.self) { group in + group.addTask { + try await withCheckedThrowingContinuation { (cont: CheckedContinuation) in + nonisolated(unsafe) var resumed = false + connection.stateUpdateHandler = { [weak self] state in + guard let self else { return } + guard !resumed else { return } + switch state { + case .ready: + // A `fallbackHostname` matters even here, not + // just in the caller's own resolve-failure + // fallback: a scoped host still carries a + // correctly-resolved port from this same SRV + // lookup, which mDNS *browsing* alone never + // provides (`ServiceRecord.port` is `0` until + // something resolves it) — losing that port by + // treating a scoped host as a hard failure would + // just move the same "not a valid URL" problem + // from a bad host to a bad (`0`) port. + if let remoteEndpoint = connection.currentPath?.remoteEndpoint, + let address = NetworkAddress( + endpoint: remoteEndpoint, + fallbackHostname: "\(recordName).local" + ) { + resumed = true + cont.resume(returning: address) + } else { + resumed = true + cont.resume(throwing: ServiceDiscoveryError.resolveFailed(recordName)) + } + connection.cancel() + case .failed(let error): + self.logger.error("Resolve failed for '\(recordName)': \(error)") + resumed = true + cont.resume(throwing: error) + connection.cancel() + case .cancelled: + if !resumed { + resumed = true + cont.resume(throwing: CancellationError()) + } + default: + break + } } + connection.start(queue: self.queue) + } + } + if let timeout { + group.addTask { + try await Task.sleep(for: timeout) connection.cancel() - case .failed(let error): - self.logger.error("Resolve failed for '\(record.name)': \(error)") - resumed = true - cont.resume(throwing: error) - connection.cancel() - case .cancelled: - if !resumed { - resumed = true - cont.resume(throwing: CancellationError()) - } - default: - break + throw ServiceDiscoveryError.resolveFailed(recordName) } } - connection.start(queue: self.queue) + defer { group.cancelAll() } + return try await group.next()! } } diff --git a/Sources/MDNSApple/NWEndpoint+NetworkAddress.swift b/Sources/MDNSApple/NWEndpoint+NetworkAddress.swift index 02eb954..056bb86 100644 --- a/Sources/MDNSApple/NWEndpoint+NetworkAddress.swift +++ b/Sources/MDNSApple/NWEndpoint+NetworkAddress.swift @@ -14,16 +14,55 @@ extension NetworkAddress { /// /// Only `.hostPort` endpoints are supported — returns `nil` for /// `.service`, `.unix`, `.url`, or other endpoint types. - public init?(endpoint: NWEndpoint) { + /// + /// Substitutes `fallbackHostname` (if provided) for an + /// **interface-scoped** `.ipv4`/`.ipv6` address (`addr.interface != nil` + /// — both address families support this, not just IPv6). + /// Network.framework attaches a scope whenever a resolved path is tied + /// to a specific local interface (routine on a multi-interface host — + /// Wi-Fi + Ethernet, a VPN, a bridge — not just a link-local-address + /// edge case), and its string form appends a `%` suffix + /// (`"192.168.1.23%en1"`, `"fe80::1%en0"`). That suffix is not valid + /// inside a URL authority component — `URLComponents` cannot parse it — + /// and it is not portable even where some caller could work around + /// that: `en1` names an interface on *this* process's view of the + /// network, meaningless after a restart, from a different host, or once + /// persisted as a plain string. + /// + /// Substituting the hostname (rather than failing the whole resolve) + /// matters because **`port` is still correct** — it came from the same + /// successful SRV resolution as the rejected host, and mDNS browsing + /// alone never carries a port at all (`ServiceRecord.port` is `0` until + /// something resolves it). A caller that instead discards this whole + /// address and falls back to a bare "`.local`" guess, as if + /// nothing had been resolved, loses that port along with it — producing + /// a URL like `http://foo.local:0/rpc`, invalid for an entirely + /// different reason than the one this fix addresses. With no + /// `fallbackHostname` supplied, the address is rejected (`nil`) exactly + /// as before. + public init?(endpoint: NWEndpoint, fallbackHostname: String? = nil) { guard case .hostPort(let host, let port) = endpoint else { return nil } let hostString: String switch host { case .ipv4(let addr): - hostString = "\(addr)" + if let fallbackHostname, addr.interface != nil { + hostString = fallbackHostname + } else { + guard addr.interface == nil else { return nil } + hostString = "\(addr)" + } case .ipv6(let addr): - hostString = "\(addr)" + if let fallbackHostname, addr.interface != nil { + hostString = fallbackHostname + } else { + guard addr.interface == nil else { return nil } + // Bracket per RFC 3986 — a bare IPv6 literal is not a valid + // URL host. Only reachable for a non-scoped address (global + // unicast or ULA); a scoped one is handled above. + hostString = "[\(addr)]" + } case .name(let name, _): hostString = name @unknown default: diff --git a/Sources/MDNSLinux/LinuxServiceDiscovery.swift b/Sources/MDNSLinux/LinuxServiceDiscovery.swift index 6e6544b..128f9ae 100644 --- a/Sources/MDNSLinux/LinuxServiceDiscovery.swift +++ b/Sources/MDNSLinux/LinuxServiceDiscovery.swift @@ -252,40 +252,70 @@ public actor LinuxServiceDiscovery: ServiceDiscovery { } private func handleResponse(_ msg: DNSMessage) { - for record in msg.answers + msg.additionals { - switch record.rdata { - case .ptr(let domain): - let parts = record.name.split(separator: ".") - guard parts.count >= 3 else { continue } - // record.name is e.g. "_hap._tcp.local" — strip ".local" for lookup key - let serviceTypeKey = parts.dropLast().joined(separator: ".") - guard let conts = browseStreams[serviceTypeKey], !conts.isEmpty else { continue } - // domain is "InstanceName._hap._tcp.local" - let domainParts = domain.split(separator: ".") - let instanceName = domainParts.first.map(String.init) ?? domain - let serviceTypeParts = domainParts.dropFirst().dropLast().joined(separator: ".") - let discovered = ServiceRecord( - name: instanceName, serviceType: ServiceType(serviceTypeParts), - host: "", port: 0, txtRecords: [:] - ) - for cont in conts.values { cont.yield(discovered) } - - case .a(let bytes): - guard bytes.count == 4 else { continue } - let host = "\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])" - fulfillResolve(recordName: record.name, host: host) - - case .aaaa(let bytes): - guard bytes.count == 16 else { continue } - let groups = stride(from: 0, to: 16, by: 2).map { i in - String(format: "%x", UInt16(bytes[i]) << 8 | UInt16(bytes[i + 1])) - } - fulfillResolve(recordName: record.name, host: groups.joined(separator: ":")) + let records = msg.answers + msg.additionals + + for record in records { + guard case .ptr(let domain) = record.rdata else { continue } + let parts = record.name.split(separator: ".") + guard parts.count >= 3 else { continue } + // record.name is e.g. "_hap._tcp.local" — strip ".local" for lookup key + let serviceTypeKey = parts.dropLast().joined(separator: ".") + guard let conts = browseStreams[serviceTypeKey], !conts.isEmpty else { continue } + // domain is "InstanceName._hap._tcp.local" + let domainParts = domain.split(separator: ".") + let instanceName = domainParts.first.map(String.init) ?? domain + let serviceTypeParts = domainParts.dropFirst().dropLast().joined(separator: ".") + let discovered = ServiceRecord( + name: instanceName, serviceType: ServiceType(serviceTypeParts), + host: "", port: 0, txtRecords: [:] + ) + for cont in conts.values { cont.yield(discovered) } + } + + for (recordName, host) in Self.resolvableHosts(from: records) { + fulfillResolve(recordName: recordName, host: host) + } + } - default: - break + /// Extracts `(recordName, host)` pairs from A/AAAA records in resolve + /// order — **every A match before any AAAA match**, regardless of the + /// order records happened to appear in the packet. `fulfillResolve` + /// resolves and discards the pending continuation on its first call for + /// a given name, so whichever pair is considered first always wins; + /// without this ordering, an AAAA record placed earlier in the same + /// response would win over an A record placed later, even though this + /// library otherwise prefers IPv4 throughout (see `MDNSApple`'s + /// equivalent preference in `AppleServiceDiscovery.resolve`). This only + /// resolves the single-packet case — an AAAA-only packet arriving + /// before a separate, later A-only packet for the same name is a + /// cross-packet race this cannot fix without buffering, which isn't + /// implemented here. + /// + /// An AAAA record's 16 raw bytes are formatted as a bracketed IPv6 + /// literal (`"[fe80:0:0:0:1234:5678:9abc:def0]"`) — RFC 3986 requires + /// the brackets; a bare colon-separated literal is not a valid URL host + /// and previously broke `URLComponents`-based transports downstream + /// (see the sibling fix in `MDNSApple`'s `NetworkAddress(endpoint:)`). + /// No zero-run compression (`::`) — verbose but unambiguous, and this + /// hand-rolled parser has no need to match `nw_path`/`getnameinfo`'s + /// exact canonical form. + /// + /// Pure and side-effect-free — the actual regression coverage for both + /// fixes lives here, unit-testable with no live socket involved. + static func resolvableHosts(from records: [DNSRecord]) -> [(recordName: String, host: String)] { + var results: [(recordName: String, host: String)] = [] + for record in records { + guard case .a(let bytes) = record.rdata, bytes.count == 4 else { continue } + results.append((record.name, "\(bytes[0]).\(bytes[1]).\(bytes[2]).\(bytes[3])")) + } + for record in records { + guard case .aaaa(let bytes) = record.rdata, bytes.count == 16 else { continue } + let groups = stride(from: 0, to: 16, by: 2).map { i in + String(format: "%x", UInt16(bytes[i]) << 8 | UInt16(bytes[i + 1])) } + results.append((record.name, "[\(groups.joined(separator: ":"))]")) } + return results } private func fulfillResolve(recordName: String, host: String) { diff --git a/Tests/MDNSAppleTests/NetworkAddressTests.swift b/Tests/MDNSAppleTests/NetworkAddressTests.swift new file mode 100644 index 0000000..ea3a02c --- /dev/null +++ b/Tests/MDNSAppleTests/NetworkAddressTests.swift @@ -0,0 +1,120 @@ +// NetworkAddressTests.swift +// Copyright 2026 Monagle Pty Ltd + +import Testing +import Foundation +import MDNSCore + +#if canImport(Network) +import Network +@testable import MDNSApple + +/// `NetworkAddress(endpoint:)` used to hand back whatever raw string +/// `IPv4Address`/`IPv6Address` interpolation produced — including, for an +/// interface-scoped address (routine on any multi-interface host, not just +/// a link-local-IPv6 edge case), a `%` suffix that +/// `URLComponents` cannot parse at all. That broke every URL-based +/// transport built on top of this library with a crash, not a catchable +/// error — confirmed against real hardware, see `acumen-shelly`'s CLAUDE.md. +/// These pin down both the rejection of scoped addresses and the URL +/// safety of everything this initializer does still accept. +@Suite("NetworkAddress(endpoint:) never yields a host that breaks URL construction") +struct NetworkAddressTests { + @Test("a plain (unscoped) IPv4 endpoint resolves to a usable, URL-safe host") + func plainIPv4() throws { + let addr = try #require(IPv4Address("192.168.1.23")) + let endpoint = NWEndpoint.hostPort(host: .ipv4(addr), port: 80) + let address = try #require(NetworkAddress(endpoint: endpoint)) + #expect(address.host == "192.168.1.23") + #expect(address.port == 80) + try #require(urlBuilds(fromHost: address.host)) + } + + @Test("an interface-scoped IPv4 endpoint (e.g. \"192.168.1.23%en1\") is rejected when no fallback is given") + func scopedIPv4Rejected() throws { + let addr = try #require(IPv4Address("192.168.1.23%en1")) + #expect(addr.interface != nil) // sanity: this really is what we think it is + let endpoint = NWEndpoint.hostPort(host: .ipv4(addr), port: 80) + #expect(NetworkAddress(endpoint: endpoint) == nil) + } + + @Test("a scoped IPv4 endpoint substitutes the fallback hostname but keeps the resolved port") + func scopedIPv4UsesFallbackHostnamePreservingPort() throws { + let addr = try #require(IPv4Address("192.168.173.104%en1")) + let endpoint = NWEndpoint.hostPort(host: .ipv4(addr), port: 80) + let address = try #require(NetworkAddress(endpoint: endpoint, fallbackHostname: "shelly1.local")) + #expect(address.host == "shelly1.local") + // The real, actual bug this guards against: a caller that discards + // a scoped address's port along with its rejected host — falling + // back to a bare ".local" guess with no port info, since mDNS + // browsing alone never carries one — ends up dispatching to port 0, + // which fails at request-submission time (NSURLErrorBadURL) even + // though `URLComponents` itself doesn't reject it. Confirmed + // against real hardware — see CLAUDE.md item 5's fix history. + #expect(address.port == 80) + try #require(urlBuilds(fromHost: address.host, port: Int(address.port))) + } + + @Test("a plain (unscoped) global/ULA IPv6 endpoint resolves to a bracketed, URL-safe host") + func plainIPv6() throws { + let addr = try #require(IPv6Address("fd00::1")) + let endpoint = NWEndpoint.hostPort(host: .ipv6(addr), port: 8080) + let address = try #require(NetworkAddress(endpoint: endpoint)) + #expect(address.host == "[fd00::1]") + try #require(urlBuilds(fromHost: address.host)) + } + + @Test("a link-local (interface-scoped) IPv6 endpoint is rejected, not bracketed as-is, when no fallback is given") + func scopedIPv6Rejected() throws { + let addr = try #require(IPv6Address("fe80::1234:5678:9abc:def0%en0")) + #expect(addr.interface != nil) + let endpoint = NWEndpoint.hostPort(host: .ipv6(addr), port: 80) + #expect(NetworkAddress(endpoint: endpoint) == nil) + } + + @Test("a scoped IPv6 endpoint substitutes the fallback hostname but keeps the resolved port") + func scopedIPv6UsesFallbackHostnamePreservingPort() throws { + let addr = try #require(IPv6Address("fe80::1234:5678:9abc:def0%en0")) + let endpoint = NWEndpoint.hostPort(host: .ipv6(addr), port: 8080) + let address = try #require(NetworkAddress(endpoint: endpoint, fallbackHostname: "shelly1.local")) + #expect(address.host == "shelly1.local") + #expect(address.port == 8080) + try #require(urlBuilds(fromHost: address.host, port: Int(address.port))) + } + + @Test("a plain, unscoped address ignores a supplied fallback hostname entirely") + func unscopedAddressIgnoresFallback() throws { + let addr = try #require(IPv4Address("192.168.1.23")) + let endpoint = NWEndpoint.hostPort(host: .ipv4(addr), port: 80) + let address = try #require(NetworkAddress(endpoint: endpoint, fallbackHostname: "shelly1.local")) + #expect(address.host == "192.168.1.23") + } + + @Test("a .name host (an mDNS hostname target) passes through unchanged") + func nameHostPassesThrough() throws { + let endpoint = NWEndpoint.hostPort(host: .name("shelly1pmg4-abc123.local", nil), port: 80) + let address = try #require(NetworkAddress(endpoint: endpoint)) + #expect(address.host == "shelly1pmg4-abc123.local") + try #require(urlBuilds(fromHost: address.host)) + } + + @Test("a non-hostPort endpoint (e.g. .service) yields nil") + func nonHostPortEndpointRejected() { + let endpoint = NWEndpoint.service(name: "x", type: "_shelly._tcp", domain: "local.", interface: nil) + #expect(NetworkAddress(endpoint: endpoint) == nil) + } + + /// The actual regression guard: whatever `NetworkAddress(endpoint:)` + /// does hand back must be usable to build a real URL — this is the + /// exact construction `ShellyHTTPTransportConfiguration.makeURL()` (and + /// any future consumer) performs. + private func urlBuilds(fromHost host: String, port: Int = 80) -> Bool { + var components = URLComponents() + components.scheme = "http" + components.host = host + components.port = port + components.path = "/rpc" + return components.url != nil + } +} +#endif diff --git a/Tests/MDNSLinuxTests/LinuxServiceDiscoveryResolveTests.swift b/Tests/MDNSLinuxTests/LinuxServiceDiscoveryResolveTests.swift new file mode 100644 index 0000000..07fe38a --- /dev/null +++ b/Tests/MDNSLinuxTests/LinuxServiceDiscoveryResolveTests.swift @@ -0,0 +1,83 @@ +// LinuxServiceDiscoveryResolveTests.swift +// Copyright 2026 Monagle Pty Ltd + +import Testing +import Foundation +@testable import MDNSLinux + +/// `LinuxServiceDiscovery.resolvableHosts(from:)` — the pure, side-effect-free +/// core of `handleResponse`'s A/AAAA handling. Pins down two fixes: +/// +/// 1. An AAAA record used to format as a bare, unbracketed colon-separated +/// literal (`"fe80:0:0:0:1234:5678:9abc:def0"`), which is not a valid URL +/// host — the same class of defect fixed in `MDNSApple`'s +/// `NetworkAddress(endpoint:)` (see that file's tests), just reached via +/// hand-rolled DNS message parsing instead of Network.framework. +/// 2. `handleResponse` resolved on whichever record type it encountered +/// first in the response, with no preference — an AAAA record appearing +/// before an A record in the same packet would silently win. This +/// library prefers IPv4 throughout (see `MDNSApple`'s equivalent +/// preference in `AppleServiceDiscovery.resolve`); the Linux backend +/// should not have been the odd one out. +@Suite("LinuxServiceDiscovery.resolvableHosts — A/AAAA formatting and ordering") +struct LinuxServiceDiscoveryResolveTests { + @Test("an A record formats as a plain, URL-safe dotted-quad host") + func aRecordFormatsPlain() throws { + let record = DNSRecord(name: "shelly1pmg4-abc123.local", type: .a, ttl: 120, rdata: .a(Data([192, 168, 1, 23]))) + let results = LinuxServiceDiscovery.resolvableHosts(from: [record]) + #expect(results.count == 1) + #expect(results[0].recordName == "shelly1pmg4-abc123.local") + #expect(results[0].host == "192.168.1.23") + try #require(urlBuilds(fromHost: results[0].host)) + } + + @Test("an AAAA record formats as a bracketed, URL-safe IPv6 host") + func aaaaRecordFormatsBracketed() throws { + let bytes = Data([0xfe, 0x80, 0, 0, 0, 0, 0, 0, 0x12, 0x34, 0x56, 0x78, 0x9a, 0xbc, 0xde, 0xf0]) + let record = DNSRecord(name: "shelly1pmg4-abc123.local", type: .aaaa, ttl: 120, rdata: .aaaa(bytes)) + let results = LinuxServiceDiscovery.resolvableHosts(from: [record]) + #expect(results.count == 1) + #expect(results[0].host == "[fe80:0:0:0:1234:5678:9abc:def0]") + try #require(urlBuilds(fromHost: results[0].host)) + } + + @Test("when both an A and AAAA record are present, the A-derived host is ordered first regardless of packet order") + func preferOrdersARecordFirst() { + let name = "shelly1pmg4-abc123.local" + let aaaaRecord = DNSRecord( + name: name, type: .aaaa, ttl: 120, + rdata: .aaaa(Data([0xfe, 0x80, 0, 0, 0, 0, 0, 0, 0x12, 0x34, 0x56, 0x78, 0x9a, 0xbc, 0xde, 0xf0])) + ) + let aRecord = DNSRecord(name: name, type: .a, ttl: 120, rdata: .a(Data([192, 168, 1, 23]))) + + // AAAA listed BEFORE A in the input, matching a packet where the + // responder happened to put the AAAA answer first. + let results = LinuxServiceDiscovery.resolvableHosts(from: [aaaaRecord, aRecord]) + + #expect(results.count == 2) + #expect(results[0].host == "192.168.1.23") // A wins the race regardless of input order + #expect(results[1].host == "[fe80:0:0:0:1234:5678:9abc:def0]") + } + + @Test("malformed byte counts are skipped rather than producing a garbled host") + func malformedByteCountsSkipped() { + let shortA = DNSRecord(name: "x.local", type: .a, ttl: 120, rdata: .a(Data([192, 168, 1]))) + let shortAAAA = DNSRecord(name: "x.local", type: .aaaa, ttl: 120, rdata: .aaaa(Data([0xfe, 0x80]))) + #expect(LinuxServiceDiscovery.resolvableHosts(from: [shortA, shortAAAA]).isEmpty) + } + + @Test("non-address records (e.g. PTR) contribute nothing") + func nonAddressRecordsIgnored() { + let ptr = DNSRecord(name: "_shelly._tcp.local", type: .ptr, ttl: 120, rdata: .ptr(domain: "shelly1.local")) + #expect(LinuxServiceDiscovery.resolvableHosts(from: [ptr]).isEmpty) + } + + private func urlBuilds(fromHost host: String) -> Bool { + var components = URLComponents() + components.scheme = "http" + components.host = host + components.port = 80 + components.path = "/rpc" + return components.url != nil + } +}