This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch support/struts-6-x-x
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/support/struts-6-x-x by this
push:
new bfc73448d WW-5724 fix(core): hand out a clone of the cached
MessageFormat (#1915)
bfc73448d is described below
commit bfc73448dd07616e08b19ca4a419fdf8f09f21a1
Author: Lukasz Lenart <[email protected]>
AuthorDate: Sat Sep 12 18:38:03 2026 +0200
WW-5724 fix(core): hand out a clone of the cached MessageFormat (#1915)
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.
Backport of the main change (#1914) to the 6.x line.
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../xwork2/util/AbstractLocalizedTextProvider.java | 3 +-
.../util/StrutsLocalizedTextProviderTest.java | 60 ++++++++++++++++++++++
2 files changed, 62 insertions(+), 1 deletion(-)
diff --git
a/core/src/main/java/com/opensymphony/xwork2/util/AbstractLocalizedTextProvider.java
b/core/src/main/java/com/opensymphony/xwork2/util/AbstractLocalizedTextProvider.java
index 6b972d923..6571529dc 100644
---
a/core/src/main/java/com/opensymphony/xwork2/util/AbstractLocalizedTextProvider.java
+++
b/core/src/main/java/com/opensymphony/xwork2/util/AbstractLocalizedTextProvider.java
@@ -452,7 +452,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/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java
b/core/src/test/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java
index dea14bec8..bd7454934 100644
---
a/core/src/test/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java
+++
b/core/src/test/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java
@@ -40,11 +40,15 @@ import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.lang.reflect.Field;
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;
/**
@@ -273,6 +277,62 @@ public class StrutsLocalizedTextProviderTest extends
XWorkTestCase {
*
* @since 6.0.0
*/
+ 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("com/opensymphony/xwork2/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 testLocalizedTextProviderClearingMethods() {
TestStrutsLocalizedTextProvider testStrutsLocalizedTextProvider = new
TestStrutsLocalizedTextProvider();
assertTrue("testStrutsLocalizedTextProvider not instance of
AbstractLocalizedTextProvider ?",
