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

[Mark Powers]

On Mon, 25 Sep 2023 15:29:42 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> 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`.

Fixed.

> test/jdk/sun/security/x509/X509CRLImpl/UnexpectedNPE.java line 41:
> 
>> 39:     private static final String in = "MAsGCSqGSMP7TQEHAjI1Bgn///////8wCwUyAQ==";
>> 40: 
>> 41:     public UnexpectedNPE() {}
> 
> Useless constructor.

Removed.

> 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`?

Done.

> 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.

Good suggestion.

> 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.

Doing the former.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1343103788
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1343102818
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1343102946
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1343102629
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1343103415