Add annotations and facet check for PrefixLengthType

- currently we silently ignore facets on the prefixLengthType simple type and we don't execute asserts that try to force facet checking, thereby not doing any validation on prefixLengthType. This ticket aims to fix that issue by performing the checks and SDEing when we have statement annotation on the simpleType of the prefixLengthType, which are not supported according to the DFDL workgroup. - adds specific statement annotations to SDE - Added tests to ensure prefix value of 0 causes parse error - Added tests to ensure prefix value which results in zero-length string is allowed - Added tests to ensure prefix value greater than string lengths results in parse error DAFFODIL-2660
apache · Oct 29, 2024 · 357f224 · 357f224
1 parent e5851a7
commit 357f224
Show file tree

Hide file tree

Showing 5 changed files with 271 additions and 0 deletions.
diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala
@@ -191,6 +191,12 @@ trait ElementBaseGrammarMixin
         "%s is specified as a dfdl:prefixLengthType, but specifies a dfdl:trailingSkip other than 0",
         prefixLengthType
       )
+      schemaDefinitionWhen(
+        detachedElementDecl.statements.nonEmpty,
+        "%s is specified as a dfdl:prefixLengthType, but specifies one or more statement annotations (%s)",
+        prefixLengthType,
+        detachedElementDecl.statements.mkString(", ")
+      )
 
       if (
         detachedElementDecl.lengthKind == LengthKind.Prefixed &&

diff --git a/...rser/src/main/scala/org/apache/daffodil/unparsers/runtime1/SpecifiedLengthUnparsers.scala b/...rser/src/main/scala/org/apache/daffodil/unparsers/runtime1/SpecifiedLengthUnparsers.scala
@@ -339,6 +339,20 @@ trait KnownPrefixedLengthUnparserMixin {
 
     plElement.setDataValue(java.lang.Integer.valueOf(adjustedLenInUnits.toInt))
 
+    // do checks on facets expressed on prefixLengthType
+    val optSTRD = plElement.erd.optSimpleTypeRuntimeData
+    if (optSTRD.isDefined) {
+      val strd = optSTRD.get
+      val check = strd.executeCheck(plElement)
+      if (check.isError) {
+        UnparseError(
+          One(state.schemaFileLocation),
+          One(state.currentLocation),
+          s"The calculated value of ${prefixedLengthERD.namedQName} ($adjustedLenInUnits) failed check due to ${check.errMsg}"
+        )
+      }
+    }
+
     // unparse the prefixed length element
     state.currentInfosetNodeStack.push(One(plElement))
     prefixedLengthUnparser.unparse1(state)
@@ -389,6 +403,19 @@ trait CalculatedPrefixedLengthUnparserMixin {
     }
     val adjustedLenInUnits = lenInUnits + prefixedLengthAdjustmentInUnits
     plElem.setDataValue(java.lang.Integer.valueOf(adjustedLenInUnits.toInt))
+    // do checks on facets expressed on prefixLengthType
+    val optSTRD = plElem.erd.optSimpleTypeRuntimeData
+    if (optSTRD.isDefined) {
+      val strd = optSTRD.get
+      val check = strd.executeCheck(plElem)
+      if (check.isError) {
+        UnparseError(
+          One(state.schemaFileLocation),
+          One(state.currentLocation),
+          s"The calculated value of ${elem.namedQName} ($adjustedLenInUnits) failed check due to ${check.errMsg}"
+        )
+      }
+    }
   }
 }
 

diff --git a/...1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/BinaryNumberTraits.scala b/...1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/BinaryNumberTraits.scala
@@ -103,6 +103,18 @@ trait PrefixedLengthParserMixin {
         "Prefixed length result must be non-negative after dfdl:prefixIncludesPrefixLength adjustment , but was: %d",
         adjustedLen
       )
