On Mon, 27 Jan 2025 12:39:45 GMT, Konanki Sreenath <d...@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

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.

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.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23315#discussion_r1937648399
PR Review Comment: https://git.openjdk.org/jdk/pull/23315#discussion_r1937661846
PR Review Comment: https://git.openjdk.org/jdk/pull/23315#discussion_r1937659335

Reply via email to