KTOR-8490 Add InetSocketAddress::resolveAddress #4857
Conversation
## Walkthrough
A new function `resolveAddress()` was added to the `InetSocketAddress` expect class to return the raw IP address as a nullable byte array. Platform-specific actual implementations were provided for JVM, POSIX, and JS/WASM targets, with JS/WASM returning null. Helper functions to parse IPv4 and IPv6 strings were added. Corresponding API declarations were also added. A test class was introduced to verify the correctness of the IP string parsing functions.
## Changes
| Files | Change Summary |
|---------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------|
| ktor-network/common/src/io/ktor/network/sockets/SocketAddress.kt | Added expect function `resolveAddress()` returning `ByteArray?` to `InetSocketAddress`. |
| ktor-network/jvm/src/io/ktor/network/sockets/SocketAddressJvm.kt | Added actual implementation of `resolveAddress()` returning Java InetAddress bytes. |
| ktor-network/nonJvm/src/io/ktor/network/sockets/SocketAddress.nonJvm.kt | Added actual `resolveAddress()` delegating to internal expect extension `platformResolveAddress()`. |
| ktor-network/jsAndWasmShared/src/io/ktor/network/sockets/SocketAddress.nonJvm.jsAndWasmShared.kt | Added actual `platformResolveAddress()` returning `null` for JS/WASM platform. |
| ktor-network/posix/src/io/ktor/network/sockets/SocketAddress.nonJvm.posix.kt | Added actual `platformResolveAddress()` resolving IPv4/IPv6 addresses to byte arrays for POSIX platforms. |
| ktor-network/posix/src/io/ktor/network/util/SocketAddressUtils.kt | Added internal functions `parseIPv4String` and `parseIPv6String` to parse IP address strings into byte arrays. |
| ktor-network/posix/test/io/ktor/network/sockets/tests/IPStringParseTest.kt | Added test class `IPStringParseTest` with tests for `parseIPv4String` and `parseIPv6String` functions. |
| ktor-network/api/ktor-network.api, ktor-network/api/ktor-network.klib.api | Added final method `resolveAddress()` returning `ByteArray?` to `InetSocketAddress` class API. |📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ktor-network/common/src/io/ktor/network/sockets/SocketAddress.kt (1)
51-51: Missing documentationThe new
address()function lacks KDoc documentation. Consider adding documentation that explains:
- The function's purpose (retrieving the raw IP address bytes)
- Return value meaning (null when address cannot be resolved)
- Format of the returned ByteArray (IPv4 vs IPv6)
+ /** + * Returns the raw IP address bytes of this socket address. + * + * The returned array is 4 bytes for IPv4 addresses and 16 bytes for IPv6 addresses. + * Returns null if the address cannot be resolved or is not a valid IP address. + * + * [Report a problem](https://ktor.io/feedback/?fqname=io.ktor.network.sockets.InetSocketAddress.address) + */ public fun address(): ByteArray?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ktor-network/common/src/io/ktor/network/sockets/SocketAddress.kt(1 hunks)ktor-network/jvm/src/io/ktor/network/sockets/SocketAddressJvm.kt(1 hunks)ktor-network/nonJvm/src/io/ktor/network/sockets/SocketAddress.nonJvm.kt(1 hunks)
🔇 Additional comments (1)
ktor-network/jvm/src/io/ktor/network/sockets/SocketAddressJvm.kt (1)
22-22: Implementation looks goodThe JVM implementation of
address()elegantly leverages the underlying JavaInetSocketAddressand correctly uses safe call operator (?.) to handle potential null values.
ktor-network/nonJvm/src/io/ktor/network/sockets/SocketAddress.nonJvm.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Thank you for your contribution!
Parsing IP addresses from a hostname is an interesting idea. However, I'm concerned it could introduce inconsistency in behavior across platforms. Addresses with domain names in the hostname would always return null on all platforms except JVM.
Please check the review comments where I've described a way to get an actual IP address on native platforms.
ktor-network/common/src/io/ktor/network/sockets/SocketAddress.kt
Outdated
Show resolved
Hide resolved
ktor-network/nonJvm/src/io/ktor/network/sockets/SocketAddress.nonJvm.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
ktor-network/common/src/io/ktor/network/sockets/SocketAddress.kt(1 hunks)ktor-network/jsAndWasmShared/src/io/ktor/network/sockets/SocketAddress.nonJvm.jsAndWasmShared.kt(1 hunks)ktor-network/jvm/src/io/ktor/network/sockets/SocketAddressJvm.kt(1 hunks)ktor-network/nonJvm/src/io/ktor/network/sockets/SocketAddress.nonJvm.kt(2 hunks)ktor-network/posix/src/io/ktor/network/sockets/SocketAddress.nonJvm.posix.kt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- ktor-network/jsAndWasmShared/src/io/ktor/network/sockets/SocketAddress.nonJvm.jsAndWasmShared.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- ktor-network/jvm/src/io/ktor/network/sockets/SocketAddressJvm.kt
- ktor-network/nonJvm/src/io/ktor/network/sockets/SocketAddress.nonJvm.kt
- ktor-network/common/src/io/ktor/network/sockets/SocketAddress.kt
🔇 Additional comments (2)
ktor-network/posix/src/io/ktor/network/sockets/SocketAddress.nonJvm.posix.kt (2)
11-12: LGTM! Clean function signature and delegation pattern.The function properly delegates to the native resolution mechanism and handles the null case gracefully.
14-26: IPv4 parsing implementation looks correct.The IPv4 parsing logic correctly:
- Splits the IP string into octets with a proper limit
- Converts each octet to unsigned byte then to signed byte (appropriate for network byte order)
- Handles malformed inputs with padding to ensure a 4-byte result
- Uses proper exception handling
ktor-network/posix/src/io/ktor/network/sockets/SocketAddress.nonJvm.posix.kt
Outdated
Show resolved
Hide resolved
ktor-network/posix/src/io/ktor/network/sockets/SocketAddress.nonJvm.posix.kt
Outdated
Show resolved
Hide resolved
ktor-network/posix/src/io/ktor/network/sockets/SocketAddress.nonJvm.posix.kt
Outdated
Show resolved
Hide resolved
ktor-network/posix/src/io/ktor/network/sockets/SocketAddress.nonJvm.posix.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ktor-network/posix/src/io/ktor/network/sockets/SocketAddress.nonJvm.posix.kt(1 hunks)ktor-network/posix/src/io/ktor/network/util/SocketAddressUtils.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ktor-network/posix/src/io/ktor/network/sockets/SocketAddress.nonJvm.posix.kt
ktor-network/posix/src/io/ktor/network/util/SocketAddressUtils.kt
Outdated
Show resolved
Hide resolved
ktor-network/posix/src/io/ktor/network/util/SocketAddressUtils.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
ktor-network/posix/test/io/ktor/network/sockets/tests/IPStringParseTest.kt (1)
63-69: Good invalid IPv6 test cases.The invalid cases appropriately test malformed syntax, insufficient groups, cross-format confusion, and negative values.
Consider adding these additional edge cases for more comprehensive coverage:
+ "", + ":::", + "12345::1", + "g::1"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ktor-network/posix/test/io/ktor/network/sockets/tests/IPStringParseTest.kt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
ktor-network/posix/test/io/ktor/network/sockets/tests/IPStringParseTest.kt (1)
ktor-network/posix/src/io/ktor/network/util/SocketAddressUtils.kt (2)
parseIPv4String(23-33)parseIPv6String(35-62)
🔇 Additional comments (4)
ktor-network/posix/test/io/ktor/network/sockets/tests/IPStringParseTest.kt (4)
1-12: LGTM: Clean imports and package structure.The file header, package declaration, and imports are well-structured and appropriate for the test functionality.
16-20: LGTM: Comprehensive IPv4 test cases with proper byte conversion.The valid test cases cover important scenarios including edge values (0.0.0.0, 255.255.255.255) and common addresses (127.0.0.1). The byte conversion using
.toByte()correctly handles the signed byte representation for values > 127.
22-28: Good edge case coverage for invalid IPv4 addresses.The invalid test cases appropriately cover negative values, out-of-range values, malformed strings, and cross-format confusion (IPv6 syntax in IPv4 parser).
43-61: Excellent IPv6 test coverage with accurate byte representations.The test cases cover crucial IPv6 scenarios:
- Zero compression (
::)- Mixed compression (
2001:0db8:85a3::8a2e:0370:7334)- Link-local addresses (
fe80::1)- Loopback (
::1)The byte array representations are correctly calculated, properly handling 16-bit group conversion to high/low byte pairs.
ktor-network/posix/test/io/ktor/network/sockets/tests/IPStringParseTest.kt
Outdated
Show resolved
Hide resolved
ktor-network/posix/test/io/ktor/network/sockets/tests/IPStringParseTest.kt
Outdated
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
OmniacDev
changed the title
OmniacDev
changed the title
|
🥳 |
|
Just a moment after I merged this PR I realized that we already have byte-representation of IP address in |
Subsystem
ktor-network
Motivation
KTOR-8490 Support accessing resolved IP address on an instance of
io.ktor.network.sockets.InetSocketAddressSolution
InetAddress.getAddress()is used.