Address some comments from review.

Rebased on PR #1337
apache · Dec 18, 2024 · 416bf4f · 416bf4f
1 parent 9cde7f5
commit 416bf4f
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 9 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
@@ -26,7 +26,8 @@ import org.apache.daffodil.lib.equality.TypeEqual
 import org.apache.daffodil.lib.exceptions.Assert
 import org.apache.daffodil.lib.util.Maybe
 import org.apache.daffodil.lib.util.MaybeChar
-import org.apache.daffodil.runtime1.dpath.NodeInfo
+import org.apache.daffodil.runtime1.dpath.InvalidPrimitiveDataException
+import org.apache.daffodil.runtime1.dpath.NodeInfo.PrimType
 import org.apache.daffodil.runtime1.infoset.DataValue.DataValueNumber
 import org.apache.daffodil.runtime1.processors.ElementRuntimeData
 import org.apache.daffodil.runtime1.processors.FieldDFAParseEv
@@ -41,7 +42,7 @@ trait PackedBinaryConversion {
   def toBigInteger(num: Array[Byte]): JBigInteger
   def toBigDecimal(num: Array[Byte], scale: Int): JBigDecimal
 
-  def toInteger(context: ElementRuntimeData, num: Array[Byte]): DataValueNumber = {
+  def toPrimitiveType(context: ElementRuntimeData, num: Array[Byte]): DataValueNumber = {
     context.optPrimType.get match {
       case pn: PrimType.PrimNumeric => pn.fromNumber(toBigInteger(num))
       // Non-numeric types such as Time can still use these funcitons and
@@ -50,7 +51,7 @@ trait PackedBinaryConversion {
     }
   }
 
-  def toDecimal(context: ElementRuntimeData, num: Array[Byte], scale: Int): DataValueNumber = {
+  def toPrimitiveType(context: ElementRuntimeData, num: Array[Byte], scale: Int): DataValueNumber = {
     context.optPrimType.get match {
       case pn: PrimType.PrimNumeric => pn.fromNumber(toBigDecimal(num, scale))
       // Non-numeric types such as Time can still use these funcitons and
@@ -114,10 +115,11 @@ abstract class PackedBinaryDecimalBaseParser(
     }
 
     try {
-      val dec = toDecimal(context, dis.getByteArray(nBits, start), binaryDecimalVirtualPoint)
+      val dec = toPrimitiveType(context, dis.getByteArray(nBits, start), binaryDecimalVirtualPoint)
       start.simpleElement.setDataValue(dec)
     } catch {
       case n: NumberFormatException => PE(start, "Error in packed data: \n%s", n.getMessage())
+      case i: InvalidPrimitiveDataException => PE(start, "Error in packed data: \n%s", i.getMessage())
     }
   }
 }
@@ -131,7 +133,7 @@ abstract class PackedBinaryIntegerBaseParser(
 
   val signed = {
     context.optPrimType.get match {
-      case n: NodeInfo.PrimType.PrimNumeric => n.isSigned
+      case n: PrimType.PrimNumeric => n.isSigned
       // context.optPrimType can be of type date/time via ConvertZonedCombinator
       case _ => false
     }
@@ -153,10 +155,11 @@ abstract class PackedBinaryIntegerBaseParser(
     }
 
     try {
-      val int = toInteger(context, dis.getByteArray(nBits, start))
+      val int = toPrimitiveType(context, dis.getByteArray(nBits, start))
       start.simpleElement.setDataValue(int)
     } catch {
       case n: NumberFormatException => PE(start, "Error in packed data: \n%s", n.getMessage())
+      case i: InvalidPrimitiveDataException => PE(start, "Error in packed data: \n%s", i.getMessage())
     }
   }
 }
@@ -193,11 +196,12 @@ abstract class PackedBinaryIntegerDelimitedBaseParser(
         return
       } else {
         try {
-          val num = toInteger(context, fieldBytes)
+          val num = toPrimitiveType(context, fieldBytes)
           state.simpleElement.setDataValue(num)
         } catch {
           case n: NumberFormatException =>
             PE(state, "Error in packed data: \n%s", n.getMessage())
+          case i: InvalidPrimitiveDataException => PE(state, "Error in packed data: \n%s", i.getMessage())
         }
 
         if (result.matchedDelimiterValue.isDefined) state.saveDelimitedParseResult(parseResult)
@@ -252,11 +256,12 @@ abstract class PackedBinaryDecimalDelimitedBaseParser(
         return
       } else {
         try {
-          val num = toDecimal(e, fieldBytes, binaryDecimalVirtualPoint)
+          val num = toPrimitiveType(e, fieldBytes, binaryDecimalVirtualPoint)
           state.simpleElement.setDataValue(num)
         } catch {
           case n: NumberFormatException =>
             PE(state, "Error in packed data: \n%s", n.getMessage())
+          case i: InvalidPrimitiveDataException => PE(state, "Error in packed data: \n%s", i.getMessage())
         }
 
         if (result.matchedDelimiterValue.isDefined) state.saveDelimitedParseResult(parseResult)

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
@@ -983,7 +983,7 @@
 
   <!-- Daffodil 2961 -->
   <tdml:parserTestCase name="bcdBigIntToLongExpr" root="bcdBigIntToLong" model="bcdBigIntToLongExprSch"
-    description="Verify that usign packed values in an expression work correctly">
+    description="Verify that unsign packed values in an expression work correctly">
 
     <tdml:document>
       <tdml:documentPart type="byte">0568656C6C6F</tdml:documentPart>
@@ -998,5 +998,27 @@
     </tdml:infoset>
   </tdml:parserTestCase>
 
+  <tdml:defineSchema name="packedDecimalRangeChecks">
+    <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"
+    lengthUnits="bytes" binaryNumberRep="packed" binaryPackedSignCodes="C D F C" binaryNumberCheckPolicy="strict"/>
+
+    <xs:element name="uint01" type="xs:unsignedInt" dfdl:representation="binary" dfdl:length="2" />
+
+  </tdml:defineSchema>
+
+  <tdml:parserTestCase name="packedNegativeUnsigned" root="uint01" model="packedDecimalRangeChecks"
+    description="Using BCD formatted hex to indicate an integer number">
+
+    <tdml:document>
+      <tdml:documentPart type="byte">123D</tdml:documentPart>
+    </tdml:document>
+
+    <tdml:errors>
+      <tdml:error>Parse Error</tdml:error>
+      <tdml:error>out of range for type</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
 
 </tdml:testSuite>
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
@@ -75,6 +75,8 @@ class TestPacked {
   @Test def testBCDCharset12(): Unit = { runner.runOneTest("bcdCharset12") }
   @Test def testBCDCharset13(): Unit = { runner.runOneTest("bcdCharset13") }
 
+  @Test def testPackedNegativeUnsigned(): Unit = { runner.runOneTest("packedNegativeUnsigned") }
+
   @Test def testIBM4690Charset01(): Unit = { runner.runOneTest("IBM4690Charset01") }
   @Test def testIBM4690Charset02(): Unit = { runner.runOneTest("IBM4690Charset02") }
   @Test def testIBM4690Charset03(): Unit = { runner.runOneTest("IBM4690Charset03") }