Valerie,
Thanks for your review.
Best regards,
John Jiang
On 2015/9/24 3:51, Valerie Peng wrote:
Updated webrev looks fine.
Thanks,
Valerie
On 9/22/2015 10:21 PM, John Jiang wrote:
Hi Valerie,
Thanks for your comments.
Please review the updated patch at
http://cr.openjdk.java.net/~fyuan/jjiang/8075286/webrev.01/
Best regards,
John Jiang
On 2015/9/12 7:50, Valerie Peng wrote:
Mostly are fine, just a few comments:
1) the convention is to place the regression tests based on the
provider which they are for, e.g.
SunJCE provider -> com/sun/crypto/provider
SUN provider -> sun/security/provider
SunEC provider -> sun/security/ec
Given that there is not a lot of dependency between these tests, can
u place them into the sub-directories under the dedicated path?
The utility class TestSignatureOidHelper.java into can be placed
under sun/security.
2) several tests has "...must be failed", probably should be changed
to "...should fail"
3) the variable naming seems confusing, e.g. cipherAlgorithm and
cipherOid both refer to Cipher objects while cipherText refers to
encrypted text. For future development, u may want to name the
cipher variables differently so it's obvious that they represent
Cipher objects.
Thanks,
Valerie
On 8/13/2015 1:43 AM, Sha Jiang wrote:
Hi Security developers,
I have a security test bug
https://bugs.openjdk.java.net/browse/JDK-8075286, which adds more
cases for NSA Suite B algorithms in jdk repo.
Please review this patch at
http://cr.openjdk.java.net/~fyuan/jjiang/8075286/webrev.00/
Every comment is appreciated.
Best Regards,
John Jiang
