Looks fine, just one nit. On line 241 of Config.java can you add braces
to the if-clause?
Thanks,
Sean
On 04/04/2014 07:00 AM, Wang Weijun wrote:
Updated webrev at
http://cr.openjdk.java.net/~weijun/8029995/webrev.01
Several differences:
1. Only true/false/yes/no are supported now.
2. Some words in javax/security/auth/kerberos/package-info.java
3. getBoolean() renamed to getBooleanObject() because it's quite easy to forget
the return value is a Boolean (instead of boolean) and could be null.
Thanks
Max
On Jan 29, 2014, at 5:46, Sean Mullan <[email protected]> wrote:
On 01/28/2014 03:53 AM, Wang Weijun wrote:
Please review the fix at
http://cr.openjdk.java.net/~weijun/8029995/webrev.00/
The supported boolean values in this fix cover what MIT krb5 does and
we also added 'f'.
The old getBooleanValue() method returns true for “true” and false
otherwise but the new method returns null if the value is not
supported. I’ve carefully changed how the method is called to ensure
maximum compatibility, but there is still one left:
We support DNS lookup for realm name by default, which means we do it
if dns_lookup_realm is not set to false, or when unset, if
dns_fallback is not set to false. Before this change, when
dns_lookup_realm is set to “unknown”, it means false so DNS lookup is
not performed. After this change, it’s equivalent to dns_lookup_realm
unset, and dns_fallback is used. I think the current behavior is
better than the old one.
I agree, but since it is a behavior change, it should be specified in the CCC
and release notes.
Fix looks fine otherwise.
--Sean
