On Mon, 11 May 2026 09:46:42 GMT, Mikhail Yankelevich <[email protected]> wrote:
>> Adding new methods to `X509Certificate` to return `Instant` instead of >> `Date` as well as moving away from `Date` in internal packages wherever >> possible. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Mikhail Yankelevich has updated the pull request incrementally with one > additional commit since the last revision: > > comments Some comments so far, mostly minor. src/java.base/share/classes/sun/security/util/DerOutputStream.java line 497: > 495: > 496: /** > 497: * Takes an instant and chooses UTC or GeneralizedTime as per RFC > 2630 Add period at end of sentence. src/java.base/share/classes/sun/security/util/DerValue.java line 1085: > 1083: /** > 1084: * Determines whether {@code Instant} was encoded as UTC or > Generalized time and > 1085: * calls getUTCInstant or getGeneralizedInstant accordingly Add period at end of sentence. src/java.base/share/classes/sun/security/x509/PrivateKeyUsageExtension.java line 1: > 1: /* Copyright update. src/java.base/share/classes/sun/security/x509/PrivateKeyUsageExtension.java line 69: > 67: > 68: private Instant notBefore = null; > 69: private Instant notAfter = null; No need to set to `null`. Also, I think these can be `final`. src/java.base/share/classes/sun/security/x509/X509CertImpl.java line 542: > 540: * yet valid with respect to the <code>instant</code> supplied. > 541: */ > 542: public void checkValidity(Instant instant) Suggest adding `@Override` annotations for the new methods, even though it isn't used consistently on other methods. test/jdk/java/security/cert/X509Certificate/VerifyDefault.java line 1: > 1: /* This is an unusual test to add these changes to, since it has nothing to do with the original issue that this test was created for. I'm ok with it, as long as you update the `@summary`. I also think you should test the embedded cert and not the `TestX509Certificate` wrapper. test/jdk/java/security/cert/X509Certificate/VerifyDefault.java line 152: > 150: CertificateExpiredException { > 151: > 152: final Date notBeforeDate =cert.getNotBefore(); space after `=`. ------------- PR Review: https://git.openjdk.org/jdk/pull/30047#pullrequestreview-4265339162 PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r3220396754 PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r3220381899 PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r3220489611 PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r3220494139 PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r3220447057 PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r3220583484 PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r3220523907
