[9] request for review 8046777: apple.security.KeychainStore has a problem searching for identities

Vincent Ryan vincent.x.ryan at oracle.com
Mon Nov 10 21:53:15 UTC 2014


I’ve updated the fix to reduce the casts to KeyEntry:
  http://cr.openjdk.java.net/~vinnie/8046777/webrev.02/


On 10 Nov 2014, at 18:48, Vincent Ryan <vincent.x.ryan at oracle.com> wrote:

> 
> On 10 Nov 2014, at 18:13, Sean Mullan <sean.mullan at oracle.com> wrote:
> 
>> You missed one of my comments:
>> 
>>>> 623-629 similar comment as above
> 
> I made the change to join the two if statements into a single one.
> Did you want me to use a local variable instead of casts?
> 
>> 
>> Otherwise looks fine.
>> 
>> --Sean
>> 
>> On 11/10/2014 01:05 PM, Vincent Ryan wrote:
>>> Thanks Sean. I’ve generated a new webrev that addresses your comments:
>>>  http://cr.openjdk.java.net/~vinnie/8046777/webrev.01/
>>> 
>>> On 10 Nov 2014, at 16:50, Sean Mullan <sean.mullan at oracle.com> wrote:
>>> 
>>>> A few comments:
>>>> 
>>>> * KeystoreImpl.m
>>>> 
>>>> - it is odd to have a comment embedded in the parameter list and makes the line very long. Can you move this comment above the call?
>>>> 
>>>> - update copyright year
>>>> 
>>>> * KeychainStore.java
>>>> 
>>>> 297-304 I think it would be cleaner to write these lines as:
>>>> 
>>>>           } else {
>>>>               KeyEntry ke = (KeyEntry)entry;
>>>>               if (ke.chain == null || ke.chain.length == 0) {
>>>>                   return null;
>>>>               }
>>>>               return ke.chain[0];
>>>>           }
>>>> 
>>>> 623-629 similar comment as above
>>>> 
>>>> * ListKeychainStore.sh
>>>> 
>>>> # See JDK-8046777
>>>> 
>>>> 123,166: No need to add this comment, @bug label and source code history is sufficient.
>>>> 
>>>> 156 if [ $RECOUNT -lt `expr $COUNT + 3 + 1` ]; then
>>>> 
>>>> change to $COUNT + 4
>>>> 
>>>> 157     echo "Error: expected >=4 private key entries in the Keychain keystores"
>>>> 
>>>> $COUNT is the number of private keys in the keystore before testing, so original code is still correct: "expected >$COUNT"
>>>> 
>>>> --Sean
>>>> 
>>>> On 10/29/2014 07:48 PM, Vincent Ryan wrote:
>>>>> Please review this fix contributed by David Kocher.
>>>>> 
>>>>> It corrects an issue in the Keychain keystore implementation for Mac OS where private keys
>>>>> configured with a key usage other than ‘any’ are not retrieved.
>>>>> 
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8046777
>>>>> Webrev: http://cr.openjdk.java.net/~vinnie/8046777/webrev.00/
>>>>> 
>>>>> Thanks.
>>>>> 
>>> 
> 



More information about the security-dev mailing list