[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 18:05:37 UTC 2014


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