On Sun, 17 Sep 2023 13:38:08 GMT, Aleksei Efimov <aefi...@openjdk.org> 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.InetAddress 
> 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. If a string supplied is not an address literal it is not forwarded to the 
> system-wide resolver, but IllegalArgumentException is thrown instead. The 
> system-wide resolver is never called from these new 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.InetAddress java.net.Inet6Address.ofLiteral(java.lang.String)
> java.net.InetAddress:                   method public static 
> java.net.InetAddress java.net.InetAddress.ofLiteral(java.lang.String)

I'd have a mild preference for the new name rather than overloading the 
existing `getByAddress`. With IDE auto completion, I think a distinct name is a 
bit clearer for the developer as to what exactly the method will do. 

I think if it can be specified and implemented that `InetAddress.ofLiteral` 
calls the sub-type variants (based on string length maybe) then I think it 
makes sense to specify the parsing details in the sub-classes. If not, then I'm 
not sure we need the new methods in the sub-classes.

src/java.base/share/classes/java/net/Inet4Address.java line 163:

> 161:      *  Inet4Address.ofLiteral("02130706689") ==> /127.0.1.1
> 162:      * }
> 163:      * <p>If the provided address literal cannot represent a valid IPv4 
> address an

I think this behavior is the same as existing methods, except that only the 
decimal format is supported? I wonder is it really necessary to describe it 
here, as it it would be quite an arcane usage, and the section in the class doc 
"Textual representation if IP addresses" already partially covers it. The 
snippet example with the leading zero seems to be implying that octal format is 
not supported, but without really saying it out loud. 

Maybe the apidoc for the method could just say that the decimal formats are the 
only ones supported, but otherwise refer to the class docs for how the 
different variants work ? And maybe move the snippet there as well?

src/java.base/share/classes/java/net/Inet6Address.java line 525:

> 523:      *         parsed as an IPv6 address literal.
> 524:      * @throws NullPointerException if the {@code ipv6AddressLiteral} is 
> {@code null}.
> 525:      */

Similar comment to Inet4Address. The snippet should be incorporated into the 
general class docs section "Textual representation of IP addresses" and the 
apidoc here should limit itself to any limitations that only apply to this 
method imo.

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

PR Comment: https://git.openjdk.org/jdk/pull/15775#issuecomment-1725280222
PR Review Comment: https://git.openjdk.org/jdk/pull/15775#discussion_r1352797174
PR Review Comment: https://git.openjdk.org/jdk/pull/15775#discussion_r1352812605

Reply via email to