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