On Sun, 17 Sep 2023 13:38:08 GMT, Aleksei Efimov <[email protected]> wrote:
> ### Summary
>
> The changes in this PR add new API to `java.net.InetAddress`,
> `java.net.Inet4Address`, and
> `java.net.Inet6Address` classes to parse IP address literals:
> ```
> method public static java.net.InetAddress
> java.net.InetAddress.ofLiteral(java.lang.String)
> method public static java.net.Inet4Address
> java.net.Inet4Address.ofLiteral(java.lang.String)
> method public static java.net.Inet6Address
> java.net.Inet6Address.ofLiteral(java.lang.String)
> ```
>
> ### How new methods differ from existing ones
>
> These methods differ from `InetAddress.getByName` and
> `InetAddress.getAllByName` in the following ways:
> 1. Address literal strings are not forwarded to the system-wide resolver -
> they are only parsed and validated by the internal methods.
> 2. No reverse lookup is performed to resolve a hostname for the supplied
> address literal - the `InetAddress[46 ]` instances returned by the new
> `ofLiteral` API has no hostname set.
> 3. Each `ofLiteral` static method returns addresses of its class only. It
> gives the ability to check if an IP address literal is of a specific address
> type.
>
> ### The list of noteworthy changes
> - `IPv4-mapped IPv6 address` and `IPv4-compatible IPv6 addresses` require
> some special handling in the new API to implement all supported IP address
> types.
> - All address literal parsing code has been moved from
> `InetAddress.getAllByName` to address type-specific
> `Inet4Address.parseAddressString` and `Inet6Address.parseAddressString`
> methods.
> - The text with scoped IPv6 addresses architecture draft IETF file has been
> replaced from `[draft-ietf-ipngwg-scoping-arch-04.txt]` to reference `RFC
> 4007: IPv6 Scoped Address Architecture`. The "RFC 4007" has been also added
> as `@ spec` into Inet6Address class-level Javadoc.
>
> ### Testing
>
> `jdk-tier1`, `jdk-tier2`, and `jdk-tier3` test sets show no failure with the
> changes.
>
> `java/net` JCK tests are failing with new methods added failure (CSR is
> planned for this change):
>
> Added Methods
> -------------
>
> java.net.Inet4Address: method public static
> java.net.Inet4Address java.net.Inet4Address.ofLiteral(java.lang.String)
> java.net.Inet6Address: method public static
> java.net.Inet6Address java.net.Inet6Address.ofLiteral(java.lang.String)
> java.net.InetAddress: method public static
> java.net.InetAddress java.net.InetAddress.ofLiteral(java.lang.String)
Good work Aleksei, certainly enough to get the discussion going :-)
I'm not entirely sure of some aspects of this proposal though. See my other
comments inline.
src/java.base/share/classes/java/net/Inet4Address.java line 148:
> 146: * <blockquote><ul style="list-style-type:none">
> 147: * <li>{@code 1.2.3.4}</li>
> 148: * <li>{@code 06.07.08.09}</li>
It might be clearer to transform this in a kind of JShell like snippet, showing
the Inet4Adress call and what is returned? I would also expect to see an
example of IPv4-compatible IPv6 address literal here.
Something like:
* {@snippet:
* Inet4Address.ofLiteral("1.2.3.4") ==> /1.2.3.4
* Inet4Address.ofLiteral("06.07.08.09") ==> /6.7.8.9
* Inet4Address.ofLiteral(""::ffff:1020:3040") ==> /16.32.48.64
* }
As a side note, I suspect that Inet4Adress.ofLiteral() accepting
IPv4-compatible IPv6 addresses will be surprising for most users. I wonder if
we should change that, and have Inet6Address.ofLiteral() return InetAddress
instead of Inet6Address, and take care of that edge case.
As a user of the API I could very well see myself trying to do something like:
if (addr.startsWith("[") && addr.endsWith("]")) {
var ipv6 = addr.substring(1, addr.length() - 1);
try {
Inet6Address.ofLiteral(ipv6);
} catch (...) {
throw new IllegalArgumentException("Invalid IPv6 literal between "[
]"");
}
}
I'm not saying we shouldn't do what you're suggesting here, but I believe we
should get more feedback.
src/java.base/share/classes/java/net/Inet6Address.java line 490:
> 488: * java.net.spi.InetAddressResolver resolver} is not queried to
> resolve
> 489: * the provided literal, and no reverse lookup is performed.
> 490: * @implNote <a
> href="Inet6Address.html#special-ipv6-address-heading">
`@implNote` is not appropriate here. This should be part of the spec (if we
keep it). See my other comment above in `Inet4Address`.
-------------
PR Review: https://git.openjdk.org/jdk/pull/15775#pullrequestreview-1631177877
PR Review Comment: https://git.openjdk.org/jdk/pull/15775#discussion_r1328799014
PR Review Comment: https://git.openjdk.org/jdk/pull/15775#discussion_r1328804396
