fixup! fixup! fixup! fixup! fixup! fixup! Parse Error on Out of Range…

… Binary Integers - add tests for 0 bit length and multiple of 4 check - add runtime multiple of 4 length check Deprecation/Compatibility Although still supported via the tunable allowSignedIntegerLength1Bit, it is recommended that schemas be updated to not depend on 1-bit signed binary integers DAFFODIL-2297
apache · Dec 13, 2024 · ce2d163 · ce2d163
1 parent f9ff30f
commit ce2d163
Show file tree

Hide file tree

Showing 3 changed files with 104 additions and 19 deletions.
diff --git a/...-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/PackedBinaryTraits.scala b/...-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/PackedBinaryTraits.scala
@@ -41,25 +41,52 @@ trait PackedBinaryConversion {
   def toBigDecimal(num: Array[Byte], scale: Int): JBigDecimal
 }
 
+trait PackedBinaryLengthCheck {
+  def PE(state: PState, str: String, args: Any*): Unit
+  def checkLengthNotEqualToZero(nBits: Int, start: PState, packedType: String): Boolean = {
+    if (nBits == 0) {
+      PE(
+        start,
+        s"Number of bits %d out of range for a packed $packedType.",
+        nBits
+      )
+      false
+    } else {
+      true
+    }
+  }
+
+  def checkLengthIsMultipleOf4(nBits: Int, start: PState): Boolean = {
+    if ((nBits % 4) != 0) {
+      PE(
+        start,
+        "The given length (%s bits) must be a multiple of 4 when using packed binary formats",
+        nBits
+      )
+      false
+    } else {
+      true
+    }
+  }
+}
+
 abstract class PackedBinaryDecimalBaseParser(
   override val context: ElementRuntimeData,
   binaryDecimalVirtualPoint: Int
 ) extends PrimParser
