On Wed, 10 Apr 2024 15:30:52 GMT, Sergey Chernyshev <schernys...@openjdk.org> wrote:
>> 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"? @sercher the word "loose" is still there - it's not removed? >> 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. I would rather move the description of the loose syntax fully here, since the only method that accepts it is `Inet4Address.ofPosixLiteral`. We need to be absolutely clear that this is the only method that accepts it. Also, I feel that a link to an external document is not sufficient in this case. It's good to have it, but since we actually implement the parsing in java we should describe clearly the accepted syntax and how we parse it here. We have no control over external documents, they can move, their content can change, etc... I believe that the anchor `Inet4Address##format-posix` should lead to this method here, not to the class-level documentation. Having a short paragraph in the class level documentation that explains that an alternative loose syntax is accepted by a specific method is fine. However I don't feel that describing this loose syntax in the class level API documentation is a good idea. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18493#discussion_r1560697697 PR Review Comment: https://git.openjdk.org/jdk/pull/18493#discussion_r1560705046
