Thanks Rob, looks fine to me now. Looking forward for a JDK 8 + JDK 11 backport and a testable package for Windows.
Michael > -----Original Message----- > From: Rob McKenna <rob.mcke...@oracle.com> > Sent: Thursday, October 25, 2018 6:35 PM > To: core-libs-dev@openjdk.java.net > Cc: Osipov, Michael (GS IT PD LD PLM) <michael.osi...@siemens.com>; > sean.mul...@oracle.com > Subject: [RFR] 8160768: Add capability to custom resolve host/domain names > within the default JDNI LDAP provider > > This recently received CSR approval, so it seems like a good time to pick > the codereview up again: > > http://cr.openjdk.java.net/~robm/8160768/webrev.08/ > > Referencing: > > http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-December/050794.html > > 1) I'm copying the behaviour from > LdapCtxFactory.getInitialContext(Hashtable<?,?>) where an empty String is > the default value used when the provider url is null. > > I don't think HostnameChecker (by way of SNIHostname) will accept either > empty or null values when doing the comparison. > > Somewhat oddly however, LdapCtx.extendedOperation(ExtendedRequest) > appears to pass the String "null" to > StartTlsResponseImpl.setConnection(Connection, String), which attempts > to substitute null values with the Connections host so there may be a bug > here. > > I'm happy to allow null returns here if necessary. Sean, can you > comment on the distinction between null & "" as far as hostname > verification is concerned? > > 2) In the latest iteration lookupEndpoints() returns an > Optional<LdapDnsResult>. Currently neither getEndpoints() nor > getDomainName() can be null. (they can be an empty list and/or an empty > String however) > > 3) Corrected. > > 4) See https://www.oracle.com/technetwork/java/seccodeguide-139067.html#4-5 > > -Rob