-  with PackedBinaryConversion {
+  with PackedBinaryConversion
+  with PackedBinaryLengthCheck {
   override lazy val runtimeDependencies = Vector()
 
   protected def getBitLength(s: ParseOrUnparseState): Int
 
   def parse(start: PState): Unit = {
     val nBits = getBitLength(start)
-    if (nBits == 0) {
-      PE(
-        start,
-        "Number of bits %d out of range for a packed decimal.",
-        nBits
-      )
-      return
-    }
+    val lengthEqualsZero = !checkLengthNotEqualToZero(nBits, start, packedType = "decimal")
+    if (lengthEqualsZero) return
+    val lengthNotMultipleOf4 = !checkLengthIsMultipleOf4(nBits, start)
+    if (lengthNotMultipleOf4) return
+
     val dis = start.dataInputStream
 
     if (!dis.isDefinedForLength(nBits)) {
@@ -79,7 +106,8 @@ abstract class PackedBinaryDecimalBaseParser(
 abstract class PackedBinaryIntegerBaseParser(
   override val context: ElementRuntimeData
 ) extends PrimParser
-  with PackedBinaryConversion {
+  with PackedBinaryConversion
+  with PackedBinaryLengthCheck {
   override lazy val runtimeDependencies = Vector()
 
   val signed = {
@@ -93,14 +121,11 @@ abstract class PackedBinaryIntegerBaseParser(
 
   def parse(start: PState): Unit = {
     val nBits = getBitLength(start)
-    if (nBits == 0) {
-      PE(
-        start,
-        "Number of bits %d out of range for a packed integer.",
-        nBits
-      )
-      return
-    }
+    val lengthEqualsZero = !checkLengthNotEqualToZero(nBits, start, packedType = "integer")
+    if (lengthEqualsZero) return
+    val lengthNotMultipleOf4 = !checkLengthIsMultipleOf4(nBits, start)
+    if (lengthNotMultipleOf4) return
+
     val dis = start.dataInputStream
 
     if (!dis.isDefinedForLength(nBits)) {

diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/section13/packed/packed.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/section13/packed/packed.tdml
@@ -124,11 +124,22 @@
   <tdml:defineSchema name="s8">
     <xs:include schemaLocation="/org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
     <dfdl:format ref="ex:GeneralFormat" lengthKind="explicit" encoding="X-DFDL-HEX-MSBF" occursCountKind="implicit"
-    textNumberCheckPolicy="strict" textNumberPadCharacter="0" textNumberJustification="right"
+    textNumberPadCharacter="0" textNumberJustification="right"
     lengthUnits="bits" binaryNumberRep="packed" binaryPackedSignCodes="C D F C" binaryNumberCheckPolicy="strict"/>
 
     <xs:element name="int01" type="xs:int" dfdl:representation="binary" dfdl:length="7" />
 
+    <xs:element name="int02" type="xs:decimal" dfdl:representation="binary" dfdl:length="0" dfdl:binaryDecimalVirtualPoint="0" />
+
+    <xs:element name="int03" dfdl:lengthKind="implicit">
+      <xs:complexType>
+        <xs:sequence>
+          <xs:element name="len" type="xs:unsignedInt" dfdl:length="1" dfdl:lengthUnits="bytes"/>
+          <xs:element name="value" type="xs:decimal" dfdl:representation="binary" dfdl:length="{ ../ex:len}" dfdl:binaryDecimalVirtualPoint="0" />
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
+
   </tdml:defineSchema>
 
   <tdml:defineSchema name="s9">
@@ -416,6 +427,45 @@
     </tdml:errors>
   </tdml:parserTestCase>
 
+  <tdml:parserTestCase name="zeroLengthPackedCharset" root="int02" model="s8"
+      description="Attempting to use a length of 0 bits with IBM4690 Packed Decimal should result in SDE">
+
+    <tdml:document>
+      <tdml:documentPart type="bits">0010101</tdml:documentPart>
+    </tdml:document>
+    <tdml:errors>
+      <tdml:error>Parse Error</tdml:error>
+      <tdml:error>bits 0 out of range</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="runtimeLengthPackedCharset1" root="int03" model="s8"
+      description="Attempting to use a runtime length with IBM4690 Packed Decimal should result in PE">
+
+    <tdml:document>
+      <tdml:documentPart type="byte">00</tdml:documentPart>
+      <tdml:documentPart type="bits">0010101</tdml:documentPart>
+    </tdml:document>
+    <tdml:errors>
+      <tdml:error>Parse Error</tdml:error>
+      <tdml:error>bits 0 out of range</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="runtimeLengthPackedCharset2" root="int03" model="s8"
+      description="Attempting to use a runtime length with IBM4690 Packed Decimal should result in PE">
+
+    <tdml:document>
+      <tdml:documentPart type="byte">07</tdml:documentPart>
+      <tdml:documentPart type="bits">0010101</tdml:documentPart>
+    </tdml:document>
+    <tdml:errors>
+      <tdml:error>Parse Error</tdml:error>
+      <tdml:error>The given length</tdml:error>
+      <tdml:error>must be a multiple of 4 when using packed binary formats</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
   <tdml:parserTestCase name="packedCharset10" root="int01" model="s11"
     description="Attempting to use an alignment of 1 bit with Packed Decimal should result in SDE">
 

diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/section13/packed/TestPacked.scala b/daffodil-test/src/test/scala/org/apache/daffodil/section13/packed/TestPacked.scala
@@ -51,6 +51,16 @@ class TestPacked {
   @Test def testPackedCharset09(): Unit = { runner.runOneTest("packedCharset09") }
   @Test def testPackedCharset10(): Unit = { runner.runOneTest("packedCharset10") }
 
+  @Test def testZeroLengthPackedCharset(): Unit = {
+    runner.runOneTest("zeroLengthPackedCharset")
+  }
+  @Test def testRuntimeLengthPackedCharset1(): Unit = {
+    runner.runOneTest("runtimeLengthPackedCharset1")
+  }
+  @Test def testRuntimeLengthPackedCharset2(): Unit = {
+    runner.runOneTest("runtimeLengthPackedCharset2")
+  }
+
   @Test def testBCDCharset01(): Unit = { runner.runOneTest("bcdCharset01") }
   @Test def testBCDCharset02(): Unit = { runner.runOneTest("bcdCharset02") }
   @Test def testBCDCharset03(): Unit = { runner.runOneTest("bcdCharset03") }