fixup! fixup! fixup! fixup! Fix Bug with valueLength being overwritte…

…n after Trim - revert changes to specifiedLength function and only allow it to be applied in specific conditions - revert deletion of HexBinaryLengthPrefixed
apache · Feb 3, 2025 · 6362dde · 6362dde
1 parent 12ed999
commit 6362dde
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 64 deletions.
diff --git a/daffodil-codegen-c/src/main/scala/org/apache/daffodil/codegen/c/DaffodilCCodeGenerator.scala b/daffodil-codegen-c/src/main/scala/org/apache/daffodil/codegen/c/DaffodilCCodeGenerator.scala
@@ -49,7 +49,7 @@ import org.apache.daffodil.core.grammar.primitives.ChoiceCombinator
 import org.apache.daffodil.core.grammar.primitives.ElementCombinator
 import org.apache.daffodil.core.grammar.primitives.ElementParseAndUnspecifiedLength
 import org.apache.daffodil.core.grammar.primitives.ElementUnused
-import org.apache.daffodil.core.grammar.primitives.HexBinaryEndOfBitLimit
+import org.apache.daffodil.core.grammar.primitives.HexBinaryLengthPrefixed
 import org.apache.daffodil.core.grammar.primitives.HexBinarySpecifiedLength
 import org.apache.daffodil.core.grammar.primitives.OrderedSequence
 import org.apache.daffodil.core.grammar.primitives.RepOrderedExactlyNSequenceChild
@@ -289,8 +289,7 @@ object DaffodilCCodeGenerator
       case g: ElementParseAndUnspecifiedLength =>
         elementParseAndUnspecifiedLengthGenerateCode(g, cgState)
       case g: ElementUnused => noop(g)
