<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Tristan,<br>
<br>
- Given that the tests are against SunRsaSign provider, I think
sun/security/rsa subdirectory is a better choice than
java/security/rsa.<br>
- Put @test and @bug on separate lines<br>
- Elaborate more on what is tested and what is the expected result
in the bug record. The current content isn't very clear.<br>
- Can u have more unique names for these tests? Currently, they are
too similarly named and it's somewhat confusing. The directory
already contains RSA, I don't feel that u need to have RSA again in
the names of the test especially when SunRsaSign provider doesn't
support non-RSA algorithms.<br>
<br>
For KeyTest.java<br>
1) typo on line 69, rsaPrivateKey2 should be rsaPrivateKey<br>
2) This test is based on the provider specific behavior that CRT key
is generated by default for RSA. It'd be much clearer if you can add
an instanceof check on the generated RSA private key objects before
the KeyFactory + RSAPrivateKeySpec code. In addition, add additional
code to check for equality with the matching KeySpec (e.g.
RSAPrivateCrtKeySpec for RSAPrivateCrtKey) is used.<br>
3) line 58, add a space between '//" and text.<br>
<br>
For RSAKeySizeTest.java<br>
1) specify provider "SunRSASign" for all getInstance() calls.<br>
2) line 55 can be moved up to before the for-loop.<br>
3) instead of using toString(int), probably bitLength() would be a
better choice.<br>
4) sizeTest() doesn't really check size but rather it compares the
modulus values being equal. This check can be moved to
RSAKeySizeTest.java.<br>
<br>
For RSATest.java<br>
1) specify provider "SunRSASign" for all getInstance() calls.<br>
<br>
Thanks,<br>
Valerie<br>
<br>
On 8/3/2015 10:27 AM, Tristan Yan wrote:
<blockquote
cite="mid:B3CFB909-2DB2-4FA0-AB89-EB04F4F2536D@oracle.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=ISO-8859-1">
<pre style="background-color: rgb(255, 255, 255);" class="">
</pre>
<pre style="background-color: rgb(255, 255, 255);" class="">Hi
Please review new tests for RSA keys and key specifications
Bug: <a moz-do-not-send="true" href="https://bugs.openjdk.java.net/browse/JDK-8044199" class="">https://bugs.openjdk.java.net/browse/JDK-8044199</a>
Webrev: <a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Etyan/8044199/webrev.01/" class="">http://cr.openjdk.java.net/~tyan/8044199/webrev.01/</a> <<a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Etyan/8044199/webrev.01/" class="">http://cr.openjdk.java.net/~tyan/8044199/webrev.01/</a>>
Thanks
Tristan</pre>
<div class=""><br class="">
</div>
</blockquote>
</body>
</html>