Rework fix for #94, to allow lenience, but test for correct handling

cowtowncoder · cowtowncoder · commit 2b2d794c9627 · 2020-10-21T21:01:38.000-07:00
diff --git a/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/LocalDateTimeDeserializer.java b/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/LocalDateTimeDeserializer.java
@@ -22,13 +22,14 @@
 import java.time.format.DateTimeFormatter;
 
 import com.fasterxml.jackson.annotation.JsonFormat;
+
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.core.JsonToken;
 import com.fasterxml.jackson.core.JsonTokenId;
+
 import com.fasterxml.jackson.databind.DeserializationContext;
 import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.JavaType;
-import com.fasterxml.jackson.databind.util.ClassUtil;
 
 /**
  * Deserializer for Java 8 temporal {@link LocalDateTime}s.
@@ -152,21 +153,25 @@ protected LocalDateTime _fromString(JsonParser p, DeserializationContext ctxt,
             return null;
         }
         try {
-            // 21-Oct-2020, tatu: Removed as per [modules-base#94];
-            //   original flawed change from [modules-base#56]
+            // 21-Oct-2020, tatu: Changed as per [modules-base#94] for 2.12,
+            //    had bad timezone handle change from [modules-base#56]
             if (_formatter == DEFAULT_FORMATTER) {
+                // ... only allow iff lenient mode enabled since
                 // JavaScript by default includes time and zone in JSON serialized Dates (UTC/ISO instant format).
+                // And if so, do NOT use zoned date parsing as that can easily produce
+                // incorrect answer.
                 if (string.length() > 10 && string.charAt(10) == 'T') {
                    if (string.endsWith("Z")) {
+                       if (isLenient()) {
+                           return LocalDateTime.parse(string.substring(0, string.length()-1),
+                                   _formatter);
+                       }
                        JavaType t = getValueType(ctxt);
                        return (LocalDateTime) ctxt.handleWeirdStringValue(t.getRawClass(),
                                string,
-"Invalid value ('%s') for %s: should not contain timezone/offset (define custom `@JsonFormat(pattern=)` if you must accept)",
-string, ClassUtil.getTypeDescription(t));
+"Should not contain offset when 'strict' mode set for property or type (enable 'lenient' handling to allow)"
+                               );
                    }
-//                       return LocalDateTime.ofInstant(Instant.parse(string), ZoneOffset.UTC);
-//                   } else {
-//                       return LocalDateTime.parse(string, DEFAULT_FORMATTER);
                 }
             }
            return LocalDateTime.parse(string, _formatter);
diff --git a/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/ModuleTestBase.java b/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/ModuleTestBase.java
@@ -1,5 +1,6 @@
 package com.fasterxml.jackson.datatype.jsr310;
 
+import java.time.ZoneId;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Map;
@@ -12,6 +13,11 @@
 
 public class ModuleTestBase
 {
+    protected static final ZoneId UTC = ZoneId.of("UTC");
+
+    protected static final ZoneId Z_CHICAGO = ZoneId.of("America/Chicago");
+    protected static final ZoneId Z_BUDAPEST = ZoneId.of("Europe/Budapest");
+    
     public static class NoCheckSubTypeValidator
         extends PolymorphicTypeValidator.Base
     {
diff --git a/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/LocalDateTimeDeserTest.java b/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/LocalDateTimeDeserTest.java
@@ -49,6 +49,14 @@ public class LocalDateTimeDeserTest
 {
     private final static ObjectMapper MAPPER = newMapper();
     private final static ObjectReader READER = MAPPER.readerFor(LocalDateTime.class);
+
+    private final static ObjectMapper STRICT_MAPPER;
+    static {
+        STRICT_MAPPER = newMapper();
+        STRICT_MAPPER.configOverride(LocalDateTime.class)
+            .setFormat(JsonFormat.Value.forLeniency(false));
+    }
+
     private final TypeReference<Map<String, LocalDateTime>> MAP_TYPE_REF = new TypeReference<Map<String, LocalDateTime>>() { };
 
     final static class StrictWrapper {
@@ -155,28 +163,56 @@ public void testDeserializationAsString01() throws Exception
     public void testDeserializationAsString02() throws Exception
     {
         LocalDateTime time = LocalDateTime.of(2013, Month.AUGUST, 21, 9, 22, 57);
-        LocalDateTime value = MAPPER.readValue('"' + time.toString() + '"', LocalDateTime.class);
+        LocalDateTime value = MAPPER.readValue(q(time.toString()), LocalDateTime.class);
         assertEquals("The value is not correct.", time, value);
     }
 
     @Test
     public void testDeserializationAsString03() throws Exception
     {
         LocalDateTime time = LocalDateTime.of(2005, Month.NOVEMBER, 5, 22, 31, 5, 829837);
-        LocalDateTime value = MAPPER.readValue('"' + time.toString() + '"', LocalDateTime.class);
+        LocalDateTime value = MAPPER.readValue(q(time.toString()), LocalDateTime.class);
         assertEquals("The value is not correct.", time, value);
     }
 
-    // [modules-java#94]: Should not include timezone
+    /*
+    /**********************************************************
+    /* Tests for deserializing from textual representation,
+    /* fail cases, leniency checking
+    /**********************************************************
+     */
+
+    // [modules-java#94]: "Z" offset MAY be allowed, requires leniency
     @Test
-    public void testBadDeserializationOfTimeWithTimeZone() throws Exception
+    public void testAllowZuluIfLenient() throws Exception
+    {
+        final LocalDateTime EXP = LocalDateTime.of(2020, Month.OCTOBER, 22, 4, 16, 20, 504000000);
+        final String input = q("2020-10-22T04:16:20.504Z");
+        final ObjectReader r = MAPPER.readerFor(LocalDateTime.class);
+
+        // First, defaults:
+        assertEquals("The value is not correct.", EXP, r.readValue(input));
+
+        // but ensure that global timezone setting doesn't matter
+        LocalDateTime value = r.with(TimeZone.getTimeZone(Z_CHICAGO))
+                .readValue(input);
+        assertEquals("The value is not correct.", EXP, value);
+
+        value = r.with(TimeZone.getTimeZone(Z_BUDAPEST))
+                .readValue(input);
+        assertEquals("The value is not correct.", EXP, value);
+    }
+
+    // [modules-java#94]: "Z" offset not allowed if strict mode
+    @Test
+    public void testFailOnZuluIfStrict() throws Exception
     {
         try {
-            MAPPER.readValue(q("2020-10-22T00:16:20.504Z"), LocalDateTime.class);
-            fail("expected fail");
+            STRICT_MAPPER.readValue(q("2020-10-22T00:16:20.504Z"), LocalDateTime.class);
+            fail("Should not pass");
         } catch (InvalidFormatException e) {
-            verifyException(e, "Invalid value");
-            verifyException(e, "for `java.time.LocalDateTime`");
+            verifyException(e, "Cannot deserialize value of type ");
+            verifyException(e, "Should not contain offset when 'strict' mode");
         }
     }
 
@@ -192,10 +228,9 @@ public void testBadDeserializationAsString01() throws Throwable
         }
     }
 
-        /*
+    /*
     /**********************************************************
     /* Tests for empty string handling
-     */
     /**********************************************************
      */
 
@@ -224,20 +259,17 @@ public void testLenientDeserializeFromEmptyString() throws Exception {
     public void testStrictDeserializeFromEmptyString() throws Exception {
 
         final String key = "datetime";
-        final ObjectMapper mapper = mapperBuilder().build();
-        mapper.configOverride(LocalDateTime.class)
-            .setFormat(JsonFormat.Value.forLeniency(false));
-        final ObjectReader objectReader = mapper.readerFor(MAP_TYPE_REF);
+        final ObjectReader objectReader = STRICT_MAPPER.readerFor(MAP_TYPE_REF);
         final String dateValAsNullStr = null;
 
         // even with strict, null value should be deserialized without throwing an exception
-        String valueFromNullStr = mapper.writeValueAsString(asMap(key, dateValAsNullStr));
+        String valueFromNullStr = STRICT_MAPPER.writeValueAsString(asMap(key, dateValAsNullStr));
         Map<String, LocalDateTime> actualMapFromNullStr = objectReader.readValue(valueFromNullStr);
         assertNull(actualMapFromNullStr.get(key));
 
         String dateValAsEmptyStr = "";
         // TODO: nothing stops us from writing an empty string, maybe there should be a check there too?
-        String valueFromEmptyStr = mapper.writeValueAsString(asMap("date", dateValAsEmptyStr));
+        String valueFromEmptyStr = STRICT_MAPPER.writeValueAsString(asMap("date", dateValAsEmptyStr));
         // with strict, deserializing an empty string is not permitted
         objectReader.readValue(valueFromEmptyStr);
     }
diff --git a/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/OffsetDateTimeDeserTest.java b/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/OffsetDateTimeDeserTest.java
@@ -29,7 +29,6 @@ public class OffsetDateTimeDeserTest
 {
     private static final DateTimeFormatter FORMATTER = DateTimeFormatter.ISO_OFFSET_DATE_TIME;
     private final TypeReference<Map<String, OffsetDateTime>> MAP_TYPE_REF = new TypeReference<Map<String, OffsetDateTime>>() { };
-    private static final ZoneId UTC = ZoneId.of("UTC");
 
     private static final ZoneId Z1 = ZoneId.of("America/Chicago");
 
diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x
@@ -104,6 +104,11 @@ Samantha Williamson (samwill@github)
   * Contributed fix to #148: Allow strict `LocalDate` parsing
    (2.11.0)
 
+Antti S. Lankila (alankila@github)
+  * Reported #94: Deserialization of timestamps with UTC timezone to LocalDateTime
+    doesn't yield correct time
+   (2.12.0)
+
 Joni Syri (jpsyri@github)
   * Reported #165: Problem in serializing negative Duration values
    (2.12.0)
diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -10,6 +10,9 @@ Modules:
 
 2.12.0 (not yet released)
 
+#94: Deserialization of timestamps with UTC timezone to LocalDateTime
+   doesn't yield correct time
+ (reported by Antti L)
 #165: Problem in serializing negative Duration values
  (reported by Joni S)
 #166: Cannot deserialize `OffsetDateTime.MIN` or `OffsetDateTime.MAX` with

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,6 @@ public class OffsetDateTimeDeserTest`
`29`	`29`	`{`
`30`	`30`	`private static final DateTimeFormatter FORMATTER = DateTimeFormatter.ISO_OFFSET_DATE_TIME;`
`31`	`31`	`private final TypeReference<Map<String, OffsetDateTime>> MAP_TYPE_REF = new TypeReference<Map<String, OffsetDateTime>>() { };`
`32`		`- private static final ZoneId UTC = ZoneId.of("UTC");`
`33`	`32`
`34`	`33`	`private static final ZoneId Z1 = ZoneId.of("America/Chicago");`
`35`	`34`