Skip to content

Commit

Permalink
Fix Bug with valueLength being overwritten after Trim
Browse files Browse the repository at this point in the history
- currently after trimming the value of the element, we set the valueLength, and then overwrite it after returning from the parse that does the trimming. This results in the wrong value for value length. This fixes it by checking if the valueLength has already been set, and only setting it in SpecifiedLengthParserBase.parse if it hasn't
- add asserts to setAbsStartPos0bInBits and setAbsEndPos0bInBits to ensure they're not being overwritten
- do not capture the value lengths of choices/complexTypes in specified length parser base
- fix bug where padding is being added around prefixed length element (DAFFODIL-2943) by changing CaptureLengthRegion to wrap around contentlengthStart and padding
- fix bug where we use the valuelength to calculate the prefix length, according to the spec it should be the content length
- fix bug where we were missing return after PE for Out of Range Binary Integers (DAFFODIL-2942)
- refactor Prefixed parsers to use state's bitLimit to get the prefix length (BitLengthFromBitLimitMixin) since the specifiedLengthPrefixedParser will take care of parsing the prefix length
- refactored Prefixed unparsers to not try to unparse prefix length since that is taken care of by SpecifiedLengthPrefixedUnparser
- refactored prefixed parsers and unparsers to remove unused prefixed length parser related members
- rename custom prefixedlength parsers to *BitLengthParsers to more accurately reflect what they're doing
- rename custom prefixedlength unparsers to MinimumLengthUnparsers to more accurately reflect what they're doing
- add tests

DAFFODIL-2658
  • Loading branch information
olabusayoT committed Feb 4, 2025
1 parent 8974ee6 commit 018570f
Show file tree
Hide file tree
Showing 28 changed files with 348 additions and 629 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import org.apache.daffodil.core.grammar.primitives.RightFill
import org.apache.daffodil.core.grammar.primitives.ScalarOrderedSequenceChild
import org.apache.daffodil.core.grammar.primitives.SpecifiedLengthExplicit
import org.apache.daffodil.core.grammar.primitives.SpecifiedLengthImplicit
import org.apache.daffodil.core.grammar.primitives.SpecifiedLengthPrefixed
import org.apache.daffodil.lib.api.Diagnostic
import org.apache.daffodil.lib.api.WarnID
import org.apache.daffodil.lib.schema.annotation.props.gen.FailureType
Expand Down Expand Up @@ -301,6 +302,7 @@ object DaffodilCCodeGenerator
case g: SeqComp => seqCompGenerateCode(g, cgState)
case g: SpecifiedLengthExplicit => specifiedLengthExplicit(g, cgState)
case g: SpecifiedLengthImplicit => specifiedLengthImplicit(g, cgState)
case g: SpecifiedLengthPrefixed => specifiedLengthPrefixed(g, cgState)
case _ => gram.SDE("Code generation not supported for: %s", Misc.getNameFromClass(gram))
}
}
Expand Down Expand Up @@ -409,4 +411,11 @@ object DaffodilCCodeGenerator
): Unit = {
DaffodilCCodeGenerator.generateCode(g.eGram, cgState)
}

private def specifiedLengthPrefixed(
g: SpecifiedLengthPrefixed,
cgState: CodeGeneratorState
): Unit = {
DaffodilCCodeGenerator.generateCode(g.eGram, cgState)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ trait ElementBaseGrammarMixin
}
}

