RFR: JDK-8315042 NPE in PKCS7.parseOldSignedData [v3]

[Weijun Wang]

On Mon, 25 Sep 2023 03:40:47 GMT, Mark Powers <mpowers at openjdk.org> wrote:

>> https://bugs.openjdk.org/browse/JDK-8315042
>
> Mark Powers has updated the pull request incrementally with one additional commit since the last revision:
> 
>   comment from Weijun

test/jdk/sun/security/x509/X509CRLImpl/UnexpectedNPE.java line 27:

> 25:  * @test
> 26:  * @library /test/lib
> 27:  * @bug 5052433 8315042

Usually we put `@bug` before `@library`.

test/jdk/sun/security/x509/X509CRLImpl/UnexpectedNPE.java line 39:

> 37: public class UnexpectedNPE {
> 38:     CertificateFactory cf = null ;
> 39:     private static final String in = "MAsGCSqGSMP7TQEHAjI1Bgn///////8wCwUyAQ==";

Just embed the string inside the definition of `decodedBytes`. No need to create a field for it.

test/jdk/sun/security/x509/X509CRLImpl/UnexpectedNPE.java line 41:

> 39:     private static final String in = "MAsGCSqGSMP7TQEHAjI1Bgn///////8wCwUyAQ==";
> 40: 
> 41:     public UnexpectedNPE() {}

Useless constructor.

test/jdk/sun/security/x509/X509CRLImpl/UnexpectedNPE.java line 47:

> 45:         byte[] encoded_2 = { 0x30, 0x01, 0x00, 0x00 };
> 46:         byte[] encoded_3 = { 0x30, 0x01, 0x00 };
> 47:         byte[] decodedBytes = Base64.getDecoder().decode(in);

Maybe rename to `encoded_4`?

test/jdk/sun/security/x509/X509CRLImpl/UnexpectedNPE.java line 49:

> 47:         byte[] decodedBytes = Base64.getDecoder().decode(in);
> 48: 
> 49:         UnexpectedNPE unpe = new UnexpectedNPE() ;

Just modify `run` to static. No class instance needed.

test/jdk/sun/security/x509/X509CRLImpl/UnexpectedNPE.java line 60:

> 58:         if (cf == null) {
> 59:             try {
> 60:                 cf = CertificateFactory.getInstance("X.509", "SUN");

Make `cf` static and move the line above to `main`. Or, make it a local variable in `main` and pass it to all `run` calls.

test/jdk/sun/security/x509/X509CRLImpl/UnexpectedNPE.java line 76:

> 74: 
> 75:         System.out.println("CRLException has not been thrown");
> 76:         return false;

I think only `generateCRLs` is enough here since neither bug is about X.509 encoding.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1336058841
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1336054466
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1336055352
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1336055601
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1336055142
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1336055988
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1336058145