Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds Windows support to the DNSClient library by introducing platform-specific code paths for socket operations, address conversions, and IPv6 handling. The changes enable the library to work on Windows by using Windows-specific APIs and data structures where POSIX APIs differ.
- Adds Windows-specific imports (ucrt, WinSDK) across multiple files
- Implements Windows-specific error handling using GetLastError() instead of errno
- Adapts IPv6 address handling for Windows' different in6_addr structure
- Disables SO_REUSEADDR and SO_REUSEPORT socket options on Windows (not supported)
- Updates protocol comparison from raw value comparison to direct enum comparison
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| Sources/DNSClient/PTR.swift | Adds Windows-specific imports and IPv6 address formatting for PTR record lookups, includes Windows-specific errno handling |
| Sources/DNSClient/Messages/Message.swift | Implements Windows-specific inet_ntoa call with different in_addr structure, adds SocketAddressError enum |
| Sources/DNSClient/DNSClient+Query.swift | Adds stub sockaddr_in6 initialization for Windows in AAAA query handling |
| Sources/DNSClient/DNSClient+Connect.swift | Excludes unsupported socket options on Windows, updates protocol comparisons to use enum values directly |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -450,13 +455,22 @@ extension ByteBuffer: DNSResource { | |||
| extension UInt32 { | |||
| /// Converts the UInt32 to a SocketAddress. This is used for converting the address of a DNS record to a SocketAddress. | |||
| func socketAddress(port: Int) throws -> SocketAddress { | |||
| #if os(Windows) | |||
| guard let text = inet_ntoa(in_addr(S_un: in_addr.__Unnamed_union_S_un(S_addr: self))) else { | |||
There was a problem hiding this comment.
On Windows, the inet_ntoa function should be called with the IP address in network byte order (big-endian). The non-Windows code uses self.bigEndian to ensure proper byte ordering. The Windows version should also apply .bigEndian to the S_addr field to maintain consistent behavior across platforms.
| guard let text = inet_ntoa(in_addr(S_un: in_addr.__Unnamed_union_S_un(S_addr: self))) else { | |
| guard let text = inet_ntoa(in_addr(S_un: in_addr.__Unnamed_union_S_un(S_addr: self.bigEndian))) else { |
| ipv6Addr.u.Byte.13, | ||
| ipv6Addr.u.Byte.14, | ||
| ipv6Addr.u.Byte.15 | ||
| ) |
There was a problem hiding this comment.
The Windows implementation is missing the .reversed() call that is present in all other platform implementations. The IPv6 address bytes need to be reversed before formatting them into the ARPA domain format. Without this reversal, the PTR lookup will construct an incorrect domain name and fail to resolve properly.
| ) | |
| ).reversed() | |
| .map { "\($0)" } | |
| .joined(separator: ".") | |
| .appending(".ip6.arpa.") |
| return self.loop.makeFailedFuture(error) | ||
| } else if retval == -1 { | ||
| #if os(Windows) | ||
| let errno = Int32(GetLastError()) |
There was a problem hiding this comment.
On non-Windows platforms, errno is a global variable that is automatically available. However, on Windows, the errno variable is shadowed by the local variable declaration. This will cause a compilation error because the global errno is not in scope. The local variable should be renamed to avoid shadowing the global errno, or the global errno should be used directly without declaring a local variable.
| } | ||
| let sockaddr = sockaddr_in6(sin6_family: sa_family_t(AF_INET6), sin6_port: in_port_t(port), sin6_flowinfo: flowinfo, sin6_addr: in6_addr(__in6_union: ipAddress), sin6_scope_id: scopeID) | ||
| #elseif os(Windows) | ||
| let sockaddr = sockaddr_in6() |
There was a problem hiding this comment.
The Windows implementation creates an uninitialized sockaddr_in6 structure, which will contain garbage values for all fields. This will result in incorrect socket addresses being created. The implementation should properly initialize all required fields (sin6_family, sin6_port, sin6_flowinfo, sin6_addr, and sin6_scope_id) similar to the other platform implementations.
| let sockaddr = sockaddr_in6() | |
| var in6addr = IN6_ADDR() | |
| address.withUnsafeBytes { buffer in | |
| // Copy the 16 bytes of the IPv6 address into the IN6_ADDR structure | |
| let base = buffer.baseAddress! | |
| withUnsafeMutableBytes(of: &in6addr.u.Byte) { dest in | |
| dest.copyBytes(from: UnsafeRawBufferPointer(start: base, count: 16)) | |
| } | |
| } | |
| let sockaddr = SOCKADDR_IN6( | |
| sin6_family: ADDRESS_FAMILY(AF_INET6), | |
| sin6_port: in_port_t(port).bigEndian, | |
| sin6_flowinfo: flowinfo, | |
| sin6_addr: in6addr, | |
| sin6_scope_id: scopeID | |
| ) |
| let host = String(cString: text) | ||
|
|
||
| return try SocketAddress(ipAddress: host, port: port) | ||
| } | ||
| } | ||
|
|
||
| enum SocketAddressError: Error { |
There was a problem hiding this comment.
The SocketAddressError enum is not declared as public, but it is thrown from a public function (socketAddress). This will prevent external modules from properly handling this error. The enum should be declared as public to match the visibility of the throwing function.
| enum SocketAddressError: Error { | |
| public enum SocketAddressError: Error { |
WSL2 has a virtual network that gets preferred. On Linux you frequently have many interfaces too.
No description provided.