KTOR-8490 Add SocketAddress::address #4857
Conversation
|
""" WalkthroughA new function, address(), was introduced to the InetSocketAddress class across common, JVM, and non-JVM source sets. This function provides the raw byte representation of the socket address, with platform-specific implementations for JVM and non-JVM environments. No other properties or logic were modified. Changes
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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.
| public actual fun address(): ByteArray? { | ||
| val maybeIPv4 = hostname.contains('.') | ||
| val maybeIPv6 = hostname.contains(':') | ||
|
|
||
| if (maybeIPv4) { | ||
| try { | ||
| val octets = hostname.split('.', limit = 4).map { it.toInt().toByte() } | ||
| return List(4) { octets.getOrElse(it) { 0 } }.toByteArray() | ||
| } catch (_: Throwable) {} | ||
| } | ||
|
|
||
| if (maybeIPv6) { | ||
| try { | ||
| val groups = hostname.split(':', limit = 8) | ||
| val emptyGroups = 8 - groups.count { it.isNotEmpty() } | ||
|
|
||
| val bytes = groups.flatMap { | ||
| if (it.isEmpty()) { | ||
| List(emptyGroups * 2) { 0 } | ||
| } else { | ||
| val int = it.toInt(16) | ||
| listOf((int shr 8).toByte(), int.toByte()) | ||
| } | ||
| } | ||
| return List(16) { bytes.getOrElse(it) { 0 } }.toByteArray() | ||
| } catch (_: Throwable) {} | ||
| } | ||
|
|
||
| return null | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
IPv4/IPv6 parsing issues
The current IP address parsing has several potential issues:
-
The detection heuristic using
.contains()is too simplistic:- Domain names can contain dots but aren't IPv4 addresses
- Some strings might contain colons but aren't IPv6 addresses
-
IPv6 parsing doesn't handle:
- Mixed IPv4-IPv6 notation (e.g.,
::ffff:192.0.2.1) - Multiple consecutive empty groups
- Out-of-range hexadecimal values
- Mixed IPv4-IPv6 notation (e.g.,
-
IPv4 parsing doesn't validate if octets are in range (0-255)
Additionally, catching all throwables silently could mask unexpected errors.
Consider a more robust implementation:
public actual fun address(): ByteArray? {
- val maybeIPv4 = hostname.contains('.')
- val maybeIPv6 = hostname.contains(':')
-
- if (maybeIPv4) {
+ // Try IPv4 format (more specific check with regex)
+ if (hostname.matches(Regex("\\d{1,3}(\\.\\d{1,3}){3}"))) {
try {
- val octets = hostname.split('.', limit = 4).map { it.toInt().toByte() }
+ val octets = hostname.split('.', limit = 4).map {
+ val value = it.toInt()
+ if (value in 0..255) value.toByte() else throw IllegalArgumentException("IPv4 octet out of range: $value")
+ }
return List(4) { octets.getOrElse(it) { 0 } }.toByteArray()
- } catch (_: Throwable) {}
+ } catch (e: NumberFormatException) {
+ // Not a valid IPv4 address
+ } catch (e: IllegalArgumentException) {
+ // Not a valid IPv4 address
+ }
}
- if (maybeIPv6) {
+ // Try IPv6 format (more specific check)
+ if (hostname.contains(':')) {
try {
- val groups = hostname.split(':', limit = 8)
- val emptyGroups = 8 - groups.count { it.isNotEmpty() }
+ // Handle compressed notation (::)
+ val compressed = hostname.indexOf("::")
+ val rawGroups = if (compressed >= 0) {
+ val before = hostname.substring(0, compressed).split(':')
+ val after = hostname.substring(compressed + 2).split(':')
+ val emptyGroups = 8 - (before.size + after.size)
+ before + List(emptyGroups) { "" } + after
+ } else {
+ hostname.split(':')
+ }
+
+ val bytes = mutableListOf<Byte>()
+ for (group in rawGroups) {
+ if (group.isEmpty()) continue
+
+ // Handle embedded IPv4
+ if (group.contains('.')) {
+ val ipv4Parts = group.split('.').map {
+ val value = it.toInt()
+ if (value in 0..255) value.toByte() else throw IllegalArgumentException()
+ }
+ bytes.addAll(ipv4Parts)
+ continue
+ }
- val bytes = groups.flatMap {
- if (it.isEmpty()) {
- List(emptyGroups * 2) { 0 }
- } else {
- val int = it.toInt(16)
- listOf((int shr 8).toByte(), int.toByte())
- }
+ val value = group.toInt(16)
+ if (value > 0xFFFF) throw IllegalArgumentException("IPv6 group out of range: $value")
+ bytes.add((value shr 8).toByte())
+ bytes.add(value.toByte())
}
- return List(16) { bytes.getOrElse(it) { 0 } }.toByteArray()
- } catch (_: Throwable) {}
+
+ // Ensure we have 16 bytes for IPv6
+ return List(16) { bytes.getOrElse(it) { 0 } }.toByteArray()
+ } catch (e: NumberFormatException) {
+ // Not a valid IPv6 address
+ } catch (e: IllegalArgumentException) {
+ // Not a valid IPv6 address
+ }
}
return null
}This is a complex area and you might want to consider using a dedicated IP address parsing library instead of implementing it from scratch.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public actual fun address(): ByteArray? { | |
| val maybeIPv4 = hostname.contains('.') | |
| val maybeIPv6 = hostname.contains(':') | |
| if (maybeIPv4) { | |
| try { | |
| val octets = hostname.split('.', limit = 4).map { it.toInt().toByte() } | |
| return List(4) { octets.getOrElse(it) { 0 } }.toByteArray() | |
| } catch (_: Throwable) {} | |
| } | |
| if (maybeIPv6) { | |
| try { | |
| val groups = hostname.split(':', limit = 8) | |
| val emptyGroups = 8 - groups.count { it.isNotEmpty() } | |
| val bytes = groups.flatMap { | |
| if (it.isEmpty()) { | |
| List(emptyGroups * 2) { 0 } | |
| } else { | |
| val int = it.toInt(16) | |
| listOf((int shr 8).toByte(), int.toByte()) | |
| } | |
| } | |
| return List(16) { bytes.getOrElse(it) { 0 } }.toByteArray() | |
| } catch (_: Throwable) {} | |
| } | |
| return null | |
| } | |
| public actual fun address(): ByteArray? { | |
| // Try IPv4 format (more specific check with regex) | |
| if (hostname.matches(Regex("\\d{1,3}(\\.\\d{1,3}){3}"))) { | |
| try { | |
| val octets = hostname.split('.', limit = 4).map { | |
| val value = it.toInt() | |
| if (value in 0..255) value.toByte() | |
| else throw IllegalArgumentException("IPv4 octet out of range: $value") | |
| } | |
| return List(4) { octets.getOrElse(it) { 0 } }.toByteArray() | |
| } catch (e: NumberFormatException) { | |
| // Not a valid IPv4 address | |
| } catch (e: IllegalArgumentException) { | |
| // Not a valid IPv4 address | |
| } | |
| } | |
| // Try IPv6 format (more specific check) | |
| if (hostname.contains(':')) { | |
| try { | |
| // Handle compressed notation (::) | |
| val compressed = hostname.indexOf("::") | |
| val rawGroups = if (compressed >= 0) { | |
| val before = hostname.substring(0, compressed).split(':') | |
| val after = hostname.substring(compressed + 2).split(':') | |
| val emptyGroups = 8 - (before.size + after.size) | |
| before + List(emptyGroups) { "" } + after | |
| } else { | |
| hostname.split(':') | |
| } | |
| val bytes = mutableListOf<Byte>() | |
| for (group in rawGroups) { | |
| if (group.isEmpty()) continue | |
| // Handle embedded IPv4 | |
| if (group.contains('.')) { | |
| val ipv4Parts = group.split('.').map { | |
| val value = it.toInt() | |
| if (value in 0..255) value.toByte() | |
| else throw IllegalArgumentException("IPv4 octet out of range: $value") | |
| } | |
| bytes.addAll(ipv4Parts) | |
| continue | |
| } | |
| val value = group.toInt(16) | |
| if (value > 0xFFFF) throw IllegalArgumentException("IPv6 group out of range: $value") | |
| bytes.add((value shr 8).toByte()) | |
| bytes.add(value.toByte()) | |
| } | |
| // Ensure we have exactly 16 bytes for IPv6 | |
| return List(16) { bytes.getOrElse(it) { 0 } }.toByteArray() | |
| } catch (e: NumberFormatException) { | |
| // Not a valid IPv6 address | |
| } catch (e: IllegalArgumentException) { | |
| // Not a valid IPv6 address | |
| } | |
| } | |
| return null | |
| } |
Subsystem
ktor-network
Motivation
KTOR-8490 Support accessing resolved IP address on an instance of
io.ktor.network.sockets.InetSocketAddressSolution
For JVM,
InetAddress.getAddress()is used, for non-JVM, it is manually parsed from thehostnameparameter.