Hi Max,

Here is the updated webrev: 
http://cr.openjdk.java.net/~ssahoo/8141039/webrev.04/

Changes included:
- Added new cases in ApiTest.java to address " nextBytes(.., 
DrbgParameters.nextBytes(-1, *true*, ..)) " and " 
reseed(DrbgParameters.reseed(true,..)) "
- Decoration comments defined bellow.
- Removed unnecessary checkException() call.


- I have not addressed using " Supplier<SecureRandom> " because some of API 
method throws checked exception.
- Also I am thinking the test case for SHA1PRNG bug inside 
SerializedSeedTest.java, still a valid case for other DRBG. So I am keeping the 
file unchanged.

Thanks,
Siba

-----Original Message-----
From: Wang Weijun 
Sent: Wednesday, May 18, 2016 1:37 PM
To: Sibabrata Sahoo
Cc: security-dev@openjdk.java.net
Subject: Re: [9] RFR: 8141039: Test Task: Develop new tests for JEP 273: 
DRBG-Based SecureRandom Implementations

ApiTest.java:

- Please move line 128-130 (the System.out.println) line before line 127, so 
that if getInstance() fails, we can see what parameters are failing.

- Useless line 69.

- Inside verifyAPI(), you call nextBytes(.., DrbgParameters.nextBytes(-1, 
false, ..)). Can you also call nextBytes(.., DrbgParameters.nextBytes(-1, 
*true*, ..))? It should fail unless the instantiation parameters has 
PR_AND_RESEED. You can use Capability::supportsPredictionResistance to check 
it. Same with reseed(DrbgParameters.reseed(true,..)).

- Can you use Supplier<SecureRandom> instead of creating a new RunnableCode 
type? Same in GetInstanceTest.java.

- If matchExc always calls checkException, why not use a single method? Same in 
GetInstanceTest.java.

- SUCESS is not final, you shouldn't use ALL CAPITAL letters for it. Same in 
GetInstanceTest.java.

SerializedSeedTest.java:

- The SHA1PRNG bug is fixed and has its own regression test. You can remove 
related codes here.

Thanks
Max

> On May 17, 2016, at 4:13 PM, Sibabrata Sahoo <sibabrata.sa...@oracle.com> 
> wrote:
> 
> Hi Max,
> 
> Here is the updated webrev: 
> http://cr.openjdk.java.net/~ssahoo/8141039/webrev.03/
> I misinterpreted your previous comment that the following change is only 
> applicable to getInstanceTest.java and not applicable to ApiTest.java.
> 
> The change includes,
> - ApiTest.java moved to " java/security/SecureRandom ".
> - Removed reference to MoreDrbgParameters from ApiTest.java
> 
> Thanks,
> Siba

Reply via email to