8374808: Add new methods to KeyStore and KeyStoreSpi that return the creation date as an Instant instead of Date by myankelev · Pull Request #29140 · openjdk/jdk

myankelev · 2026-01-09T14:16:41Z

Hi!

This is my proposal to transfer KeyStore and KeyStoreSpi with internal implementations to use Instances instead of Dates.
I would be very grateful for your comments and suggestions.

Thanks!

P.S. this is related to JDK-8350953

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Change requires CSR request JDK-8375012 to be approved
Commit message must refer to an issue

Issues

JDK-8374808: Add new methods to KeyStore and KeyStoreSpi that return the creation date as an Instant instead of Date (Enhancement - P4)
JDK-8375012: Add new methods to KeyStore and KeyStoreSpi that return the creation date as an Instant instead of Date (CSR)

Reviewers

Weijun Wang (@wangweij - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/29140/head:pull/29140
$ git checkout pull/29140

Update a local copy of the PR:
$ git checkout pull/29140
$ git pull https://git.openjdk.org/jdk.git pull/29140/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 29140

View PR using the GUI difftool:
$ git pr show -t 29140

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/29140.diff

Using Webrev

Link to Webrev Comment

… and KeyStoreSpi

bridgekeeper · 2026-01-09T14:19:11Z

👋 Welcome back myankelevich! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2026-01-09T14:19:27Z

@myankelev This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8374808: Add new methods to KeyStore and KeyStoreSpi that return the creation date as an Instant instead of Date

Reviewed-by: weijun

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 446 new commits pushed to the master branch:

36758b0: 8377435: Problem list serviceability/sa/TestJhsdbJstackMixedCore.java
2a8badf: 8376356: Parallel: Convert PSCardTable to use Atomic
f81bea2: 8376351: Parallel: Convert ParallelScavengeHeap to use Atomic
... and 443 more: https://git.openjdk.org/jdk/compare/6af27420e3b1980bc093776e3db76072123f7487...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2026-01-09T14:20:26Z

@myankelev The following label will be automatically applied to this pull request:

security

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

wangweij

Mostly looks good. Do you have a plan to update the callers of KeyStore::getCreationDate?

Also, I see you skipped the SunPKCS11 and MSCAPI implementations. Are they too trivial and the default implementation is enough?

myankelev · 2026-01-13T11:40:21Z

Mostly looks good. Do you have a plan to update the callers of KeyStore::getCreationDate?

Also, I see you skipped the SunPKCS11 and MSCAPI implementations. Are they too trivial and the default implementation is enough?

Apologies, missed this comment the last time I went through them.
P11KeyStore has no support for creation date and throws an exception so I think the default should be enough.
As for CKeyStore It seems that it just creates a new date and nothing else. I'm not sure if writing a whole separate method to always provide current time is worth it when we have a default implementation. But if you believe it is - I'm happy to add it

mlbridge · 2026-01-14T15:50:05Z

Webrevs

seanjmullan · 2026-01-14T16:01:36Z

Can you change the title of the issue to be more descriptive that these are new APIs, ex: "Add new methods to KeyStore and KeyStoreSpi that return the creation date as an Instant instead of Date"

wangweij

Some comments.

I have a small concern on the Instant.now() calls everywhere. As you can see in your TestKeyStoreBasic.java test, the value is truncated when persisting inside a keystore and when loaded again it's not identical to the old value. Will this create any interop issue in the real world? If we know a keystore implementation only support the precision of milliseconds, should we truncate it early?

myankelev · 2026-01-15T11:24:52Z

Some comments.

I have a small concern on the Instant.now() calls everywhere. As you can see in your TestKeyStoreBasic.java test, the value is truncated when persisting inside a keystore and when loaded again it's not identical to the old value. Will this create any interop issue in the real world? If we know a keystore implementation only support the precision of milliseconds, should we truncate it early?

I believe this would be the safest thing to do. Existing implementation is relying on new Date() which is actually calling System.currentTimeMillis() which is sufficient. Even though it shouldn't cause any interop issues as Date still remains unchanged Instant values can cause confusion as in the test.

I have changed these 'Instant.now()' to Instant.now().truncatedTo(ChronoUnit.MILLIS);

wangweij · 2026-02-05T19:01:50Z

-                        dos.writeLong(((TrustedCertEntry)entry).date.getTime());
+                        dos.writeLong(
+                                ((TrustedCertEntry)entry).date.toEpochMilli()
+                        );


Not very common to write ); on a new line.

Same below on line 659. Also in JavaKeyStore line 633.

I'd personally prefer to keep it this way as when it was inline it was really hard to read for me. If you feel strongly about it - I can change it :)

No problem. In fact, I see on line 2440 of PKCS12KeyStore you have

instant = Instant.ofEpochMilli( Long.parseLong(keyIdStr.substring(5)));

So I thought you're OK to either style.

Actually, I just missed this, changed in the next commit. Hope it looks good to you

wangweij

LGTM. Thanks for the fix!

myankelev · 2026-02-10T12:16:20Z

Thanks for the reviews @wangweij @seanjmullan @sisahoo

myankelev · 2026-02-10T12:16:43Z

/integrate