+      // do checks on facets expressed on prefixLengthType
+      val optSTRD = plElement.erd.optSimpleTypeRuntimeData
+      if (optSTRD.isDefined) {
+        val strd = optSTRD.get
+        val check = strd.executeCheck(plElement)
+        if (check.isError) {
+          val pe = state.toProcessingError(
+            s"The value of ${prefixedLengthERD.namedQName} ($parsedLen) failed check due to ${check.errMsg}"
+          )
+          state.setFailed(pe)
+        }
+      }
       adjustedLen
     } else {
       // Return zero if there was an error parsing the prefix length, the caller of this

diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/section12/lengthKind/PrefixedTests.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/section12/lengthKind/PrefixedTests.tdml
@@ -2872,4 +2872,199 @@
     </tdml:infoset>
   </tdml:parserTestCase>
 
+  <tdml:defineSchema name="PrefixFacetsCheck">
+    <xs:include schemaLocation="/org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+
+    <dfdl:format ref="GeneralFormat"
+                 lengthKind="explicit"
+                 lengthUnits="bytes" />
+
+
+    <xs:simpleType name="prefix2" dfdl:length="1">
+      <xs:restriction base="xs:int">
+        <xs:minInclusive value="2"/>
+        <xs:maxInclusive value="4"/>
+      </xs:restriction>
+    </xs:simpleType>
+
+    <xs:simpleType name="prefix3" dfdl:length="1">
+      <xs:restriction base="xs:unsignedByte">
+        <xs:minInclusive value="0"/>
+        <xs:maxInclusive value="4"/>
+      </xs:restriction>
+    </xs:simpleType>
+
+    <xs:element name="field1" type="xs:string"
+                dfdl:lengthKind="prefixed" dfdl:prefixLengthType="prefix2" dfdl:prefixIncludesPrefixLength="yes"/>
+    <xs:element name="field2" type="xs:string"
+                dfdl:lengthKind="prefixed" dfdl:prefixLengthType="prefix2" dfdl:prefixIncludesPrefixLength="no"/>
+    <xs:element name="field3" type="xs:byte" dfdl:representation="binary"
+                dfdl:lengthKind="prefixed" dfdl:prefixLengthType="prefix2" dfdl:prefixIncludesPrefixLength="no"/>
+    <xs:element name="field4" type="xs:string"
+        dfdl:lengthKind="prefixed" dfdl:prefixLengthType="prefix3" dfdl:prefixIncludesPrefixLength="yes"/>
+
+  </tdml:defineSchema>
+
+  <tdml:parserTestCase name="pl_check_prefix_facets_before_use1"
+    root="field2"
+    model="PrefixFacetsCheck">
+    <tdml:document>
+      <tdml:documentPart type="text">5Hello</tdml:documentPart>
+    </tdml:document>
+    <tdml:errors>
+      <tdml:error>failed check</tdml:error>
+      <tdml:error>field2 (prefixLength) (5)</tdml:error>
+      <tdml:error>facet maxInclusive (4)</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="pl_check_prefix_facets_before_use2"
+    root="field1"
+    model="PrefixFacetsCheck">
+    <tdml:document>
+      <tdml:documentPart type="text">6Hello</tdml:documentPart>
+    </tdml:document>
+    <tdml:errors>
+      <tdml:error>failed check</tdml:error>
+      <tdml:error>field1 (prefixLength) (6)</tdml:error>
+      <tdml:error>facet maxInclusive (4)</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
+  <tdml:unparserTestCase name="pl_check_prefix_facets_before_use3"
+    root="field1"
+    model="PrefixFacetsCheck">
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <ex:field1>Hello</ex:field1>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+    <tdml:errors>
+      <tdml:error>failed check</tdml:error>
+      <tdml:error>field1 (6)</tdml:error>
+      <tdml:error>facet maxInclusive (4)</tdml:error>
+    </tdml:errors>
+  </tdml:unparserTestCase>
+
+  <tdml:unparserTestCase name="pl_check_prefix_facets_before_use4"
+    root="field3"
+    model="PrefixFacetsCheck">
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <ex:field3>32</ex:field3>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+    <tdml:errors>
+      <tdml:error>failed check</tdml:error>
+      <tdml:error>field3 (prefixLength) (1)</tdml:error>
+      <tdml:error>facet minInclusive (2)</tdml:error>
+    </tdml:errors>
+  </tdml:unparserTestCase>
+
+  <tdml:parserTestCase name="pl_check_prefix_facets_before_use5"
+      root="field4"
+      model="PrefixFacetsCheck">
+    <tdml:document>
+      <tdml:documentPart type="text">0</tdml:documentPart>
+    </tdml:document>
+    <tdml:errors>
+      <tdml:error>Runtime Schema Definition Error</tdml:error>
+      <tdml:error>Prefixed length result</tdml:error>
+      <tdml:error>after dfdl:prefixIncludesPrefixLength adjustment</tdml:error>
+      <tdml:error>non-negative</tdml:error>
+      <tdml:error>-1</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="pl_check_prefix_facets_before_use6"
+      root="field4"
+      model="PrefixFacetsCheck">
+    <tdml:document>
+      <tdml:documentPart type="text">1H</tdml:documentPart>
+    </tdml:document>
+    <tdml:errors>
+      <tdml:error>Left over data</tdml:error>
+      <tdml:error>8 bit(s) remaining</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="pl_check_prefix_facets_before_use7"
+      root="field4"
+      model="PrefixFacetsCheck">
+    <tdml:document>
+      <tdml:documentPart type="text">2H</tdml:documentPart>
+    </tdml:document>
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <ex:field4>H</ex:field4>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="pl_check_prefix_facets_before_use8"
+      root="field4"
+      model="PrefixFacetsCheck">
+    <tdml:document>
+      <tdml:documentPart type="text">1</tdml:documentPart>
+    </tdml:document>
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <ex:field4></ex:field4>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="pl_check_prefix_facets_before_use9"
+      root="field4"
+      model="PrefixFacetsCheck">
+    <tdml:document>
+      <tdml:documentPart type="text">4He</tdml:documentPart>
+    </tdml:document>
+    <tdml:errors>
+      <tdml:error>Parse Error</tdml:error>
+      <tdml:error>Insufficient bits</tdml:error>
+      <tdml:error>needed 24 bit(s)</tdml:error>
+      <tdml:error>found only 16</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
+
+  <tdml:defineSchema name="AnnotationOnPrefixType">
+    <xs:include schemaLocation="/org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+
+    <dfdl:format ref="GeneralFormat"
+                 lengthKind="explicit"
+                 lengthUnits="bytes" />
+
+
+    <xs:simpleType name="prefix2" dfdl:length="1">
+      <annotation>
+        <appinfo source="http://www.ogf.org/dfdl/">
+          <dfdl:assert>{ dfdl:checkConstraints(.) }</dfdl:assert>
+        </appinfo>
+      </annotation>
+      <xs:restriction base="xs:int">
+        <xs:minInclusive value="0"/>
+        <xs:maxInclusive value="4"/>
+      </xs:restriction>
+    </xs:simpleType>
+
+    <xs:element name="field2" type="xs:string"
+                dfdl:lengthKind="prefixed" dfdl:prefixLengthType="prefix2" dfdl:prefixIncludesPrefixLength="no"/>
+  </tdml:defineSchema>
+
+  <tdml:parserTestCase name="pl_check_prefix_for_annotations"
+    root="field2"
+    model="AnnotationOnPrefixType">
+    <tdml:document>
+      <tdml:documentPart type="text">5Hello</tdml:documentPart>
+    </tdml:document>
+    <tdml:errors>
+      <tdml:error>specifies</tdml:error>
+      <tdml:error>one or more</tdml:error>
+      <tdml:error>statement annotations</tdml:error>
+      <tdml:error>dfdl:assert</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
 </tdml:testSuite>
diff --git a/...test/src/test/scala/org/apache/daffodil/section12/lengthKind/TestLengthKindPrefixed.scala b/...test/src/test/scala/org/apache/daffodil/section12/lengthKind/TestLengthKindPrefixed.scala
@@ -82,6 +82,37 @@ class TestLengthKindPrefixed {
   @Test def test_pl_text_string_txt_bytes_not_enough_prefix_data_includes_backtrack() = {
     runner.runOneTest("pl_text_string_txt_bytes_not_enough_prefix_data_includes_backtrack")
   }
+  // DFDL-2660
+  @Test def test_pl_check_prefix_facets_before_use1() = {
+    runner.runOneTest("pl_check_prefix_facets_before_use1")
+  }
+  @Test def test_pl_check_prefix_facets_before_use2() = {
+    runner.runOneTest("pl_check_prefix_facets_before_use2")
+  }
+  @Test def test_pl_check_prefix_facets_before_use3() = {
+    runner.runOneTest("pl_check_prefix_facets_before_use3")
+  }
+  @Test def test_pl_check_prefix_facets_before_use4() = {
+    runner.runOneTest("pl_check_prefix_facets_before_use4")
+  }
+  @Test def test_pl_check_prefix_facets_before_use5() = {
+    runner.runOneTest("pl_check_prefix_facets_before_use5")
+  }
+  @Test def test_pl_check_prefix_facets_before_use6() = {
+    runner.runOneTest("pl_check_prefix_facets_before_use6")
+  }
+  @Test def test_pl_check_prefix_facets_before_use7() = {
+    runner.runOneTest("pl_check_prefix_facets_before_use7")
+  }
+  @Test def test_pl_check_prefix_facets_before_use8() = {
+    runner.runOneTest("pl_check_prefix_facets_before_use8")
+  }
+  @Test def test_pl_check_prefix_facets_before_use9() = {
+    runner.runOneTest("pl_check_prefix_facets_before_use9")
+  }
+  @Test def test_pl_check_prefix_for_annotations() = {
+    runner.runOneTest("pl_check_prefix_for_annotations")
+  }
   // DFDL-2030, nested prefixed lengths not supported
   // @Test def test_pl_text_string_pl_txt_bytes() = { runner.runOneTest("pl_text_string_pl_txt_bytes") }
   @Test def test_pl_text_int_txt_bytes() = { runner.runOneTest("pl_text_int_txt_bytes") }