RFR 8044199: tests for RSA keys and key specifications

[Tristan Yan]

Thanks Valerie

Please review the fixed version.

http://cr.openjdk.java.net/~tyan/8044199/webrev.02/

Tristan


> Tristan,
>
> - Given that the tests are against SunRsaSign provider, I think
> sun/security/rsa subdirectory is a better choice than java/security/rsa.
> - Put @test and @bug on separate lines
> - Elaborate more on what is tested and what is the expected result in
> the bug record. The current content isn't very clear.
> - 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.
>
> For KeyTest.java
> 1) typo on line 69, rsaPrivateKey2 should be rsaPrivateKey
> 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.
> 3) line 58, add a space between '//" and text.
>
> For RSAKeySizeTest.java
> 1) specify provider "SunRSASign" for all getInstance() calls.
> 2) line 55 can be moved up to before the for-loop.
> 3) instead of using toString(int), probably bitLength() would be a
> better choice.
> 4) sizeTest() doesn't really check size but rather it compares the
> modulus values being equal. This check can be moved to RSAKeySizeTest.java.
>
> For RSATest.java
> 1) specify provider "SunRSASign" for all getInstance() calls.
>
> Thanks,
> Valerie
>
> On 8/3/2015 10:27 AM, Tristan Yan wrote:
>>/  Hi
/>>/
/>>/  Please review new tests for RSA keys and key specifications
/>>/
/>>/  Bug:https://bugs.openjdk.java.net/browse/JDK-8044199
/>>/  Webrev:http://cr.openjdk.java.net/~tyan/8044199/webrev.01/  <http://cr.openjdk.java.net/%7Etyan/8044199/webrev.01/>   <http://cr.openjdk.java.net/%7Etyan/8044199/webrev.01/>  <http://cr.openjdk.java.net/~tyan/8044199/webrev.01/  <http://cr.openjdk.java.net/%7Etyan/8044199/webrev.01/>   <http://cr.openjdk.java.net/%7Etyan/8044199/webrev.01/>>
/>>/
/>>/  Thanks
/>>/  Tristan
/>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20150820/73e9f022/attachment.htm>