openjdk · 2026-02-10T12:18:08Z

Going to push as commit 264fdc5.
Since your change was applied there have been 461 commits pushed to the master branch:

bfb6de5: 8377429: Warning as error in asan build in test_nmt_cornercases.cpp on Linux
bd2a3b8: 8373040: Mark compiler/codecache/CodeCacheSegmentSizeTest.java as flagless
665dc49: 8208752: Calling a deserialized Lambda might fail with ClassCastException
... and 458 more: https://git.openjdk.org/jdk/compare/6af27420e3b1980bc093776e3db76072123f7487...master

Your commit was automatically rebased without conflicts.

openjdk · 2026-02-10T12:18:19Z

@myankelev Pushed as commit 264fdc5.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

JDK-8374808: Attempt to add an Instance in place of Date for KeyStore…

ecbc6bf

… and KeyStoreSpi

openjdk Bot changed the title ~~JDK-8374808: Add an Instance in place of Date for KeyStore and KeyStoreSpi~~ 8374808: Add an Instance in place of Date for KeyStore and KeyStoreSpi Jan 9, 2026

openjdk Bot added the security security-dev@openjdk.org label Jan 9, 2026

wangweij reviewed Jan 9, 2026

View reviewed changes

Comment thread src/java.base/share/classes/java/security/KeyStoreSpi.java Outdated

wangweij reviewed Jan 9, 2026

View reviewed changes

Comment thread src/java.base/share/classes/java/security/KeyStore.java Outdated

Weijun's comments

711fa0b

wangweij reviewed Jan 9, 2026

View reviewed changes

openjdk Bot added the csr Pull request needs approved CSR before integration label Jan 12, 2026

comments

8891c0f

myankelev added 4 commits January 13, 2026 16:01

test

29342c0

test

7153c9e

renamed timestamp to instant

02f46da

renamed timestamp to instant

553ed93

myankelev marked this pull request as ready for review January 14, 2026 15:42

openjdk Bot added the rfr Pull request is ready for review label Jan 14, 2026

DKS tests enhanced

6c35e98

wangweij reviewed Jan 14, 2026

View reviewed changes

Comment thread src/java.base/share/classes/sun/security/provider/DomainKeyStore.java

Comment thread src/java.base/share/classes/java/security/KeyStore.java Outdated

seanjmullan reviewed Jan 14, 2026

View reviewed changes

Comment thread src/java.base/share/classes/java/security/KeyStore.java Outdated

myankelev changed the title ~~8374808: Add an Instance in place of Date for KeyStore and KeyStoreSpi~~ 8374808: Add new methods to KeyStore and KeyStoreSpi that return the creation date as an Instant instead of Date Jan 15, 2026

documentation change and truncated instances to millis

a7fd467

seanjmullan reviewed Jan 16, 2026

View reviewed changes

myankelev added 2 commits January 19, 2026 10:13

Sean's comments

940ec50

updated implementations to the latest docs

aebf247

seanjmullan reviewed Jan 20, 2026

View reviewed changes

Doc changes

a01ca62

sisahoo reviewed Jan 21, 2026

View reviewed changes

Comment thread src/java.base/share/classes/com/sun/crypto/provider/JceKeyStore.java

myankelev added 2 commits January 21, 2026 13:52

Doc changes

befe998

patter matching comment

8a5341a

openjdk Bot removed the csr Pull request needs approved CSR before integration label Jan 27, 2026

wangweij reviewed Feb 5, 2026

View reviewed changes

Comment thread src/java.base/macosx/classes/apple/security/KeychainStore.java Outdated

wangweij reviewed Feb 5, 2026

View reviewed changes

style changes

0db2300

wangweij approved these changes Feb 9, 2026

View reviewed changes

openjdk Bot added the ready Pull request is ready to be integrated label Feb 9, 2026

openjdk Bot added the integrated Pull request has been integrated label Feb 10, 2026

openjdk Bot closed this Feb 10, 2026

openjdk Bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Feb 10, 2026

Conversation

myankelev commented Jan 9, 2026 • edited by openjdk Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Reviewers

Reviewing

Uh oh!

bridgekeeper Bot commented Jan 9, 2026

Uh oh!

openjdk Bot commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk Bot commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wangweij left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

myankelev commented Jan 13, 2026

Uh oh!

mlbridge Bot commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

seanjmullan commented Jan 14, 2026

Uh oh!

wangweij left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

myankelev commented Jan 15, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wangweij Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

myankelev Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

wangweij Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

myankelev Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

wangweij left a comment

Choose a reason for hiding this comment

Uh oh!

myankelev commented Feb 10, 2026

Uh oh!

myankelev commented Feb 10, 2026

Uh oh!

openjdk Bot commented Feb 10, 2026

Uh oh!

openjdk Bot commented Feb 10, 2026

Uh oh!

Reviewers

Assignees

Labels

myankelev commented Jan 9, 2026 •

edited by openjdk Bot

Loading

openjdk Bot commented Jan 9, 2026 •

edited

Loading

openjdk Bot commented Jan 9, 2026 •

edited

Loading

mlbridge Bot commented Jan 14, 2026 •

edited

Loading

wangweij Feb 5, 2026 •

edited

Loading