RFR: 8346094: Harden X509CertImpl.getExtensionValue for NPE cases

Konanki Sreenath duke at openjdk.org
Thu Feb 6 05:58:52 UTC 2025


On Fri, 31 Jan 2025 17:29:24 GMT, Matthew Donovan <mdonovan at openjdk.org> wrote:

>> Earlier code will trigger NPE if the certificate does not contain the extensions or if the requested extensions does not exist. The better approach for hardening **getExtensionValue** here is to to check for NULL explicitly before calling **getExtensionValue()** and avoding try-catch block which ensures the readability and maintainability.
>> 
>> After scanning in multiple places where invokng getExtensions on the X509CertInfo reference, the check for NULL is added in the **getKeyUsage()** as well while calling before **getExtensionValue()**
>> 
>> The associated tests are written and added in test class **CertificateExtensions**. Which will ensure to validate the 
>> **getExtensionValue()** and **getKeyUsage()** methods in **X509CertImpl** class.
>
> test/jdk/java/security/cert/X509Certificate/CertificateExtensions.java line 1:
> 
>> 1: 
> 
> Needs a copyright statement

added the copyright statement

> test/jdk/java/security/cert/X509Certificate/CertificateExtensions.java line 29:
> 
>> 27:          * else it is incorrect
>> 28:          */
>> 29:         try {
> 
> These try/catch blocks around the assert statements seem redundant. The assert methods already throw a RuntimeException with a message.

Addressed and consolidated all try-catch blocks

> test/jdk/java/security/cert/X509Certificate/CertificateExtensions.java line 40:
> 
>> 38:          */
>> 39:         try {
>> 40:             System.out.println("------ cert x509Certimpl ------ " + x509Certimpl);
> 
> The output might be a little easier to read if you put the "---- cert X509Certimpl ----" on its own line.

was added these during the debug and continued the same in draft PR. I have removed these statements

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23315#discussion_r1940501046
PR Review Comment: https://git.openjdk.org/jdk/pull/23315#discussion_r1940502406
PR Review Comment: https://git.openjdk.org/jdk/pull/23315#discussion_r1940501952


More information about the security-dev mailing list