[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