Skip to content

Fix thread safety: replace shared Matcher and SimpleDateFormat instances #1929

Merged
jnioche merged 1 commit into
mainfrom
thread-safety
Jun 5, 2026
Merged

Fix thread safety: replace shared Matcher and SimpleDateFormat instances #1929
jnioche merged 1 commit into
mainfrom
thread-safety

Conversation

@dpol1

@dpol1 dpol1 commented Jun 4, 2026

Copy link
Copy Markdown
Member

SimpleDateFormat and shared Matcher instances are not thread-safe.
In a multi-threaded Storm topology this can cause silent data corruption
or incorrect results under concurrent load.

  • RefreshTag: replace static shared Matcher with a Pattern constant
    and a per-call Matcher instance
  • FileResponse: replace SimpleDateFormat with DateTimeFormatter
    (also use try-with-resources for FileInputStream in the same class)
  • CookieConverter: replace SimpleDateFormat with DateUtils.parseDate();
    also fix a duplicate setExpiryDate() call and add a null guard on the
    parsed date

…ances

- RefreshTag: replace static shared Matcher with Pattern + per-call Matcher
- FileResponse: replace SimpleDateFormat with DateTimeFormatter; use
  try-with-resources for FileInputStream
- CookieConverter: replace SimpleDateFormat with DateUtils.parseDate();
  fix duplicate setExpiryDate() call; add null guard on parsed date
@rzo1 rzo1 added this to the 3.6.1 milestone Jun 4, 2026
@dpol1 dpol1 requested a review from jnioche June 4, 2026 14:13
// check that it hasn't expired?
if (cookie.isExpired(new Date())) {
continue;
Date expirationDate = org.apache.http.client.utils.DateUtils.parseDate(expires);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have dependencies on o.a.httpclient in various places, it is acceptable to have it here too

@jnioche jnioche merged commit 9c62f2c into main Jun 5, 2026
2 checks passed
@jnioche jnioche deleted the thread-safety branch June 5, 2026 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants