On Mon, 9 Oct 2023 07:00:08 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> Hello Aleksei, it's good to see the addition of these new APIs. On the naming > front, I like the Hello Jaikiran > The proposed changes also have text which talks about "ambiguous" address > literals. The term ambiguous seems new in the InetAddress (and sub-classes). > The package-info.java too doesn't have previous references to it. What ambiguous IPv4 address literals mean in the code and in [the release notes](https://www.oracle.com/java/technologies/javase/18all-relnotes.html#JDK-8277608): it is a set of addresses which can't be parsed in dotted-decimal forms listed in the Inet4Address#format class-level javadoc section, but can be parsed in BSD forms (decimal, octal, or hexadecimal) defined in [`inet_addr` of IEEE Std 1003.1-2017](https://pubs.opengroup.org/onlinepubs/9699919799/functions/inet_addr.html). > Should we add some details on what addresses are considered ambiguous (and > how/if such addresses are treated by existing APIs)? The release note states that the `InetAddress` accepts IPv4 address literals in decimal quad notation. The new `ofLiteral` API also states that only dotted-decimal forms are supported, `ambigous` term is not mentioned in any public javadoc, and given that the related system property is a jdk-specific system property I would like to avoid introducing this term as part of this PR. > do you think we should add a more formal reference to the BSD formatting of > IP address I think a more formal reference to BSD forms might be needed if they are recognizable by InetAddress APIs. As of today, this is not the case. > src/java.base/share/classes/java/net/Inet4Address.java line 181: > >> 179: /** >> 180: * Parses string with an IPv4 address literal. >> 181: * If string doesn't contain a valid literal - null is returned. > > This sentence is a bit confusing, given the implementation details of this > method. I think `null` is only returned if `throwIAE` is false? Thank you for spotting it. I have updated the javadoc of the `parseAddressString` method to contain all three possible scenarios: * If string contains a non-parsable literal and {@code throwIAE} is set to {@code false} * - {@code null} is returned. * If string contains a non-parsable literal and {@code throwIAE} is set to {@code true} * - {@code IllegalArgumentException} is thrown. * If string contains an {@linkplain IPAddressUtil#validateNumericFormatV4(String, boolean) * ambiguous literal} - {@code IllegalArgumentException} is thrown irrelevant to * {@code throwIAE} value. > src/java.base/share/classes/sun/net/util/IPAddressUtil.java line 141: > >> 139: * @param src input string >> 140: * @param throwIAE {@code "true"} - throw {@code >> IllegalArgumentException} when cannot be parsed as IPv4 address string; >> 141: * {@code "false"} - throw {@code >> "IllegalArgumentException"} only when IPv4 address string is ambiguous. > > Are these double quotes around `true`, `false`, `null` and > `IllegalArgumentException` intentional? It seems odd to have those double > quotes when (rightly) using `{@code}`. Nope, there are not intentional - a leftover from the fix prototyping stage. Will remove them. ------------- PR Comment: https://git.openjdk.org/jdk/pull/15775#issuecomment-1755647174 PR Review Comment: https://git.openjdk.org/jdk/pull/15775#discussion_r1352771068 PR Review Comment: https://git.openjdk.org/jdk/pull/15775#discussion_r1352772971
