You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by sl...@apache.org on 2023/06/15 11:14:39 UTC

[daffodil] branch main updated: Correctly check for errors when parsing text calendars

This is an automated email from the ASF dual-hosted git repository.

slawrence pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/daffodil.git


The following commit(s) were added to refs/heads/main by this push:
     new d6709682a Correctly check for errors when parsing text calendars
d6709682a is described below

commit d6709682afc2646dbf26ed32185b17174a600f3a
Author: Steve Lawrence <sl...@apache.org>
AuthorDate: Wed Jun 14 09:28:56 2023 -0400

    Correctly check for errors when parsing text calendars
    
    The SimpleDateFormat parse() method does not set the error index on the
    ParsePosition if there is an error. Instead, it only sets the normal
    index to the last successful parse position of the input string.
    
    Daffodil currently relies on the error index, which means our error
    checking is incorrect.
    
    Fortunately, in most cases, this doesn't matter because our error
    checking also requires that we parse the entire input string, so if the
    index is zero it wouldn't match the string length and we would still see
    it as an error. The one exception to this is if the input string is zero
    length, in which case we do not correctly error and just create a
    date/time with value zero.
    
    This is fixed by correctly checking index for zero instead of inspecting
    the error index.
    
    This also does a little refactoring to replace the very old cache
    implementation with the much newer Evaluatable for creating
    SimpleDateFormat instances.
    
    DAFFODIL-2821
---
 .../grammar/primitives/PrimitivesDateTime.scala    | 27 ++++++-
 .../runtime1/ConvertTextCalendarUnparser.scala     | 21 ++----
 .../runtime1/processors/EvCalendarLanguage.scala   | 31 ++++++++
 .../processors/parsers/PrimitivesDateTime1.scala   | 86 +++-------------------
 .../runtime1/processors/input/TestICU.scala        | 46 ++++++++++++
 .../section05/simple_types/SimpleTypes.tdml        | 84 ++++++++++++++++-----
 .../section05/simple_types/TestSimpleTypes.scala   |  5 ++
 7 files changed, 191 insertions(+), 109 deletions(-)

diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/primitives/PrimitivesDateTime.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/primitives/PrimitivesDateTime.scala
index 5923fa1d5..23855f011 100644
--- a/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/primitives/PrimitivesDateTime.scala
+++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/primitives/PrimitivesDateTime.scala
@@ -31,6 +31,7 @@ import org.apache.daffodil.lib.schema.annotation.props.gen.CalendarPatternKind
 import org.apache.daffodil.lib.schema.annotation.props.gen.Representation
 import org.apache.daffodil.runtime1.processors.CalendarEv
 import org.apache.daffodil.runtime1.processors.CalendarLanguageEv
+import org.apache.daffodil.runtime1.processors.DateTimeFormatterEv
 import org.apache.daffodil.runtime1.processors.parsers.ConvertBinaryCalendarSecMilliParser
 import org.apache.daffodil.runtime1.processors.parsers.ConvertTextCalendarParser
 import org.apache.daffodil.unparsers.runtime1.ConvertBinaryCalendarSecMilliUnparser
@@ -165,6 +166,12 @@ abstract class ConvertTextCalendarPrimBase(e: ElementBase, guard: Boolean)
     } else {
       p
     }
