From 6362dde72aeb2606fe63928ef18d09526838acbf Mon Sep 17 00:00:00 2001 From: olabusayoT <50379531+olabusayoT@users.noreply.github.com> Date: Mon, 3 Feb 2025 15:46:56 -0500 Subject: [PATCH] fixup! fixup! fixup! fixup! Fix Bug with valueLength being overwritten after Trim - revert changes to specifiedLength function and only allow it to be applied in specific conditions - revert deletion of HexBinaryLengthPrefixed --- .../codegen/c/DaffodilCCodeGenerator.scala | 5 +- .../grammar/ElementBaseGrammarMixin.scala | 121 +++++++++--------- .../primitives/PrimitivesLengthKind.scala | 10 ++ .../lengthKind/TestLengthKindPrefixed.scala | 6 +- 4 files changed, 78 insertions(+), 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 index c05a485127..a42fab04f7 100644 --- 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 index d8a24e4cf5..14d09f8664 100644 --- 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/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/primitives/PrimitivesLengthKind.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/primitives/PrimitivesLengthKind.scala index 92bb951fe8..09ce2c1df1 100644 --- a/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/primitives/PrimitivesLengthKind.scala +++ b/daffodil-core/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/daffodil-test/src/test/scala/org/apache/daffodil/section12/lengthKind/TestLengthKindPrefixed.scala b/daffodil-test/src/test/scala/org/apache/daffodil/section12/lengthKind/TestLengthKindPrefixed.scala index c5713e63fb..3ec2eec760 100644 --- a/daffodil-test/src/test/scala/org/apache/daffodil/section12/lengthKind/TestLengthKindPrefixed.scala +++ b/daffodil-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