RFR: 8315767: InetAddress: constructing objects from BSD literal addresses [v2]

[Sergey Chernyshev]

On Tue, 9 Apr 2024 13:35:00 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> Sergey Chernyshev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   addressed review comments in Javadoc
>
> src/java.base/share/classes/java/net/Inet4Address.java line 103:
> 
>> 101:  * octal and hexadecimal address segments. Please refer to
>> 102:  * <a href="https://www.ietf.org/rfc/rfc6943.html#section-3.1.1"> <i>RFC 
>> 103:  * 6943: Issues in Identifier Comparison for Security Purposes</i></a>.
> 
> Suggestion:
> 
>  * <p> The above forms adhere "strict" decimal-only syntax.
>  * Additionally, the
>  * {@link Inet4Address#ofPosixLiteral(String)} method implements a
>  * <a href="https://pubs.opengroup.org/onlinepubs/9699919799/functions/inet_addr.html">
>  * POSIX {@code inet_addr}</a> compatible "loose" parsing algorithm, allowing
>  * octal and hexadecimal address segments. Please refer to
>  * <a href="https://www.ietf.org/rfc/rfc6943.html#section-3.1.1"> <i>RFC 
>  * 6943: Issues in Identifier Comparison for Security Purposes</i></a>.

This removes the reference of loose syntax that was opposed to strict syntax. Would you think to also remove the word "strict"?

> src/java.base/share/classes/java/net/Inet4Address.java line 124:
> 
>> 122:  *  // the constructed address bytes without any rearrangement
>> 123:  *  Inet4Address.ofPosixLiteral("017700000001"); // ==> /127.0.0.1
>> 124:  * }
> 
> Please move this part into the normative part of the `ofPosixLiteral` method description

Hi Daniel, thank you for your notes. This part is logical continuation of the sample fragment starting at line 76 of the class spec. I'd rather leave it as is, otherwise it would be the same problem with the previous fragment with `ofLiteral()` and `getByName()` examples. I avoided in-depth syntax description in the method header, preferred adding a link instead. Please have a look at the compiled HTML docs.

> src/java.base/share/classes/java/net/Inet4Address.java line 224:
> 
>> 222:      * Inet4Address##format valid IPv4 address} an {@code IllegalArgumentException} is thrown.
>> 223:      * <p> This method doesn't block, i.e. no hostname lookup is performed.
>> 224:      *
> 
> Please describe the syntax this metod accepts here, in the normative part of the specification.

I updated the docs to link to the class specification, also added an anchor in the spec.

> src/java.base/share/classes/java/net/Inet4Address.java line 232:
> 
>> 230:      * {@code 255} for {@code "0255"}, {@linkplain Inet4Address#ofPosixLiteral this}
>> 231:      * method interprets the numbers based on their prefix (hexadecimal {@code "0x"},
>> 232:      * octal {@code "0"}) and returns {@code 173} for {@code "0255"}.
> 
> Suggestion:
> 
>      * when {@code posixIPAddressLiteral} parameter contains address segments with
>      * leading zeroes. An address segment with a leading zero is always parsed as an octal
>      * number by {@code  ofPosixLiteral()}, therefore {@code 0255} (octal) will be parsed as
>      * {@code 173} (decimal) by this method. On the other hand, {@link Inet4Address#ofLiteral
>      * Inet4Address.ofLiteral} ignores leading zeros, parses all numbers as decimal and produces
>      * {@code 255}. Where this method would parse {@code 0256.0256.0256.0256} (octal) and 
>      * produce {@code 174.174.174.174} (decimal) in four dotted quad notation, 
>      * {@code Inet4Address.ofLiteral} will throw {@code IllegalArgumentException}.

Ok, accepted. I will put this together with other suggestions in a separate commit (this exact text above had some minor whitespace issues).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18493#discussion_r1559668820
PR Review Comment: https://git.openjdk.org/jdk/pull/18493#discussion_r1559668110
PR Review Comment: https://git.openjdk.org/jdk/pull/18493#discussion_r1559670713
PR Review Comment: https://git.openjdk.org/jdk/pull/18493#discussion_r1559669723