WW-5724 Hand out a clone of the cached MessageFormat (6.x)

core/src/main/java/com/opensymphony/xwork2/util/AbstractLocalizedTextProvider.java

@@ -452,7 +452,8 @@ protected MessageFormat buildMessageFormat(String pattern, Locale locale) {
             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) {

core/src/test/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java

@@ -40,11 +40,15 @@
 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 void testXW404() {
      *
      * @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 ?",