Skip to content

Make the shared Key.SDF date formatter thread-safe (CodeQL java/thread-unsafe-dateformat)#228

Open
vharseko wants to merge 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:features/codeql-thread-unsafe-dateformat
Open

Make the shared Key.SDF date formatter thread-safe (CodeQL java/thread-unsafe-dateformat)#228
vharseko wants to merge 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:features/codeql-thread-unsafe-dateformat

Conversation

@vharseko

@vharseko vharseko commented Jul 8, 2026

Copy link
Copy Markdown
Member

Summary

Resolves the java/thread-unsafe-dateformat CodeQL alert on Key.SDF.

Key.SDF was a public final static SimpleDateFormat. SimpleDateFormat
is not thread-safe — concurrent format() / parse() calls mutate its
internal Calendar and can produce garbled text, wrong dates, or
ArrayIndexOutOfBoundsException. This single shared instance is reachable
from several classes that run on multiple threads:

  • Key.decodeDisplayable / Key.toString
  • Value.toString
  • KeyParser (parse, date keys)
  • com.persistit.ui.ValueInspectorTreeNode (Swing)

Fix

Wrap the formatter in a ThreadLocal so each thread gets its own instance:

public final static ThreadLocal<SimpleDateFormat> SDF = ThreadLocal
        .withInitial(() -> new SimpleDateFormat("yyyyMMddHHmmss.SSSZ"));

and update the four call sites to Key.SDF.get().format(...) /
Key.SDF.get().parse(...).

The date pattern is unchanged, so formatting and parsing behaviour is
identical for every input; only the shared mutable state is removed.

Note

Key.SDF is a public field, so its compile-time type changes from
SimpleDateFormat to ThreadLocal<SimpleDateFormat>. All in-repo callers
are updated in this PR. Any external caller would replace Key.SDF.format(d)
with Key.SDF.get().format(d).

The other, private static SimpleDateFormat fields in the tree
(JournalTool, VolumeHeader, CheckpointManager, and the AbstractSuite
test) were not flagged by CodeQL and are left unchanged.

Testing

mvn -o -pl persistit/core,persistit/ui -am compile — BUILD SUCCESS.

…d-unsafe-dateformat)

Key.SDF was a public static SimpleDateFormat shared across all threads.
SimpleDateFormat is not thread-safe: concurrent format()/parse() calls
corrupt its internal Calendar state, producing garbled output, wrong
dates, or ArrayIndexOutOfBoundsException. Key.SDF is reachable from
Key.decodeDisplayable/toString, Value.toString, KeyParser and the
Swing ValueInspectorTreeNode, all of which can run on multiple threads.

Wrap the formatter in a ThreadLocal so each thread gets its own
SimpleDateFormat instance. The pattern ("yyyyMMddHHmmss.SSSZ") and thus
the formatting/parsing semantics are unchanged; only the shared mutable
state is removed. Update the four call sites to Key.SDF.get().

The remaining private static SimpleDateFormat fields (JournalTool,
VolumeHeader, CheckpointManager, and the AbstractSuite test) were not
flagged by CodeQL and are left unchanged.
@vharseko vharseko added bug java codeql CodeQL static-analysis findings labels Jul 8, 2026
@vharseko vharseko requested a review from maximthomas July 8, 2026 12:42
@vharseko vharseko removed the java label Jul 8, 2026
@vharseko vharseko force-pushed the features/codeql-thread-unsafe-dateformat branch from ef15eb9 to ea9c6bd Compare July 10, 2026 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug codeql CodeQL static-analysis findings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants