Merge pull request #761 from guidograzioli/master

Fix bug GEOT-5036
geotools · Apr 18, 2015 · 081361c · 081361c
2 parents 1423bcf + 3c74ee2
commit 081361c
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 13 deletions.
diff --git a/modules/unsupported/geojson/pom.xml b/modules/unsupported/geojson/pom.xml
@@ -83,6 +83,10 @@
             <version>${project.version}</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+          <groupId>commons-lang</groupId>
+          <artifactId>commons-lang</artifactId>
+        </dependency>
     </dependencies>
 
     <profiles>

diff --git a/modules/unsupported/geojson/src/main/java/org/geotools/geojson/GeoJSONUtil.java b/modules/unsupported/geojson/src/main/java/org/geotools/geojson/GeoJSONUtil.java
@@ -30,12 +30,12 @@
 import java.io.StringReader;
 import java.io.Writer;
 import java.lang.reflect.Proxy;
-import java.text.SimpleDateFormat;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
 import java.util.TimeZone;
 
+import org.apache.commons.lang.time.FastDateFormat;
 import org.geotools.util.Converters;
 import org.json.simple.JSONObject;
 import org.json.simple.parser.JSONParser;
@@ -53,10 +53,10 @@ public class GeoJSONUtil {
     /**
      * Date format (ISO 8601)
      */
-    public static SimpleDateFormat DATE_FORMAT = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ");
-    static {
-        DATE_FORMAT.setTimeZone(TimeZone.getTimeZone("GMT"));
-    }
+    public static final String DATE_FORMAT = "yyyy-MM-dd'T'HH:mm:ss.SSSZ";
+    public static final TimeZone TIME_ZONE = TimeZone.getTimeZone("GMT");
+
+    public static final FastDateFormat dateFormatter = FastDateFormat.getInstance(DATE_FORMAT, TIME_ZONE);
 
     //
     // io
@@ -175,7 +175,7 @@ public static StringBuilder entry(String key, Object value, StringBuilder sb) {
     static StringBuilder literal(Object value, StringBuilder sb) {
         //handle date as special case special case
         if (value instanceof Date) {
-            return string(DATE_FORMAT.format((Date)value), sb);
+            return string(dateFormatter.format((Date)value), sb);
         }
 
         return sb.append(value);

diff --git a/...nsupported/geojson/src/main/java/org/geotools/geojson/feature/FeatureTypeAttributeIO.java b/...nsupported/geojson/src/main/java/org/geotools/geojson/feature/FeatureTypeAttributeIO.java
@@ -1,6 +1,7 @@
 package org.geotools.geojson.feature;
 
 import java.text.ParseException;
+import java.text.SimpleDateFormat;
 import java.util.Date;
 import java.util.HashMap;
 
@@ -42,12 +43,13 @@ public Object parse(String att, String value) {
     static class DateAttributeIO implements AttributeIO {
 
         public String encode(String att, Object value) {
-            return GeoJSONUtil.DATE_FORMAT.format((Date)value);
+            return GeoJSONUtil.dateFormatter.format((Date) value);
         }
 
         public Object parse(String att, String value) {
             try {
-                return GeoJSONUtil.DATE_FORMAT.parse(value);
+                final SimpleDateFormat sdf = new SimpleDateFormat(GeoJSONUtil.DATE_FORMAT);
+                return sdf.parse(value);
             } 
             catch (ParseException e) {
                 throw new RuntimeException(e);

diff --git a/modules/unsupported/geojson/src/test/java/org/geotools/geojson/FeatureJSONExtendedTest.java b/modules/unsupported/geojson/src/test/java/org/geotools/geojson/FeatureJSONExtendedTest.java
@@ -1,12 +1,9 @@
 package org.geotools.geojson;
 
-import static org.geotools.geojson.GeoJSONUtil.DATE_FORMAT;
-
 import java.io.StringWriter;
 import java.text.ParseException;
-import java.util.Arrays;
+import java.text.SimpleDateFormat;
 import java.util.Date;
-import java.util.List;
 
 import org.geotools.feature.simple.SimpleFeatureBuilder;
 import org.geotools.feature.simple.SimpleFeatureTypeBuilder;
@@ -145,7 +142,8 @@ String toDateString(int val) {
 
     Date toDate(int val) {
         try {
-            return DATE_FORMAT.parse(toDateString(val));
+            final SimpleDateFormat sdf = new SimpleDateFormat(GeoJSONUtil.DATE_FORMAT);
+            return sdf.parse(toDateString(val));
         } 
         catch (ParseException e) {
             throw new RuntimeException(e);

diff --git a/modules/unsupported/geojson/src/test/java/org/geotools/geojson/GEOT5036RegressionTest.java b/modules/unsupported/geojson/src/test/java/org/geotools/geojson/GEOT5036RegressionTest.java
@@ -0,0 +1,86 @@
+/*
+ *    GeoTools - The Open Source Java GIS Toolkit
+ *    http://geotools.org
+ *
+ *    (C) 2002-2010, Open Source Geospatial Foundation (OSGeo)
+ *
+ *    This library is free software; you can redistribute it and/or
+ *    modify it under the terms of the GNU Lesser General Public
+ *    License as published by the Free Software Foundation;
+ *    version 2.1 of the License.
+ *
+ *    This library is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *    Lesser General Public License for more details.
+ */
+package org.geotools.geojson;
+
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Random;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+
+import junit.framework.TestCase;
+
+import org.apache.commons.lang.time.FastDateFormat;
+
+/**
+ * Exploit bug GEOT-5036, concurrent use of GeoJSONUtil.
+ * @author Guido Grazioli <guido.grazioli@gmail.com>
+ *
+ */
+public class GEOT5036RegressionTest extends TestCase {
+
+    private final Random rand = new Random();
+    private final Map<Date, Future<String>> expectationMap = new HashMap<Date, Future<String>>();
+
+    public class Task implements Callable<String> {
+        private final Date date;
+
+        public Task(Date d) {
+            this.date = d;
+        }
+
+        @Override
+        public String call() throws Exception {
+            final StringBuilder sb = new StringBuilder();
+            GeoJSONUtil.literal(date, sb);
+            return sb.toString();
+        }
+    }
+
+    @Override
+    public void setUp() throws java.lang.Exception {
+        // perform 50 conversions
+        for (int i = 0; i < 50; i++) {
+            final Date date = new Date(System.currentTimeMillis() - rand.nextInt(100) * 1000 * 3600 * 24);
+            expectationMap.put(date, null);
+        }
+    }
+
+    public void testDateFormatterResults() throws Exception {
+        final FastDateFormat sdf = GeoJSONUtil.dateFormatter;
+
+        // pool with 8 threads
+        final ExecutorService exec = Executors.newFixedThreadPool(8);
+
+        // perform date conversions
+        for (final Date key : expectationMap.keySet()) {
+            final Task task = new Task(key);
+            expectationMap.put(key, exec.submit(task));
+        }
+        exec.shutdown();
+
+        // look at the results
+        for (final Map.Entry<Date, Future<String>> entry : expectationMap.entrySet()) {
+            assertEquals("incorrect date string was returned: ", entry.getValue().get(),
+                    "\"" + sdf.format(entry.getKey()) + "\"");
+        }
+    }
+
+}