+
+    schemaDefinitionWhen(
+      patternToCheck.length == 0,
+      "dfdl:calendarPatttern contains no pattern letters",
+    )
+
     patternToCheck.toSeq.foreach(char =>
       if (!validFormatCharacters.contains(char)) {
         if (e.representation == Representation.Binary)
@@ -201,18 +208,34 @@ abstract class ConvertTextCalendarPrimBase(e: ElementBase, guard: Boolean)
     p
   }
 
+  private lazy val dateTimeFormatterEv = {
+    val ev = new DateTimeFormatterEv(
+      calendarEv,
+      localeEv,
+      pattern,
+      e.eci,
+    )
+    ev.compile(e.tunable)
+    ev
+  }
+
   override lazy val parser = new ConvertTextCalendarParser(
     e.elementRuntimeData,
     xsdType,
     prettyType,
     pattern,
     hasTZ,
-    localeEv,
     calendarEv,
+    dateTimeFormatterEv,
   )
 
   override lazy val unparser =
-    new ConvertTextCalendarUnparser(e.elementRuntimeData, pattern, localeEv, calendarEv)
+    new ConvertTextCalendarUnparser(
+      e.elementRuntimeData,
+      pattern,
+      calendarEv,
+      dateTimeFormatterEv,
+    )
 }
 
 case class ConvertTextDatePrim(e: ElementBase) extends ConvertTextCalendarPrimBase(e, true) {
diff --git a/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/unparsers/runtime1/ConvertTextCalendarUnparser.scala b/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/unparsers/runtime1/ConvertTextCalendarUnparser.scala
index 0f89e5bc0..c71ffbd1c 100644
--- a/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/unparsers/runtime1/ConvertTextCalendarUnparser.scala
+++ b/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/unparsers/runtime1/ConvertTextCalendarUnparser.scala
@@ -21,26 +21,23 @@ import org.apache.daffodil.lib.calendar.DFDLCalendar
 import org.apache.daffodil.lib.exceptions.Assert
 import org.apache.daffodil.lib.util.Misc
 import org.apache.daffodil.runtime1.processors.CalendarEv
-import org.apache.daffodil.runtime1.processors.CalendarLanguageEv
+import org.apache.daffodil.runtime1.processors.DateTimeFormatterEv
 import org.apache.daffodil.runtime1.processors.ElementRuntimeData
-import org.apache.daffodil.runtime1.processors.parsers.ConvertTextCalendarProcessorBase
 import org.apache.daffodil.runtime1.processors.unparsers._
 
 import com.ibm.icu.util.Calendar
-import com.ibm.icu.util.ULocale
 
 case class ConvertTextCalendarUnparser(
-  erd: ElementRuntimeData,
+  override val context: ElementRuntimeData,
   pattern: String,
-  localeEv: CalendarLanguageEv,
   calendarEv: CalendarEv,
-) extends ConvertTextCalendarProcessorBase(erd, pattern)
-  with TextPrimUnparser {
+  dateTimeFormatterEv: DateTimeFormatterEv,
+) extends TextPrimUnparser {
 
   /**
    * Primitive unparsers must override runtimeDependencies
    */
-  override lazy val runtimeDependencies = Vector(localeEv, calendarEv)
+  override lazy val runtimeDependencies = Vector(calendarEv, dateTimeFormatterEv)
 
   def unparse(state: UState): Unit = {
 
@@ -59,8 +56,6 @@ case class ConvertTextCalendarUnparser(
         )
     }
 
-    val locale: ULocale = localeEv.evaluate(state)
-
     val calendarOrig: Calendar = calendarEv.evaluate(state)
 
     // The clear() here actually shouldn't be necessary since we call clear()
@@ -72,11 +67,7 @@ case class ConvertTextCalendarUnparser(
     // the case.
     calendarOrig.clear()
 
-    // It's important here to use the calendarOrig that results from
-    // calendarEv.evaluate() since the tlDataFormatter is a cache the uses
-    // reference equality. For everything else we want to use a clone for
-    // reasons described below.
-    val df = tlDataFormatter(locale, calendarOrig)
+    val df = dateTimeFormatterEv.evaluate(state).get
 
     // When we evaluate calendarEV, if it is a constant we will always get back
     // the same Calendar object. Because of this it is important here to clone
diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/EvCalendarLanguage.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/EvCalendarLanguage.scala
index 44cae20ef..c1f65a6a8 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/EvCalendarLanguage.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/EvCalendarLanguage.scala
@@ -21,6 +21,7 @@ import org.apache.daffodil.lib.cookers.Converter
 import org.apache.daffodil.lib.exceptions._
 import org.apache.daffodil.runtime1.dsom._
 
+import com.ibm.icu.text.SimpleDateFormat
 import com.ibm.icu.util.Calendar
 import com.ibm.icu.util.TimeZone
 import com.ibm.icu.util.ULocale
@@ -96,3 +97,33 @@ class CalendarEv(
     cal
   }
 }
+
+class DateTimeFormatterEv(
+  calendarEv: CalendarEv,
+  localeEv: CalendarLanguageEv,
+  pattern: String,
+  eci: DPathElementCompileInfo,
+) extends Evaluatable[ThreadLocal[SimpleDateFormat]](eci)
+  with InfosetCachedEvaluatable[ThreadLocal[SimpleDateFormat]] {
+
+  override lazy val runtimeDependencies = Seq(localeEv)
+
+  override def compute(state: ParseOrUnparseState) = {
+    val calendar = calendarEv.evaluate(state)
+    val locale = localeEv.evaluate(state)
+
+    // As per ICU4J documentation, "Date formats are not synchronized. If multiple threads
+    // access a format concurrently, it must be synchronized externally." Rather than
+    // synchronzing, we create a ThreadLocal so each thread gets their own copy of the
+    // SimpleDateFormat
+    val dateFormatTL = new ThreadLocal[SimpleDateFormat] with Serializable {
+      override def initialValue = {
+        val formatter = new SimpleDateFormat(pattern, locale)
+        formatter.setCalendar(calendar)
+        formatter.setLenient(true) // TODO: should this use calendarCheckPolicy?
+        formatter
+      }
+    }
+    dateFormatTL
+  }
+}
diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/PrimitivesDateTime1.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/PrimitivesDateTime1.scala
index 729fb42b3..efbc212f3 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/PrimitivesDateTime1.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/PrimitivesDateTime1.scala
@@ -26,78 +26,22 @@ import org.apache.daffodil.lib.exceptions.Assert
 import org.apache.daffodil.lib.schema.annotation.props.gen.BinaryCalendarRep
 import org.apache.daffodil.runtime1.dsom.TunableLimitExceededError
 import org.apache.daffodil.runtime1.processors.CalendarEv
-import org.apache.daffodil.runtime1.processors.CalendarLanguageEv
+import org.apache.daffodil.runtime1.processors.DateTimeFormatterEv
 import org.apache.daffodil.runtime1.processors.ElementRuntimeData
-import org.apache.daffodil.runtime1.processors.Processor
 
-import com.ibm.icu.text.SimpleDateFormat
 import com.ibm.icu.util.Calendar
-import com.ibm.icu.util.ULocale
-
-abstract class ConvertTextCalendarProcessorBase(
-  override val context: ElementRuntimeData,
-  pattern: String,
-) extends Processor {
-  // The dfdl:calendarLanguage property can be a runtime-valued expression.
-  // Hence, locale and calendar, derived from it, can also be runtime-valued.
-  //
-  // To parse we need a SimpleDateFormat, and these are
-  // (1) not thread safe - so need to be a thread local
-  // (2) must be initialized passing the runtime-valued locale and calendar.
-  //
-  // Now, fact is, while this could be runtime-valued, even if it is, it is very unlikely
-  // to be changing a lot. So it's a shame to endlessly allocate SimpleDateFormat objects
-  //
-  // So we have a 1 slot cache here. We memorize the locale and calendar and associated
-  // thread local, and return that thread local if the locale and calendar are the same.
-  //
-  // Conservatively, they need to be the exact same object, not just equivalent in the "==" sense.
-  //
-  // This won't be great if some format really is switching among locales and calendars
-  // so that more than one of them really ought to be cached, but that's sufficiently unlikely
-  // that this trivial scheme will do for now.
-  //
-  private final class Cache(var cache: (ULocale, Calendar, SimpleDateFormat))
-
-  private object tlCache extends ThreadLocal[Cache] {
-    override def initialValue = new Cache(null)
-  }
-
-  /**
-   * tlDataFormatter can be runtime valued, as it depends on both locale and calendar
-   * each of which can be runtime valued.
-   */
-  final protected def tlDataFormatter(locale: ULocale, calendar: Calendar) = {
-    val tl = tlCache.get
-    val cache = tl.cache
-    if ((cache ne null) && (locale eq cache._1) && (calendar eq cache._2)) {
-      // cache hit. Same formatter will do
-      cache._3
-    } else {
-      // As per ICU4J documentation, "Date formats are not synchronized. If
-      // multiple threads access a format concurrently, it must be synchronized
-      // externally."
-      val formatter = new SimpleDateFormat(pattern, locale)
-      formatter.setCalendar(calendar)
-      formatter.setLenient(true)
-      tl.cache = (locale, calendar, formatter)
-      formatter
-    }
-  }
-}
 
 case class ConvertTextCalendarParser(
-  erd: ElementRuntimeData,
+  override val context: ElementRuntimeData,
   xsdType: String,
   prettyType: String,
   pattern: String,
   hasTZ: Boolean,
-  localeEv: CalendarLanguageEv,
   calendarEv: CalendarEv,
-) extends ConvertTextCalendarProcessorBase(erd, pattern)
-  with TextPrimParser {
+  dateTimeFormatterEv: DateTimeFormatterEv,
+) extends TextPrimParser {
 
-  override lazy val runtimeDependencies = Vector(localeEv, calendarEv)
+  override lazy val runtimeDependencies = Vector(calendarEv, dateTimeFormatterEv)
 
   def parse(start: PState): Unit = {
     val node = start.simpleElement
@@ -107,8 +51,6 @@ case class ConvertTextCalendarParser(
 
     val pos = new ParsePosition(0)
 
-    val locale: ULocale = localeEv.evaluate(start)
-
     val calendarOrig: Calendar = calendarEv.evaluate(start)
 
     // The clear here actually shouldn't be necessary since we call clear()
@@ -120,17 +62,13 @@ case class ConvertTextCalendarParser(
     // the case.
     calendarOrig.clear()
 
-    // It's important here to use the calendarOrig that results from
-    // calendarEv.evaluate() since the tlDataFormatter is a cache the uses
-    // reference equality. For everything else we want to use a clone for
-    // reasons described below.
-    val df = tlDataFormatter(locale, calendarOrig)
+    val df = dateTimeFormatterEv.evaluate(start).get
 
     // When we evaluate calendarEV, if it is a constant we will always get back
     // the same Calendar object. Because of this it is important here to clone
     // this calendar and always use the clone below for two reasons:
     //
-    // 1) The below code will modify modify the calendar object based on the
+    // 1) The below code will modify the calendar object based on the
     //    value of the parsed string. Any changes to the object will persist
     //    and could affect future parses. By cloning, we ensure we do not
     //    modify the original calendar object.
@@ -143,10 +81,10 @@ case class ConvertTextCalendarParser(
     df.setCalendar(calendar)
     df.parse(str, calendar, pos);
 
-    // Verify that what was parsed was what was passed exactly in byte count
-    // Use pos to verify all characters consumed & check for errors
-    if (pos.getIndex != str.length || pos.getErrorIndex >= 0) {
-      val errIndex = if (pos.getErrorIndex >= 0) pos.getErrorIndex else pos.getIndex
+    // Verify that we did not fail to parse and that we consumed the entire string. Note that
+    // getErrorIndex is never set and is always -1. Only a getIndex value of zero means there
+    // was an error
+    if (pos.getIndex == 0 || pos.getIndex != str.length) {
       PE(start, "Unable to parse xs:%s from text: %s", xsdType, str)
       return
     }
@@ -163,7 +101,7 @@ case class ConvertTextCalendarParser(
         ) < start.tunable.minValidYear)
       )
         throw new TunableLimitExceededError(
-          erd.schemaFileLocation,
+          context.schemaFileLocation,
           "Year value of %s is not within the limits of the tunables minValidYear (%s) and maxValidYear (%s)",
           calendar.get(Calendar.YEAR),
           start.tunable.minValidYear,
diff --git a/daffodil-runtime1/src/test/scala/org/apache/daffodil/runtime1/processors/input/TestICU.scala b/daffodil-runtime1/src/test/scala/org/apache/daffodil/runtime1/processors/input/TestICU.scala
index fdb5d3aca..f3b053547 100644
--- a/daffodil-runtime1/src/test/scala/org/apache/daffodil/runtime1/processors/input/TestICU.scala
+++ b/daffodil-runtime1/src/test/scala/org/apache/daffodil/runtime1/processors/input/TestICU.scala
@@ -73,6 +73,52 @@ class TestICU {
     r.foreach(parseFractionalSeconds)
   }
 
+  /**
+   * This test shows that error index is never set when parsing with a SimpleDateFormat. If ICU
+   * ever changes this to use error index, this test should fail.
+   *
+   * On success, ParsePosition.getIndex is set to where the parse finished. On error,
+   * ParsePosition.getIndex is set to zero and ParsePoition.getErrorIndex is -1 (unset)
+   */
+  @Test def test_simpleDateFormatParse() = {
+    def parseWithPattern(pattern: String, data: String): ParsePosition = {
+      val df = new SimpleDateFormat(pattern)
+      val cal = Calendar.getInstance()
+      val pos = pp
+      df.parse(data, cal, pos)
+      pos
+    }
+
+    {
+      // success, consumes all data
+      val pos = parseWithPattern("HHmm", "1122")
+      assertEquals(4, pos.getIndex)
+      assertEquals(-1, pos.getErrorIndex)
+    }
+
+    {
+      // success, parses only first 4 characters
+      val pos = parseWithPattern("HHmm", "1122extra")
+      assertEquals(4, pos.getIndex)
+      assertEquals(-1, pos.getErrorIndex)
+    }
+
+    {
+      // failure, only partially correct data
+      val pos = parseWithPattern("HHmm", "11cd")
+      assertEquals(2, pos.getIndex)
+      assertEquals(-1, pos.getErrorIndex)
+    }
+
+    {
+      // failure, empty string
+      val pos = parseWithPattern("HHmm", "")
+      assertEquals(0, pos.getIndex)
+      assertEquals(-1, pos.getErrorIndex)
+    }
+
+  }
+
   // The three following tests show an old ICU bug where if the decimal pattern
   // uses scientific notation and the number to format/unparse is positive
   // infinity, negative infinity, or not a number, ICU would include the
diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/section05/simple_types/SimpleTypes.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/section05/simple_types/SimpleTypes.tdml
index 4f3596e94..894a7d96e 100644
--- a/daffodil-test/src/test/resources/org/apache/daffodil/section05/simple_types/SimpleTypes.tdml
+++ b/daffodil-test/src/test/resources/org/apache/daffodil/section05/simple_types/SimpleTypes.tdml
@@ -5123,6 +5123,15 @@
       </xs:complexType>
     </xs:element>
 
+    <xs:element name="time33">
+      <xs:complexType>
+        <xs:sequence>
+          <xs:element name="e1" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="4" />
+          <xs:element name="e2" type="xs:time" minOccurs="0" dfdl:lengthKind="delimited" dfdl:calendarPatternKind="explicit" dfdl:calendarPattern="HHmm" />
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
+
     <xs:element name="dateTimeImp" type="xs:dateTime" dfdl:calendarPatternKind="implicit" dfdl:lengthKind="delimited" />
     <xs:element name="dateTime01" type="xs:dateTime" dfdl:calendarPatternKind="explicit" dfdl:calendarPattern="'It is 'hh:mmaa' on the 'dd'st of 'MMM', year 'yyyy"
                 dfdl:lengthKind="explicit" dfdl:length="44" 
@@ -5716,18 +5725,17 @@
      Test Name: dateEpochFillIn2
         Schema: dateTimeSchema
           Root: date28
-       Purpose: This test demonstrates that if a formatting character is not provided, the time is filled in with the epoch time.
+       Purpose: This test demonstrates that if a formatting character is not provided it is an SDE
 -->
   
   <tdml:parserTestCase name="dateEpochFillIn2" root="date28"
     model="dateTimeSchema" description="Section 13 Simple Types - epoch fill in - DFDL-13-164R">
 
     <tdml:document><![CDATA[]]></tdml:document>
-    <tdml:infoset>
-      <tdml:dfdlInfoset>
-        <date28>1970-01-01</date28>
-      </tdml:dfdlInfoset>
-    </tdml:infoset>
+    <tdml:errors>
+      <tdml:error>Schema Definition Error</tdml:error>
+      <tdml:error>dfdl:calendarPatttern contains no pattern letters</tdml:error>
+    </tdml:errors>
   </tdml:parserTestCase>
 
 <!--
@@ -7197,7 +7205,7 @@
   
   <!--
     Test name: time_calendarTimeZone_EST
-       Schema: constructorSchema
+       Schema: dateTimeSchema
          Root: time32
       Purpose: This test demonstrates the ability to construct a xs:time object
                with a time zone specified.
@@ -7219,6 +7227,46 @@
       </tdml:dfdlInfoset>
     </tdml:infoset>
   </tdml:parserTestCase>
+
+  <!--
+    Test name: time_optional_empty
+       Schema: dateTimeSchema
+         Root: time33
+      Purpose: This test demonstrates parsing an empty optional time field
+  -->
+
+  <tdml:parserTestCase name="time_optional_empty" root="time33" model="dateTimeSchema"
+    description="Section 13 - Simple Types - xs:time calendarTimeZone EST  - DFDL-13-XXXR">
+
+    <tdml:document>
+      <tdml:documentPart type="text">ABCD</tdml:documentPart>
+    </tdml:document>
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <time33>
+          <e1>ABCD</e1>
+        </time33>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:parserTestCase>
+
+  <!--
+    Test name: time_optional_invalid
+       Schema: dateTimeSchema
+         Root: time33
+      Purpose: This test demonstrates parsing an non-well-formed optional time field
+  -->
+
+  <tdml:parserTestCase name="time_optional_nonWellFormed" root="time33" model="dateTimeSchema"
+    description="Section 13 - Simple Types - xs:time calendarTimeZone EST  - DFDL-13-XXXR">
+
+    <tdml:document>
+      <tdml:documentPart type="text">ABCDaaaa</tdml:documentPart>
+    </tdml:document>
+    <tdml:errors>
+      <tdml:error>Left over data</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
   
   <!--
     Test name: date_calendarTimeZone_EmptyString
@@ -7349,18 +7397,18 @@
         Schema: dateTimeSchema
           Root: time23
        Purpose: This test demonstrates that for any pattern that omits components,
-                the values for the omitted components are supplied from the Unix epoch 1970-01-01T00:00:00.000
+                the values for the omitted components are supplied from the Unix epoch 1970-01-01T00:00:00.000.
+                It is an SDE if the property contains no pattern letters
 -->
   
   <tdml:parserTestCase name="epochFillIn2" root="time23"
     model="dateTimeSchema" description="Section 13 Simple Types - Patterns with ommitted components - DFDL-13-164R">
 
     <tdml:document><![CDATA[]]></tdml:document>
-    <tdml:infoset>
-      <tdml:dfdlInfoset>
-        <time23>00:00:00</time23>
-      </tdml:dfdlInfoset>
-    </tdml:infoset>
+    <tdml:errors>
+      <tdml:error>Schema Definition Error</tdml:error>
+      <tdml:error>dfdl:calendarPatttern contains no pattern letters</tdml:error>
+    </tdml:errors>
   </tdml:parserTestCase>
 
 <!--
@@ -7369,17 +7417,17 @@
           Root: time24
        Purpose: This test demonstrates that for any pattern that omits components,
                 the values for the omitted components are supplied from the Unix epoch 1970-01-01T00:00:00.000
+                It is an SDE if the property contains no pattern letters
 -->
   
   <tdml:parserTestCase name="epochFillIn3" root="time24"
     model="dateTimeSchema" description="Section 13 Simple Types - Patterns with ommitted components - DFDL-13-164R">
 
     <tdml:document><![CDATA[.]]></tdml:document>
-    <tdml:infoset>
-      <tdml:dfdlInfoset>
-        <time24>00:00:00</time24>
-      </tdml:dfdlInfoset>
-    </tdml:infoset>
+    <tdml:errors>
+      <tdml:error>Schema Definition Error</tdml:error>
+      <tdml:error>dfdl:calendarPatttern contains no pattern letters</tdml:error>
+    </tdml:errors>
   </tdml:parserTestCase>
 
 <!--
diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/section05/simple_types/TestSimpleTypes.scala b/daffodil-test/src/test/scala/org/apache/daffodil/section05/simple_types/TestSimpleTypes.scala
index 63cb1cdcf..283830a09 100644
--- a/daffodil-test/src/test/scala/org/apache/daffodil/section05/simple_types/TestSimpleTypes.scala
+++ b/daffodil-test/src/test/scala/org/apache/daffodil/section05/simple_types/TestSimpleTypes.scala
@@ -863,4 +863,9 @@ class TestSimpleTypes {
     runner.runOneTest("hexBinary_specifiedLengthUnaligned")
   }
 
+  @Test def test_time_optional_empty(): Unit = { runner.runOneTest("time_optional_empty") }
+  @Test def test_time_optional_nonWellFormed(): Unit = {
+    runner.runOneTest("time_optional_nonWellFormed")
+  }
+
 }