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)
Looking at the motivation for this addition in the budgd description
The requirement is that of providing a methof to parse of an string literal
address returns and return an InetAddress without necessitating a reverse
address lookup, which is what getByName does at the minute.
The additional motivation, appears to be to avoid the misinterpretation
scenario where an invalid address
String literal is considered as a hostname or fqdn resulting in a look by name.
Adding explicit factory methods is one approach, as proposed here.
An alternative is to provide a string literal validator
which returns true if the string literal address is either an IPv4 or IPv6
address.
The validator can be used by an application in conjunction with the existing
API getByName method.
For example, boolean isValidLiteralAddress(String literalAddress)
InetAddress serverAddress;
if (InetAddress.isValidLiteralAddress(someLiteralAddress) {
serverAddress = InetAddress.getByName(someAddress);
…
}
This will provide applications with a convenience method to validate inputs and
use existing API without side effects.
Also, this approach will reduce the amount of internal restructuring required,
should guarantee current behaviour, and provide a convenience application
validation method.
But, for some might consider making two method calls be too much of a burden!
If this is not considered an appropriate option, then a renaming of the
proposed methods would be beneficial so as to exhibit the semantics of the
operations.
For example, in keeping with the current nomenclature of getByName,
getByAddress then getFromLiteral or getByLiteral would be in keeping with
current naming scheme.
Alternatively, a straight forward “it does what it says on the tin” name such
as parseLiteral would seem appropriate. I'm not a big fan of these "of" factory
methods.
on further reflection, wrt method name, an overloaded getByAddress(String
literalAddress) method is most appropriate rather then the of methods
the most consistent (with the current API) and object oriented version is the
getByAddress(String addr). It is consistent with the existing API. Logically
getByAddress(byte[]) and getByAddress(String) are essentially the same. That is
to say, a literal string representing an address is essentially the same as an
array of bytes representing an address, as a String is an encapsulation of an
array of bytes. As such it logically natural to add an overloaded method.
Adding a new style of method affects the logical balance of the InetAddress
class, giving a mongrel, or hybrid API appearance.
Ceist agam ort:
Why is it necessary that the subclasses Inet4Address and Inet6Address have an
explicit public method for a getByAddress approach? Or have I misconstrued your
statement: "Overrides existing method in InetAddress, but adds new method to
Inet4Address and Inet6Address." ?
But this is the case for the current "ofLiteral" solution presented in the PR,
in that each of the derived classes Inet4Address and Inet6Address have added
new static public methods.
thanks for the considered response
> > That is to say, a literal string representing an address is essentially the
> > same as an array of bytes representing an address
>
> A literal IP address string can specify the same address in [multiple
> formats](https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/net/InetAddress.html#textual-representation-of-ip-addresses-heading),
> a byte array supports only one - an array of bytes, with length equal to 4
> for IPv4 addresses, and to 16 for IPv6 addresses.
I think you may have misinterpreted the point I was trying to make (or I didn't
make it clearly enough), for example, a 4 byte array representing an IPv4
address is logically the same as a String representing an IPv4 address in dot
notation. Thus, overloading getByAddress is a reasonable method to add and
exhibits OO characteristics.
>
> > Ceist agam ort:
> > Why is it necessary that the subclasses Inet4Address and Inet6Address have
> > an explicit public method for a getByAddress approach?
>
> The idea of this change is to add three methods capable of parsing a string
> as an IPv4, IPv6 or any IP literal address. The explicit methods need to be
> added to all three classes irrelevant to their naming.
>
> > Or have I misconstrued your statement: "Overrides existing method in
> > InetAddress, but adds new method to Inet4Address and Inet6Address." ?
> > But this is the case for the current "ofLiteral" solution presented in the
> > PR, in that each of the derived classes Inet4Address and Inet6Address have
> > added new static public methods.
>
> You are right that the statement is a bit vague, yes. What was meant there
> was overloading the `InetAddress` method, and adding new methods to
> `Inet4Address` and `Inet6Address` would introduce a bit confusing API
> structure. All three classes for both approaches would look like:
> ### ofLiteral approach
>
> ```
> // New methods
> InetAddress.ofLiteral(String)
> Inet4Address.ofLiteral(String)
> Inet6Address.ofLiteral(String)
> ```
>
> ### getByAddress approach
>
> ```
> // Existing methods with `address` being a byte array
> InetAddress.getByAddress(String, byte[])
> InetAddress.getByAddress(byte[])
>
> // New methods with `address` being a literal
> InetAddress.getByAddress(String) <-- overloaded method
> Inet4Address.getByAddress(String) <-- new and not overloaded method
> Inet6Address.getByAddress(String) <-- new and not overloaded method
> ```
There's no need to add these methods,
Inet4Address.getByAddress(String) <-- new and not overloaded method
Inet6Address.getByAddress(String) <-- new and not overloaded method
to the derived classes. The functionality can be fully provided by the
overloaded (static) getByAddress in the base InetAddress class
-------------
PR Comment: https://git.openjdk.org/jdk/pull/15775#issuecomment-1722571081
PR Comment: https://git.openjdk.org/jdk/pull/15775#issuecomment-1722595450
PR Comment: https://git.openjdk.org/jdk/pull/15775#issuecomment-1749739710
PR Comment: https://git.openjdk.org/jdk/pull/15775#issuecomment-1751688344
