On Tue, 9 Apr 2024 13:35:00 GMT, Daniel Fuchs <dfu...@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
