This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/main by this push:
new 6829de72c WW-5724 fix(core): hand out a clone of the cached
MessageFormat (#1914)
6829de72c is described below
commit 6829de72ce6a9b332066c96f9c8cb3f091f1952e
Author: Lukasz Lenart <[email protected]>
AuthorDate: Sat Sep 12 18:37:56 2026 +0200
WW-5724 fix(core): hand out a clone of the cached MessageFormat (#1914)
AbstractLocalizedTextProvider caches MessageFormat instances by pattern
and locale and returned the cached instance itself from
buildMessageFormat. MessageFormat is not thread-safe, so concurrent
callers rendering the same message formatted through one shared
instance; with a date or time sub-format that produced output for the
wrong argument or an exception.
buildMessageFormat now returns a clone of the cached instance, so the
cached entry is only ever a template and is never formatted directly.
MessageFormat.clone() deep-copies the sub-formats. The pattern-parsing
cache is kept; measured against a fresh instance per call and against
synchronizing on the shared instance, cloning is the cheapest of the
three.
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../text/AbstractLocalizedTextProvider.java | 3 +-
.../text/StrutsLocalizedTextProviderTest.java | 60 ++++++++++++++++++++++
2 files changed, 62 insertions(+), 1 deletion(-)
diff --git
a/core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java
b/core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java
index 6d4a88ea8..9d8b7b851 100644
---
a/core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java
+++
b/core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java
@@ -416,7 +416,8 @@ abstract class AbstractLocalizedTextProvider implements
LocalizedTextProvider {
messageFormats.put(key, format);
}
- return format;
+ // MessageFormat is not thread-safe; the cached instance is a template
that is never formatted directly
+ return (MessageFormat) format.clone();
}
protected String formatWithNullDetection(MessageFormat mf, Object[] args) {
diff --git
a/core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java
b/core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java
index dd836c115..a63c331b2 100644
---
a/core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java
+++
b/core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java
@@ -36,11 +36,15 @@ import org.apache.struts2.util.Bar;
import org.apache.struts2.util.ValueStack;
import java.text.DateFormat;
+import java.text.MessageFormat;
import java.text.ParseException;
import java.util.Date;
import java.util.HashMap;
import java.util.Locale;
import java.util.ResourceBundle;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
public class StrutsLocalizedTextProviderTest extends XWorkTestCase {
@@ -574,6 +578,62 @@ public class StrutsLocalizedTextProviderTest extends
XWorkTestCase {
assertEquals("Miss cache grew on repeat ?", 1,
provider.classHierarchyCacheSize());
}
+ public void testBuildMessageFormatDoesNotHandOutTheCachedInstance() {
+ TestStrutsLocalizedTextProvider provider = new
TestStrutsLocalizedTextProvider();
+
+ MessageFormat first = provider.buildMessageFormat("{0,date,short}",
Locale.US);
+ MessageFormat second = provider.buildMessageFormat("{0,date,short}",
Locale.US);
+
+ assertNotSame("cached MessageFormat shared between callers ?", first,
second);
+ assertEquals("pattern not cached once ?", 1,
provider.messageFormatsSize());
+ }
+
+ public void testConcurrentDateFormattingDoesNotMixArguments() throws
Exception {
+
localizedTextProvider.addDefaultResourceBundle("org/apache/struts2/util/LocalizedTextUtilTest");
+ Date[] dates = {
+ DateFormat.getDateInstance(DateFormat.SHORT,
Locale.US).parse("01/01/2015"),
+ DateFormat.getDateInstance(DateFormat.SHORT,
Locale.US).parse("02/02/2020"),
+ };
+ String[] expected = {"1/1/15", "2/2/20"};
+ assertEquals(expected[0],
localizedTextProvider.findDefaultText("test.format.date", Locale.US, new
Object[]{dates[0]}));
+ assertEquals(expected[1],
localizedTextProvider.findDefaultText("test.format.date", Locale.US, new
Object[]{dates[1]}));
+
+ int threads = 4;
+ int iterations = 20000;
+ AtomicInteger wrong = new AtomicInteger();
+ AtomicReference<String> sample = new AtomicReference<>();
+ AtomicReference<Throwable> thrown = new AtomicReference<>();
+ CountDownLatch start = new CountDownLatch(1);
+ Thread[] workers = new Thread[threads];
+ for (int t = 0; t < threads; t++) {
+ final int which = t % 2;
+ workers[t] = new Thread(() -> {
+ try {
+ start.await();
+ for (int i = 0; i < iterations; i++) {
+ String out =
localizedTextProvider.findDefaultText("test.format.date", Locale.US, new
Object[]{dates[which]});
+ if (!expected[which].equals(out)) {
+ wrong.incrementAndGet();
+ sample.compareAndSet(null, "expected <" +
expected[which] + "> but was <" + out + ">");
+ }
+ }
+ } catch (Throwable e) {
+ thrown.compareAndSet(null, e);
+ }
+ });
+ }
+ for (Thread worker : workers) {
+ worker.start();
+ }
+ start.countDown();
+ for (Thread worker : workers) {
+ worker.join();
+ }
+
+ assertNull("formatting threw under concurrency ?", thrown.get());
+ assertEquals("another caller's argument rendered: " + sample.get(), 0,
wrong.get());
+ }
+
public void testFormattingIsPerCallNotCached() {
TestStrutsLocalizedTextProvider provider = new
TestStrutsLocalizedTextProvider();
ValueStack valueStack = ActionContext.getContext().getValueStack();
