Thanks for the review. All comments accepted.
--Max
> On Apr 5, 2016, at 9:13 PM, Seán Coffey <sean.cof...@oracle.com> wrote:
>
> A few comments from supportability side of the table.
>
> =============
> sun/security/provider/AbstractDrbg.java
>
>> + if (dp.getStrength() > strength) {
>> + throw new IllegalArgumentException("strength too high");
>> + }
>> + if (result.length > maxNbLength) {
>> + throw new IllegalArgumentException("result too long");
>> + }
> Please print these bad strengths / results in the exception.
>
> Similar corrections are needed in the engineConfigure method :
>
>> 608 if (config.getStrength() > highestSecurity) {
>> 609 throw new IllegalArgumentException("strength too
>> big");
>> 610 }
>> 611 if (config.getPersonalizationString() != null &&
>> config.getPersonalizationString().length > maxPsLength) {
>> 612 throw new IllegalArgumentException("ps too long");
>> 613 }
>
>> throw new IllegalArgumentException("unknown params type");
> can you print the type of params that was passed in ? (X 2 calls)
>
>
>> + if (dp.getAdditionalInput() != null &&
>> dp.getAdditionalInput().length > maxAiLength) {
>> + throw new IllegalArgumentException("ai too long");
> Please print ai value.
>
>
>> + // This SEI does not support pr
>> + throw new IllegalArgumentException();
> Cou you put your comment in the body of the IllegalArgumentException ?
>
> =============
> sun/security/provider/CtrDrbg.java
>
> + try {
> + aesLimit = Cipher.getMaxAllowedKeyLength("AES");
> + } catch (Exception e) {
> + // should not happen
> + throw new AssertionError("Cannot detect AES");
> + }
>
> Just to be safe, can you add e as Throwable variable for AssertionError ?
>
>
>> + if (input.length != seedLen) {
>> + // Should not happen
>> + throw new IllegalArgumentException("input must be of seedlen
>> bytes");
>
> can you print the lengths expected ?
> =============
> sun/security/provider/DRBG.java
>
>> + if (strength < 0) {
>> + throw new IllegalArgumentException(
>> + "strength in drbg cannot be
>> negative");
>> + }
> Let's print the value of the 'part' string in this exception.
>
> + } else {
> + throw new IllegalArgumentException("Unsupported params");
> + }
>
> can you print the type of params that were passed in ?
>
> + default:
> + throw new IllegalArgumentException("Unsupported mech");
> + }
>
> can yuo print the mech value encoutered ?
> =============
> sun/security/provider/HashDrbg.java
>
>> + } catch (DigestException e) {
>> + throw new AssertionError("will not happen");
>> + }
>
> Famous last words ;)
> Can you add e as Throwable cause to AssertionError ? (happens in two areas)
>
> Regards,
> Sean.
>
> On 05/04/2016 03:34, Wang Weijun wrote:
>> Updated webrev again at
>>
>> http://cr.openjdk.java.net/~weijun/8051408/webrev.09/
>> http://cr.openjdk.java.net/~weijun/8051408/webrev.09/spec
>> http://cr.openjdk.java.net/~weijun/8051408/webrev.09/specdiff
>>
>> The only change is that SecureRandomInstantiateParameters,
>> SecureRandomNextBytesParameters and SecureRandomReseedParameters are removed
>> and only a single SecureRandomParameters is added. There seems no reason to
>> introduce 3 marker interfaces.
>>
>> Thanks
>> Max
