Your fix looks good to me. Thanks for contributing the patch.
> On 25 Mar 2015, at 14:47, Stanislav Baiduzhyi <baiduzhyi.de...@gmail.com> > wrote: > > On Tuesday 10 March 2015 17:18:06 Stanislav Baiduzhyi wrote: >> Hi All, >> >> Please review the patch for LdapURL. Patch fixes the parsing of query part >> of LDAP URL, leaving empty optional fields as null instead of "". >> >> Webrev: >> http://goo.gl/OO0V38 >> >> Bug: >> https://bugs.openjdk.java.net/browse/JDK-8074761 >> >> JTreg: >> http://goo.gl/ermmoh >> >> Details: >> >> RFC 2255 [1] allows any of the query parameters to be empty. Current >> implementation of parsing method extracts substring without checking for >> length, leaving empty fields as "" instead of null. But the code under >> com.sun.jndi.ldap package checks only for null when handling optional >> fields. So the patch modifies the parsing method to avoid substring >> operations on empty fields and leaving them as null instead. >> >> In proposed patch, I was not able to generalize the code, so using similar >> code blocks to make it obvious if additional changes will be required later. >> >> It would be even better to use java.util.Optional for this, but it will be >> compatibility-breaking change, I'm not sure it worth doing even under >> com.sun.* packages. >> >> There is wider test case in RedHat Bugzilla [2], includes OpenDS setup and >> small client app, shows the difference in results between openldap-clients >> with Java-based implementation. Proposed patch fixes the java client >> behaviour. >> >> [1]: https://tools.ietf.org/html/rfc2255#section-3 >> [2]: https://bugzilla.redhat.com/show_bug.cgi?id=1194226 > > A small reminder, please review the webrev quoted above, the patch is > extremely small, and passes the TCK. > > -- > Regards, > Stas >
