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.
src/java.base/share/classes/sun/security/x509/X509CertImpl.java line 1:
> 1: /*
Copyright year should be 2025 since the file is changed
test/jdk/java/security/cert/X509Certificate/CertificateExtensions.java line 27:
> 25: * @test
> 26: * @bug 8346094
> 27: * @summary validating getExtensionValue and getKeyUsage with specified
> and unspecified extensions on the X509Certificate.
Check line indentation issue
test/jdk/java/security/cert/X509Certificate/CertificateExtensions.java line 30:
> 28: * @library /test/lib
> 29: * @modules java.base/sun.security.x509
> 30: * java.base/sun.security.util
Check line indentation issue
test/jdk/java/security/cert/X509Certificate/CertificateExtensions.java line 58:
> 56: * else it is incorrect
> 57: */
> 58:
> x509Certimpl.getInfo().setExtensions(createCertificateExtensions(x509Certimpl.getInfo().getKey().getKey()));
Line indentation issue
test/jdk/java/security/cert/X509Certificate/CertificateExtensions.java line 61:
> 59:
> Asserts.assertNotNull(x509Certimpl.getExtensionValue("2.5.29.17"));
> 60: /**
> 61: * Certificate is created with extensions. Invoking
> getExtensionValue with invalid oid must return NULL
Same comment as above
test/jdk/java/security/cert/X509Certificate/CertificateExtensions.java line 101:
> 99: x509CertInfo.setSubject(subject);
> 100:
> 101: //create and set the subject and issuer unique identity
Give space after //
test/jdk/java/security/cert/X509Certificate/CertificateExtensions.java line 126:
> 124: byte[] signedCert = {};
> 125:
> 126: x509Certimpl = new X509CertImpl(x509CertInfo,
> AlgorithmId.get("SHA256withRSA"), signedData, signedCert);
Line indentation
test/jdk/java/security/cert/X509Certificate/CertificateExtensions.java line 134:
> 132: }
> 133:
> 134: public static sun.security.x509.CertificateExtensions
> createCertificateExtensions(PublicKey publicKey) throws IOException,
> NoSuchAlgorithmException {
Same comment as above
test/jdk/java/security/cert/X509Certificate/CertificateExtensions.java line 136:
> 134: public static sun.security.x509.CertificateExtensions
> createCertificateExtensions(PublicKey publicKey) throws IOException,
> NoSuchAlgorithmException {
> 135: // Create Extensions
> 136: sun.security.x509.CertificateExtensions certificateExtensions =
> new sun.security.x509.CertificateExtensions();
Same comment
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23315#discussion_r1942207350
PR Review Comment: https://git.openjdk.org/jdk/pull/23315#discussion_r1942209828
PR Review Comment: https://git.openjdk.org/jdk/pull/23315#discussion_r1942210035
PR Review Comment: https://git.openjdk.org/jdk/pull/23315#discussion_r1942210479
PR Review Comment: https://git.openjdk.org/jdk/pull/23315#discussion_r1942210641
PR Review Comment: https://git.openjdk.org/jdk/pull/23315#discussion_r1942210931
PR Review Comment: https://git.openjdk.org/jdk/pull/23315#discussion_r1942211209
PR Review Comment: https://git.openjdk.org/jdk/pull/23315#discussion_r1942211366
PR Review Comment: https://git.openjdk.org/jdk/pull/23315#discussion_r1942211452
