On Tue, 12 Oct 2021 15:43:24 GMT, Aleksei Efimov <aefi...@openjdk.org> wrote:
>> This change implements a new service provider interface for host name and >> address resolution, so that java.net.InetAddress API can make use of >> resolvers other than the platform's built-in resolver. >> >> The following API classes are added to `java.net.spi` package to facilitate >> this: >> - `InetAddressResolverProvider` - abstract class defining a service, and >> is, essentially, a factory for `InetAddressResolver` resolvers. >> - `InetAddressResolverProvider.Configuration ` - an interface describing the >> platform's built-in configuration for resolution operations that could be >> used to bootstrap a resolver construction, or to implement partial >> delegation of lookup operations. >> - `InetAddressResolver` - an interface that defines methods for the >> fundamental forward and reverse lookup operations. >> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the >> characteristics of one forward lookup operation. >> >> More details in [JEP-418](https://openjdk.java.net/jeps/418). >> >> Testing: new and existing `tier1:tier3` tests > > Aleksei Efimov has updated the pull request incrementally with two additional > commits since the last revision: > > - Add @since tags to new API classes > - Add checks and test for empty stream resolver results src/java.base/share/classes/java/net/InetAddress.java line 231: > 229: * The first provider found will be used to instantiate the {@link > InetAddressResolver InetAddressResolver} > 230: * by invoking the {@link > InetAddressResolverProvider#get(InetAddressResolverProvider.Configuration)} > 231: * method. The instantiated {@code InetAddressResolver} will be > installed as the system-wide Maybe "... method. The **returned** {@code InetAddressResolver} will be installed ..." src/java.base/share/classes/java/net/InetAddress.java line 486: > 484: return cns; > 485: } finally { > 486: bootstrapResolver = null; This is incorrect. This will set bootstrapResolver to null in the case where you return it at line 468 - which means that a second call will then find it null. I believe you want to set it to null in the finally clause only if you reached line 470. You could achieve this by declaring a local variable - e.g `InetAddressResolver tempResolver = null;` before entering the try-finally then set that variable at line 470 `return tempResolver = bootstrapResolver;` and in finally do `if (tempResolver == null) bootsrapResolver = null;` To test this you could try to call e.g. `InetAddress.getByName` twice in succession in your test: I believe it will fail with the code as it stands, but will succeed with my proposed changes... src/java.base/share/classes/java/net/InetAddress.java line 860: > 858: return host; > 859: } > 860: } catch (RuntimeException | UnknownHostException e) { This may deserve a comment: resolver.lookUpHostName and InetAddress.getAllbyName0 delegate to the system-wide resolver, which could be custom, and we treat any unexpected RuntimeException thrown by the provider at that point as we would treat an UnknownHostException or an unmatched host name. src/java.base/share/classes/java/net/InetAddress.java line 1063: > 1061: public Stream<InetAddress> lookupAddresses(String host, > LookupPolicy policy) > 1062: throws UnknownHostException { > 1063: Objects.requireNonNull(host); Should we also double check that `policy` is not null? Or maybe assert it? src/java.base/share/classes/java/net/InetAddress.java line 1203: > 1201: + " as hosts file " + hostsFile + " not found > "); > 1202: } > 1203: // Check number of found addresses: I wonder if the logic here could be simplified by first checking whether only IPv4 addresses are requested, and then if only IPv6 addresses are requested. That is - evacuate the simple cases first and then only deal with the more complex case where you have to combine the two lists... ------------- PR: https://git.openjdk.java.net/jdk/pull/5822
