On Tue, 10 Mar 2026 18:29:13 GMT, Weijun Wang <[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. > > src/java.base/share/classes/sun/security/tools/keytool/CertAndKeyGen.java > line 1: > >> 1: /* > > The `getSelfCertificate` methods here are used in many places and we cannot > remove them in this PR. It's worth creating a new JBS issue to rewrite it. Created [JDK-8379887](https://bugs.openjdk.org/browse/JDK-8379887) > src/java.base/share/classes/sun/security/util/DerOutputStream.java line 568: > >> 566: } >> 567: >> 568: DateTimeFormatter dateTimeFormatter = > > `tz` above is useless. removed in the next commit > src/java.base/share/classes/sun/security/x509/CertificateValidity.java line > 57: > >> 55: // Returns the first time the certificate is valid. >> 56: public Date getNotBefore() { >> 57: return new Date(notBefore.toEpochMilli()); > > Why not `Date.fromInstant(notBefore)`? Changed it entirely in the next commit > src/java.base/share/classes/sun/security/x509/CertificateValidity.java line > 83: > >> 81: * not valid >> 82: */ >> 83: public CertificateValidity(Date notBefore, Date notAfter) { > > Is this constructor still useful? > > Shall we remove the `getNotAfter` and `getNotBefore` methods? Removed in the next commit > src/java.base/share/classes/sun/security/x509/CertificateValidity.java line > 133: > >> 131: DerOutputStream pair = new DerOutputStream(); >> 132: >> 133: if (notBefore.toEpochMilli() < YR_2050) { > > This could overflow. Maybe compare seconds? Changed to `notAfter.isBefore(Instant.ofEpochMilli(YR_2050))` > src/java.base/share/classes/sun/security/x509/X509CertImpl.java line 508: > >> 506: public void checkValidity() >> 507: throws CertificateExpiredException, CertificateNotYetValidException >> { >> 508: Instant instant = Instant.now().truncatedTo(ChronoUnit.MILLIS); > > Why do this? You're right, this makes no difference here. changed > test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/common/DynamicKeyStoreUtil.java > line 238: > >> 236: final long oneDayInTheFuture = currentTime + (24 * 60 * 60 * >> 1000); >> 237: final Instant startDate = >> Instant.ofEpochMilli(oneMinuteInThePast); >> 238: final Instant expiryDate = >> Instant.ofEpochMilli(oneDayInTheFuture); > > Maybe just use `Instant.now().minusSeconds(60)` etc? Good point, changed, thanks > test/jdk/java/security/cert/X509Certificate/VerifyDefault.java line 179: > >> 177: () -> cert.checkValidity( >> 178: Instant.parse("2012-05-26T16:47:26Z"))); >> 179: > > Maybe you can check both `Instant` and `Date`. After all, we still need to > support the old methods. > > You can also add new test to check the consistency. Ok, will add the test in the next commit. Just to clarify, what do you mean by the consistency check, isn't it covered by Asserts.assertEquals(notBeforeDate.getTime(), notBeforeInstant.toEpochMilli()); Asserts.assertEquals(notAfterDate.getTime(), notAfterInstant.toEpochMilli()); I think I might be misunderstanding you > test/jdk/sun/security/pkcs/pkcs10/PKCS10AttrEncoding.java line 66: > >> 64: Object[] values = { >> 65: ObjectIdentifier.of("1.2.3.4"), >> 66: new GregorianCalendar(1970, 1, 25, 8, 56, >> 7).getTime().toInstant(), > > Is there a modern way to create an `Instant`? `LocalDateTime`? Changed it LocalDateTime in the next commit > test/jdk/sun/security/util/DerInputBuffer/TimeParsing.java line 133: > >> 131: Instant d1Instant = decodeUTCInstant(b); >> 132: if (!d0.equals(d1) >> 133: && d0.toInstant().toEpochMilli() != d1Instant.toEpochMilli()) >> { > > Why `&&`? You always call this with a fixed milliseconds value, instead of > random `now()`. This way it tests that `decodeUTCInstant` and `decodeUTC` are returning equivalent results. Just in case that there is a difference in nanos in the future in `byte[] b` as `Date` supports up to millis ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r2924288227 PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r2924304045 PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r2896829838 PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r2896831118 PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r2924936211 PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r2896833294 PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r2924344277 PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r2924747374 PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r2924780183 PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r2924867377