protected lazy val isDelimitedPrefixedPattern = {
lazy val isPrefixed: Boolean = lengthKind == LengthKind.Prefixed

protected lazy val isDelimitedPrefixedPattern: Boolean = {
import LengthKind._
lengthKind match {
case Delimited =>
Expand Down Expand Up @@ -474,12 +476,16 @@ trait ElementBaseGrammarMixin
lazy val leftPadding = leftPaddingArg
lazy val rightPadFill = rightPadFillArg
lazy val body = bodyArg
CaptureContentLengthStart(this) ~
leftPadding ~
CaptureValueLengthStart(this) ~
body ~
CaptureValueLengthEnd(this) ~
rightPadFill ~
specifiedLength(
CaptureContentLengthStart(this) ~
leftPadding ~
CaptureValueLengthStart(this) ~
body ~
CaptureValueLengthEnd(this) ~
rightPadFill
) ~
// CaptureContentLengthEnd must be outside the specified length so it can
// do any skipping of bits it needs to before capturing the end of content length
CaptureContentLengthEnd(this)
}

Expand Down Expand Up @@ -623,14 +629,14 @@ trait ElementBaseGrammarMixin

private lazy val stringPrim = {
lengthKind match {
case LengthKind.Explicit => specifiedLength(StringOfSpecifiedLength(this))
case LengthKind.Prefixed => specifiedLength(StringOfSpecifiedLength(this))
case LengthKind.Explicit => StringOfSpecifiedLength(this)
case LengthKind.Prefixed => StringOfSpecifiedLength(this)
case LengthKind.Delimited => stringDelimitedEndOfData
case LengthKind.Pattern => specifiedLength(StringOfSpecifiedLength(this))
case LengthKind.Pattern => StringOfSpecifiedLength(this)
case LengthKind.Implicit => {
val pt = this.simpleType.primType
Assert.invariant(pt == PrimType.String)
specifiedLength(StringOfSpecifiedLength(this))
StringOfSpecifiedLength(this)
}
case LengthKind.EndOfParent if isComplexType =>
notYetImplemented("lengthKind='endOfParent' for complex type")
Expand All @@ -645,7 +651,7 @@ trait ElementBaseGrammarMixin
}

private lazy val hexBinaryLengthPattern = prod("hexBinaryLengthPattern") {
new SpecifiedLengthPattern(this, new HexBinaryEndOfBitLimit(this))
new HexBinaryEndOfBitLimit(this)
}

private lazy val hexBinaryLengthPrefixed = prod("hexBinaryLengthPrefixed") {
Expand Down Expand Up @@ -1219,7 +1225,7 @@ trait ElementBaseGrammarMixin
}

private lazy val nilLitSimple = prod("nilLitSimple", isSimpleType) {
captureLengthRegions(leftPadding, specifiedLength(nilLitContent), rightPadding ~ rightFill)
captureLengthRegions(leftPadding, nilLitContent, rightPadding ~ rightFill)
}

private lazy val nilLitComplex = prod("nilLitComplex", isComplexType) {
Expand Down Expand Up @@ -1322,59 +1328,70 @@ trait ElementBaseGrammarMixin
* as well, by not enclosing the body in a specified length enforcer.
*/
private def specifiedLength(bodyArg: => Gram) = {
lazy 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 =>
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)
// we need this to evaluate before we wrap in specified length parser,
// so it can do any internal checks for example blobValue's check for
// non-explicit lengthKind
val body = bodyArg

// there are essentially two categories of processors that read/write data input/output
// stream: those that calculate lengths themselves and those that expect another
// processor to calculate the length and set the bit limit which this processor will use as
// the length. The following determines if this element requires another processor to
// calculate and set the bit limit, and if so adds the appropriate grammar to do that
val bodyRequiresSpecifiedLengthBitLimit = lengthKind != LengthKind.Delimited && (
isSimpleType && impliedRepresentation == Representation.Text ||
isSimpleType && isNillable ||
isComplexType && lengthKind != LengthKind.Implicit ||
lengthKind == LengthKind.Prefixed ||
isSimpleType && primType == PrimType.HexBinary && lengthKind == LengthKind.Pattern
)
if (!bodyRequiresSpecifiedLengthBitLimit) {
body
} else {
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.
}
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
val lk = lengthKind
lk match {
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 && impliedRepresentation == Representation.Binary =>
new SpecifiedLengthImplicit(this, body, implicitBinaryLengthInBits)
case LengthKind.EndOfParent if isComplexType =>
notYetImplemented("lengthKind='endOfParent' for complex type")
case LengthKind.EndOfParent =>
notYetImplemented("lengthKind='endOfParent' for simple type")
}
}
}
Expand All @@ -1396,7 +1413,7 @@ trait ElementBaseGrammarMixin
private lazy val sharedComplexContentRegion: Gram =
schemaSet.sharedComplexContentFactory.getShared(
shareKey,
captureLengthRegions(EmptyGram, specifiedLength(complexContent), elementUnused) ~
captureLengthRegions(EmptyGram, complexContent, elementUnused) ~
terminatorRegion
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,18 @@ package org.apache.daffodil.core.grammar.primitives

import org.apache.daffodil.core.dsom.ElementBase
import org.apache.daffodil.core.grammar.Terminal
import org.apache.daffodil.runtime1.processors.parsers.BCDDecimalBitLimitLengthParser
import org.apache.daffodil.runtime1.processors.parsers.BCDDecimalKnownLengthParser
import org.apache.daffodil.runtime1.processors.parsers.BCDDecimalPrefixedLengthParser
import org.apache.daffodil.runtime1.processors.parsers.BCDDecimalRuntimeLengthParser
import org.apache.daffodil.runtime1.processors.parsers.BCDIntegerBitLimitLengthParser
import org.apache.daffodil.runtime1.processors.parsers.BCDIntegerKnownLengthParser
import org.apache.daffodil.runtime1.processors.parsers.BCDIntegerPrefixedLengthParser
import org.apache.daffodil.runtime1.processors.parsers.BCDIntegerRuntimeLengthParser
import org.apache.daffodil.runtime1.processors.unparsers.Unparser
import org.apache.daffodil.unparsers.runtime1.BCDDecimalKnownLengthUnparser
import org.apache.daffodil.unparsers.runtime1.BCDDecimalPrefixedLengthUnparser
import org.apache.daffodil.unparsers.runtime1.BCDDecimalMinimumLengthUnparser
import org.apache.daffodil.unparsers.runtime1.BCDDecimalRuntimeLengthUnparser
import org.apache.daffodil.unparsers.runtime1.BCDIntegerKnownLengthUnparser
import org.apache.daffodil.unparsers.runtime1.BCDIntegerPrefixedLengthUnparser
import org.apache.daffodil.unparsers.runtime1.BCDIntegerMinimumLengthUnparser
import org.apache.daffodil.unparsers.runtime1.BCDIntegerRuntimeLengthUnparser

class BCDIntegerRuntimeLength(val e: ElementBase) extends Terminal(e, true) {
Expand All @@ -52,20 +52,10 @@ class BCDIntegerKnownLength(val e: ElementBase, lengthInBits: Long) extends Term

class BCDIntegerPrefixedLength(val e: ElementBase) extends Terminal(e, true) {

override lazy val parser = new BCDIntegerPrefixedLengthParser(
e.elementRuntimeData,
e.prefixedLengthBody.parser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
)
override lazy val parser = new BCDIntegerBitLimitLengthParser(e.elementRuntimeData)

override lazy val unparser: Unparser = new BCDIntegerPrefixedLengthUnparser(
e.elementRuntimeData,
e.prefixedLengthBody.unparser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
override lazy val unparser: Unparser = new BCDIntegerMinimumLengthUnparser(
e.elementRuntimeData
)
}

Expand Down Expand Up @@ -102,21 +92,9 @@ class BCDDecimalKnownLength(val e: ElementBase, lengthInBits: Long) extends Term

class BCDDecimalPrefixedLength(val e: ElementBase) extends Terminal(e, true) {

override lazy val parser = new BCDDecimalPrefixedLengthParser(
e.elementRuntimeData,
e.prefixedLengthBody.parser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.binaryDecimalVirtualPoint,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
)
override lazy val parser =
new BCDDecimalBitLimitLengthParser(e.elementRuntimeData, e.binaryDecimalVirtualPoint)

override lazy val unparser: Unparser = new BCDDecimalPrefixedLengthUnparser(
e.elementRuntimeData,
e.prefixedLengthBody.unparser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.binaryDecimalVirtualPoint,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
)
override lazy val unparser: Unparser =
new BCDDecimalMinimumLengthUnparser(e.elementRuntimeData, e.binaryDecimalVirtualPoint)
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ package org.apache.daffodil.core.grammar.primitives

import org.apache.daffodil.core.dsom.ElementBase
import org.apache.daffodil.core.grammar.Terminal
import org.apache.daffodil.runtime1.processors.parsers.BinaryBooleanBitLimitLengthParser
import org.apache.daffodil.runtime1.processors.parsers.BinaryBooleanParser
import org.apache.daffodil.runtime1.processors.parsers.BinaryBooleanPrefixedLengthParser
import org.apache.daffodil.runtime1.processors.unparsers.Unparser
import org.apache.daffodil.unparsers.runtime1.BinaryBooleanPrefixedLengthUnparser
import org.apache.daffodil.unparsers.runtime1.BinaryBooleanMinimumLengthUnparser
import org.apache.daffodil.unparsers.runtime1.BinaryBooleanUnparser

class BinaryBoolean(val e: ElementBase) extends Terminal(e, true) {
Expand All @@ -46,23 +46,17 @@ class BinaryBoolean(val e: ElementBase) extends Terminal(e, true) {
}

class BinaryBooleanPrefixedLength(val e: ElementBase) extends Terminal(e, true) {
override lazy val parser = new BinaryBooleanPrefixedLengthParser(
override lazy val parser = new BinaryBooleanBitLimitLengthParser(
e.elementRuntimeData,
e.prefixedLengthBody.parser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.binaryBooleanTrueRep,
e.binaryBooleanFalseRep,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
e.lengthUnits
)

override lazy val unparser: Unparser = new BinaryBooleanPrefixedLengthUnparser(
override lazy val unparser: Unparser = new BinaryBooleanMinimumLengthUnparser(
e.elementRuntimeData,
e.prefixedLengthBody.unparser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.binaryBooleanTrueRep,
e.binaryBooleanFalseRep,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
e.lengthUnits
)
}
Loading

0 comments on commit 018570f

Please sign in to comment.