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)

At a. high-level, introducing static methods named "ofLiteral" to parse a 
string as an IPv4 or IPv6 literal address is good. My guess is that the methods 
will require work on the class descriptions, speak bout parsing the input 
string, and only mention "blocking" in the context of specifying that they 
don't attempt reverse name resolution. My guess is that the API additions will 
require a bit of re-work of the class descriptions, esp. as they are specified 
in terms of "textual representation".

On Inet4Address.ofLiteral, I can't tell from the method description if it 
supports all 4 forms that are listed in the class description. It may a 
surprise that this method parses IPv4-mapped IPv6 addresses. So I think my main 
comment here is that the method description will need to more completely 
specify the forms that the input string may take.

Similar comment on Inet6Address.ofLiteral. I can't tell from the API docs if 
the input needs enclosing square brackets, I can't tell if it supports 
shortened forms, I can't tell if it's legal or not to include a scope ID. So I 
think my comment is just that you'll probably go through a few iterations to 
fully specify the forms that the input string can take.

InetAddress.ofLiteral doesn't need "throws IAE" in the method signature as it's 
an unchecked exception. I think this method will end up linking to the other 
two, and specifying that it first attempts to parse the input as an IPv4 
address.

My comments here are just about the API for now, I didn't look at the 
implementation or tests at the time.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/15775#issuecomment-1722501929

Reply via email to