RFR: 4985694: Incomplete spec for most of the getInstances

Xuelei Fan xuelei.fan at oracle.com
Tue Nov 1 00:36:11 UTC 2016


Looks fine to me.

Thanks,
Xuelei

On 11/1/2016 5:19 AM, Bradford Wetmore wrote:
> Here's the latest, and have checked in:
>
>     https://bugs.openjdk.java.net/browse/JDK-4985694
>     http://cr.openjdk.java.net/~wetmore/4985694/webrev.01/
>
>>> Egads, good catch.  I completely missed this one.  I will put in
>>> Objects.requireNonNull throughout.
>>>
>>> Cipher.java is the one weird one, null or empty gets the same exception.
>
> Updated both Mac and Cipher to throw upon entrance.
>
> Thanks for the reviews.
>
> Brad
>
>
> On 10/30/2016 10:56 AM, Sean Mullan wrote:
>> Looks good to me.
>>
>> --Sean
>>
>> On 10/28/2016 08:17 PM, Bradford Wetmore wrote:
>>> Egads, good catch.  I completely missed this one.  I will put in
>>> Objects.requireNonNull throughout.
>>>
>>> Cipher.java is the one weird one, null or empty gets the same exception.
>>>
>>> Brad
>>>
>>>
>>>
>>> On 10/28/2016 5:13 PM, Xuelei Fan wrote:
>>>> Looks fine to me.
>>>>
>>>> For the update in Mac.java, I may prefer to check the null parameter
>>>> explicitly as earlier as possible instead of delegate to the further
>>>> calls (GetInstance.getServices()).  Similar comment to Cipher.java.
>>>> Not
>>>> an issue, just a preference.
>>>>
>>>> Thanks,
>>>> Xuelei
>>>>
>>>> On 10/29/2016 3:06 AM, Bradford Wetmore wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> I need a review for this P2 TCK-RED-9 bug.
>>>>>
>>>>> The original request (JDK-8166350) was for documenting null algorithm
>>>>> names in the 3 new DRBG getInstances() in SecureRandom, but there is a
>>>>> 12 year old bug for the same thing throughout the
>>>>> JCA/JCE/JSSE/JGSS/JAAS.  I've gone ahead and fixed.
>>>>>
>>>>>     https://bugs.openjdk.java.net/browse/JDK-4985694
>>>>>     http://cr.openjdk.java.net/~wetmore/4985694/webrev.00/
>>>>>
>>>>> Other comments:
>>>>>
>>>>> .  Instead of waiting for a NP to be generated by the code, added a
>>>>> hardcoded NPE parameter sanity check:
>>>>>
>>>>>     Objects.requireNonNull(algorithm, "null algorithm name");
>>>>>
>>>>> .  verified APIs to ensure proper exceptions are thrown for null/empty
>>>>> algorithm/provider Strings and null Providers.
>>>>>
>>>>> .  Added a full test suite for all getInstances to check for above,
>>>>> including a reflection check for future getInstances.
>>>>>
>>>>> .  Some minor javadoc cleanup/reorgs, mainly to the
>>>>> @returns/@throws/@exceptions tags (e.g. {@code
>>>>> ...}/alphabetizing/ending
>>>>> "." on phrases} for consistency and to conform to current javadoc
>>>>> standards.  I've tried to be consistent throughout (@code's around
>>>>> class
>>>>> names}, but I know I have missed a couple things here/there (no
>>>>> @code's
>>>>> around parameter names).  I do need to move onto other things.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Brad



More information about the security-dev mailing list