Reject interface-scoped addresses; prefer IPv4 in mDNS resolve

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<NetworkAddress, Error>) 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<NetworkAddress, Error>) 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()!
         }
     }
 

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 `%<interfaceName>` 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 "`<name>.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:

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) {