RFR 8029995: accept yes/no for boolean krb5.conf settings

Sean Mullan sean.mullan at oracle.com
Fri Apr 4 13:07:36 UTC 2014


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 <sean.mullan at oracle.com> 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
>>
>



More information about the security-dev mailing list