-      case g: HexBinaryEndOfBitLimit if g.e.isPrefixed =>
-        hexBinaryLengthPrefixedGenerateCode(g.e, cgState)
+      case g: HexBinaryLengthPrefixed => hexBinaryLengthPrefixedGenerateCode(g.e, cgState)
       case g: HexBinarySpecifiedLength => hexBinarySpecifiedLengthGenerateCode(g.e, cgState)
       case g: OrderedSequence => orderedSequenceGenerateCode(g, cgState)
       case g: Prod => prod(g, cgState)

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
@@ -655,7 +655,7 @@ trait ElementBaseGrammarMixin
   }
 
   private lazy val hexBinaryLengthPrefixed = prod("hexBinaryLengthPrefixed") {
-    new HexBinaryEndOfBitLimit(this)
+    new HexBinaryLengthPrefixed(this)
   }
 
   private lazy val hexBinaryValue = prod("hexBinaryValue") {
@@ -1332,66 +1332,71 @@ trait ElementBaseGrammarMixin
     // so it can do any internal checks for example blobValue's check for
     // non-explicit lengthKind
     val body = bodyArg
-    lazy val bitsMultiplier = lengthUnits match {
-      case LengthUnits.Bits => 1
-      case LengthUnits.Bytes => 8
-      case LengthUnits.Characters if knownEncodingIsFixedWidth => this.knownEncodingWidthInBits
-      case _ => 0 // zero means can't multiply to get width in bits.
-    }
-    val lk = lengthKind
-    lk match {
-      case LengthKind.Delimited => body
-      case LengthKind.Pattern => new SpecifiedLengthPattern(this, body)
-      case LengthKind.Explicit if bitsMultiplier != 0 =>
-        if (isSimpleType && primType == PrimType.HexBinary) {
-          // hexBinary has some checks that need to be done that SpecifiedLengthExplicit
-          // gets in the way of
-          body
-        } else {
+    if (
+      isSimpleType && impliedRepresentation == Representation.Text ||
+      isSimpleType && isNillable ||
+      isComplexType ||
+      lengthKind == LengthKind.Prefixed ||
+      isSimpleType && primType == PrimType.HexBinary && lengthKind == LengthKind.Pattern
+    ) {
+      lazy val bitsMultiplier = lengthUnits match {
+        case LengthUnits.Bits => 1
+        case LengthUnits.Bytes => 8
+        case LengthUnits.Characters if knownEncodingIsFixedWidth =>
+          this.knownEncodingWidthInBits
+        case _ => 0 // zero means can't multiply to get width in bits.
+      }
+      val lk = lengthKind
+      lk match {
+        case LengthKind.Delimited => body
+        case LengthKind.Pattern => new SpecifiedLengthPattern(this, body)
+        case LengthKind.Explicit if bitsMultiplier != 0 =>
           new SpecifiedLengthExplicit(this, body, bitsMultiplier)
+        case LengthKind.Explicit => {
+          Assert.invariant(!knownEncodingIsFixedWidth)
+          Assert.invariant(lengthUnits eq LengthUnits.Characters)
+          new SpecifiedLengthExplicitCharacters(this, body)
+        }
+        case LengthKind.Prefixed if (bitsMultiplier != 0) =>
+          new SpecifiedLengthPrefixed(this, body, bitsMultiplier)
+        case LengthKind.Prefixed => {
+          Assert.invariant(!knownEncodingIsFixedWidth)
+          Assert.invariant(lengthUnits eq LengthUnits.Characters)
+          new SpecifiedLengthPrefixedCharacters(this, body)
+        }
+        case LengthKind.Implicit
+            if isSimpleType && primType == PrimType.String &&
+              encodingInfo.knownEncodingIsFixedWidth => {
+          //
+          // Important case to optimize
+          // If we can convert to a number of bits, then we should do so
+          //
+          val nBits =
+            encodingInfo.knownFixedWidthEncodingInCharsToBits(this.maxLength.longValue)
+          new SpecifiedLengthImplicit(this, body, nBits)
+        }
+        case LengthKind.Implicit if isSimpleType && primType == PrimType.String =>
+          new SpecifiedLengthImplicitCharacters(this, body, this.maxLength.longValue)
+
+        case LengthKind.Implicit if isSimpleType && primType == PrimType.HexBinary =>
+          new SpecifiedLengthImplicit(this, body, this.maxLength.longValue * bitsMultiplier)
+        case LengthKind.Implicit
+            if isSimpleType && impliedRepresentation == Representation.Binary =>
+          new SpecifiedLengthImplicit(this, body, implicitBinaryLengthInBits)
+        case LengthKind.Implicit if isComplexType =>
+          body // for complex types, implicit means "roll up from the bottom"
+        case LengthKind.EndOfParent if isComplexType =>
+          notYetImplemented("lengthKind='endOfParent' for complex type")
+        case LengthKind.EndOfParent =>
+          notYetImplemented("lengthKind='endOfParent' for simple type")
+        case _ => {
+          // TODO: implement other specified length like end of parent
+          // for now, no restriction
+          body
         }
-      case LengthKind.Explicit => {
-        Assert.invariant(!knownEncodingIsFixedWidth)
-        Assert.invariant(lengthUnits eq LengthUnits.Characters)
-        new SpecifiedLengthExplicitCharacters(this, body)
-      }
-      case LengthKind.Prefixed if (bitsMultiplier != 0) =>
-        new SpecifiedLengthPrefixed(this, body, bitsMultiplier)
-      case LengthKind.Prefixed => {
-        Assert.invariant(!knownEncodingIsFixedWidth)
-        Assert.invariant(lengthUnits eq LengthUnits.Characters)
-        new SpecifiedLengthPrefixedCharacters(this, body)
-      }
-      case LengthKind.Implicit
-          if isSimpleType && primType == PrimType.String &&
-            encodingInfo.knownEncodingIsFixedWidth => {
-        //
-        // Important case to optimize
-        // If we can convert to a number of bits, then we should do so
-        //
-        val nBits = encodingInfo.knownFixedWidthEncodingInCharsToBits(this.maxLength.longValue)
-        new SpecifiedLengthImplicit(this, body, nBits)
-      }
-      case LengthKind.Implicit if isSimpleType && primType == PrimType.String =>
-        new SpecifiedLengthImplicitCharacters(this, body, this.maxLength.longValue)
-      case LengthKind.Implicit
-          if isSimpleType &&
-            impliedRepresentation == Representation.Binary &&
-            primType != PrimType.HexBinary =>
-        new SpecifiedLengthImplicit(this, body, implicitBinaryLengthInBits)
-      case LengthKind.Implicit =>
-        body // for complex types, implicit means "roll up from the bottom"
-      // for simple types, the primitives have custom parsers that handle implicit length logic
-      // and don't use the limit provided by the SpecifiedLengthImplicit parser
-      case LengthKind.EndOfParent if isComplexType =>
-        notYetImplemented("lengthKind='endOfParent' for complex type")
-      case LengthKind.EndOfParent =>
-        notYetImplemented("lengthKind='endOfParent' for simple type")
-      case _ => {
-        // TODO: implement other specified length like end of parent
-        // for now, no restriction
-        body
       }
+    } else {
+      body
     }
   }
 

diff --git a/...ore/src/main/scala/org/apache/daffodil/core/grammar/primitives/PrimitivesLengthKind.scala b/...ore/src/main/scala/org/apache/daffodil/core/grammar/primitives/PrimitivesLengthKind.scala
@@ -181,6 +181,16 @@ case class HexBinaryEndOfBitLimit(e: ElementBase) extends Terminal(e, true) {
     new HexBinaryMinLengthInBytesUnparser(e.minLength.longValue, e.elementRuntimeData)
 }
 
+case class HexBinaryLengthPrefixed(e: ElementBase) extends Terminal(e, true) {
+
+  override lazy val parser: DaffodilParser = new HexBinaryEndOfBitLimitParser(
+    e.elementRuntimeData
+  )
+
+  override lazy val unparser: DaffodilUnparser =
+    new HexBinaryMinLengthInBytesUnparser(e.minLength.longValue, e.elementRuntimeData)
+}
+
 abstract class PackedIntegerDelimited(
   e: ElementBase,
   packedSignCodes: PackedSignCodes

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
@@ -172,11 +172,11 @@ class TestLengthKindPrefixed extends TdmlTests {
   @Test def pl_simpleContentLengthBytes_1 = test
 
   // DAFFODIL-2658
-  @Test def test_pl_simpleValueLengthBytes_1() = test
-  @Test def test_pl_simpleValueLengthBytes_2() = test
+  @Test def pl_simpleValueLengthBytes_1() = test
+  @Test def pl_simpleValueLengthBytes_2() = test
 
   // DAFFODIL-2948
-  @Test def test_pl_simpleValueLengthBytes_3() = test
+  @Test def pl_simpleValueLengthBytes_3() = test
 
   @Test def pl_simpleContentLengthCharacters_1 = test
   @Test def pl_complexContentLengthCharacters_1 = test