You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@daffodil.apache.org by GitBox <gi...@apache.org> on 2018/02/12 18:46:14 UTC
[GitHub] mbeckerle closed pull request #37: Fix bitOrder bug detected by Link16 schema.
mbeckerle closed pull request #37: Fix bitOrder bug detected by Link16 schema.
URL: https://github.com/apache/incubator-daffodil/pull/37
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/FramingUnparsers.scala b/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/FramingUnparsers.scala
index 41b695540..19b5df45d 100644
--- a/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/FramingUnparsers.scala
+++ b/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/FramingUnparsers.scala
@@ -25,7 +25,7 @@ import org.apache.daffodil.processors.TermRuntimeData
class SkipRegionUnparser(
skipInBits: Int,
override val context: TermRuntimeData)
- extends PrimUnparser {
+ extends AlignmentPrimUnparser {
override def runtimeDependencies = Nil
@@ -65,7 +65,7 @@ class AlignmentFillUnparserSuspendableOperation(
class AlignmentFillUnparser(
alignmentInBits: Int,
override val context: TermRuntimeData)
- extends PrimUnparser
+ extends AlignmentPrimUnparser
with SuspendableUnparser {
override def runtimeDependencies = Nil
diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/unparsers/Unparser.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/unparsers/Unparser.scala
index db623fe78..9442a67e2 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/unparsers/Unparser.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/unparsers/Unparser.scala
@@ -41,33 +41,36 @@ sealed trait Unparser
val savedProc = ustate.maybeProcessor
ustate.setProcessor(this)
- //
- // Since the state is being overwritten (in most case) now,
- // we must explicitly make a copy when debugging so we can compute a delta
- // after
- //
// ?? TODO: Should this be after the split below ??
if (ustate.dataProc.isDefined) ustate.dataProc.get.before(ustate, this)
- try {
- unparse(ustate)
- // TODO: Remove this call to ustate.bitOrder below.
- // Figure out where this is needed elsewhere in unparser code.
- //
- // Clearly calling this here is overkilling the problem.
- //
- // In theory some places in the unparser code dealing with splitting/suspending
- // or outputValueCalc elemetns are missing proper checking of bitOrder, or
- // keeping track of prior bit order. Finding those has been problematic.
- //
- // So this is a temporary fix, until we can figure out where else to do this.
- //
- this.context match {
- case trd: TermRuntimeData =>
- ustate.bitOrder // asking for bitOrder checks bit order changes.
+ // TODO: Remove this call to ustate.bitOrder below.
+ // Figure out where this is needed elsewhere in unparser code.
+ //
+ // Clearly calling this here is overkilling the problem.
+ //
+ // In theory some places in the unparser code dealing with splitting/suspending
+ // or outputValueCalc elements are missing proper checking of bitOrder, or
+ // keeping track of prior bit order. Finding those has been problematic.
+ //
+ // So this is a temporary fix, until we can figure out where else to do this.
+ //
+ this match {
+ // bit order only applies to primitives, not combinators, nor "noData" unparsers.
+ case af: AlignmentPrimUnparser => // ok. Don't check bitOrder before Aligning.
+ case u: PrimUnparser => {
+ u.context match {
+ case trd: TermRuntimeData =>
+ ustate.bitOrder // asking for bitOrder checks bit order changes.
// this splits DOS on bitOrder changes if absoluteBitPos not known
- case _ => //ok
+ case rd: RuntimeData => Assert.invariantFailed("Primitive unparser " + u + " has non-Term runtime data: " + rd)
+ }
}
+ case _ => // ok
+ }
+ try {
+ unparse(ustate)
+
} finally {
ustate.resetFormatInfoCaches()
}
@@ -95,6 +98,17 @@ trait PrimUnparser
extends Unparser
with PrimProcessor
+/**
+ * A marker trait for the unparsers that perform alignment.
+ *
+ * Needed to distinguish alignment operations from regular primitives so that
+ * we can inspect for bitOrder changes on most primitives, but not
+ * alignments - since their purpose may be to align so that the bitOrder change
+ * is on the right boundary. Checking bit order before them defeats the purpose
+ * of alignment.
+ */
+trait AlignmentPrimUnparser extends PrimUnparser
+
/**
* An unparser that is primitive (no sub-unparsers), but doesn't write anything
* to a data stream (buffered or real), so alignment, bitOrder, etc. cannot
diff --git a/daffodil-tdml/src/test/scala-debug/org/apache/daffodil/tdml/TestTDMLRunner3.scala b/daffodil-tdml/src/test/scala-debug/org/apache/daffodil/tdml/TestTDMLRunner3.scala
new file mode 100644
index 000000000..65c425aac
--- /dev/null
+++ b/daffodil-tdml/src/test/scala-debug/org/apache/daffodil/tdml/TestTDMLRunner3.scala
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.daffodil.tdml
+
+import org.apache.daffodil.xml.XMLUtils
+import junit.framework.Assert.assertEquals
+import org.apache.daffodil.util._
+import org.junit.Test
+
+class TestTDMLRunner3 {
+
+ val tdml = XMLUtils.TDML_NAMESPACE
+ val dfdl = XMLUtils.DFDL_NAMESPACE
+ val xsi = XMLUtils.XSI_NAMESPACE
+ val xsd = XMLUtils.XSD_NAMESPACE
+ val example = XMLUtils.EXAMPLE_NAMESPACE
+ val tns = example
+
+ /**
+ * Test illustrates problem with multiple document parts having a RTL byte order
+ * not being assembled properly.
+ *
+ * That or the document bitOrder is causing the parts bitOrders to be assembled incorrectly.
+ *
+ * There are many other tests that use RTL byte order to assemble bits together, so it is
+ * something about mixing byteOrder RTL with LTR that is causing the problem.
+ *
+ * Bug DAFFODIL-1898
+ */
+ @Test def testMixedBigEndianMSBFWithLittleEndianLSBF() {
+ val xml = <document bitOrder="MSBFirst" xmlns="http://www.ibm.com/xmlns/dfdl/testData">
+ <documentPart type="byte" bitOrder="MSBFirst" byteOrder="LTR">AA </documentPart>
+ <documentPart type="bits" bitOrder="LSBFirst" byteOrder="RTL">XXXX X001</documentPart>
+ <documentPart type="bits" bitOrder="LSBFirst" byteOrder="RTL">1111 1XXX</documentPart>
+ <!-- The above is AAF9 -->
+ </document>
+ val doc = new Document(xml, null)
+ val bytes = doc.documentBytes
+ val hexDigits = Misc.bytes2Hex(bytes)
+ val expected = "AAF9".replace(" ", "")
+ assertEquals(expected, hexDigits)
+ }
+}
diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/unparser/envelopePayload.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/unparser/envelopePayload.tdml
index 66d57173d..ce1a2ae6f 100644
--- a/daffodil-test/src/test/resources/org/apache/daffodil/unparser/envelopePayload.tdml
+++ b/daffodil-test/src/test/resources/org/apache/daffodil/unparser/envelopePayload.tdml
@@ -340,7 +340,108 @@ looking at DFDL schemas for some complex formats like STANAG 5516 with NACT wrap
</tdml:document>
</tdml:parserTestCase>
-
+ <tdml:defineSchema name="model3" elementFormDefault="unqualified">
+ <dfdl:format ref="be" />
+ <dfdl:defineFormat name="be">
+ <dfdl:format ref="ex:daffodilTest1" representation="binary"
+ encoding="ascii" lengthUnits="bits" alignmentUnits="bits"
+ alignment="1" fillByte="X" binaryNumberRep="binary" lengthKind="explicit" />
+ </dfdl:defineFormat>
+ <dfdl:defineFormat name="le">
+ <dfdl:format ref="be" byteOrder="littleEndian" bitOrder="leastSignificantBitFirst" />
+ </dfdl:defineFormat>
+
+ <xs:element name="records" dfdl:lengthKind="implicit">
+ <xs:complexType>
+ <xs:sequence>
+ <xs:element name="record" minOccurs="0" maxOccurs="unbounded" dfdl:occursCountKind="parsed" dfdl:lengthKind="implicit"
+ dfdl:alignmentUnits="bytes" dfdl:alignment="1">
+ <xs:complexType>
+ <xs:sequence>
+ <xs:element name="n" type="xs:int" dfdl:length="16" />
+ <xs:element name="len" type="xs:int" dfdl:length="16"
+ dfdl:outputValueCalc='{ dfdl:valueLength(../payload, "bits") }' />
+ <!--
+ The payload has a different byte order and bit order than the surrounding envelope and is not
+ a multiple of 8 bits long.
+ -->
+ <xs:element name="payload" dfdl:lengthKind="explicit" dfdl:length="{ ../len }" dfdl:ref="le">
+ <xs:complexType>
+ <xs:sequence dfdl:ref="le">
+ <xs:element name="kind" type="xs:int" dfdl:length="3" dfdl:ref="le">
+ <xs:annotation>
+ <xs:appinfo source="http://www.ogf.org/dfdl/">
+ <dfdl:discriminator>{ . eq 1 }</dfdl:discriminator>
+ </xs:appinfo>
+ </xs:annotation>
+ </xs:element>
+ <xs:element name="w" type="xs:int" dfdl:length="16" dfdl:ref="le" />
+ </xs:sequence>
+ </xs:complexType>
+ </xs:element>
+ </xs:sequence>
+ </xs:complexType>
+ </xs:element>
+ <xs:sequence dfdl:alignment="1" dfdl:alignmentUnits="bytes"/>
+ </xs:sequence>
+ </xs:complexType>
+ </xs:element>
+
+ </tdml:defineSchema>
+
+ <tdml:parserTestCase name="ep3" root="records" model="model3"
+ description="Envelope/payload adds complexity of repeating MSBF/BE envelope and non-byte-length LSBF/LE payload."
+ roundTrip="false">
+ <tdml:infoset>
+ <tdml:dfdlInfoset xmlns:ex="http://example.com">
+ <ex:records>
+ <record>
+ <n>255</n><!-- 00FF -->
+ <len>19</len><!-- 0013 -->
+ <payload>
+ <kind>1</kind>
+ <w>7</w>
+ </payload>
+ </record>
+ <record>
+ <n>255</n><!-- 00FF -->
+ <len>19</len><!-- 0013 -->
+ <payload>
+ <kind>1</kind>
+ <w>15</w>
+ </payload>
+ </record>
+ </ex:records>
+ </tdml:dfdlInfoset>
+ </tdml:infoset>
+ <document bitOrder="MSBFirst" xmlns="http://www.ibm.com/xmlns/dfdl/testData">
+ <documentPart type="byte">00FF 0013 3900 F8 00FF 0013 7900 F8 </documentPart>
+ <!--
+ Note TDML Runner not assembling mixtures of bitOrder/byteOrder correctly. The
+ above hex should be the same as what is below, but is not.
+ A unit test that shows the problem is testMixedBigEndianMSBFWithLittleEndianLSBF
+
+ See bug DAFFODIL-1898
+ -->
+ <!-- Record 1 -->
+ <!--
+ <documentPart type="byte" bitOrder="MSBFirst" byteOrder="LTR">00FF 0013 </documentPart>
+ <documentPart type="bits" bitOrder="LSBFirst" byteOrder="RTL">iWord Kind XXXX X001</documentPart>
+ <documentPart type="bits" bitOrder="LSBFirst" byteOrder="RTL">iWord w X000 0000 0000 0011 1XXX</documentPart>
+ <documentPart type="bits" bitOrder="LSBFirst" byteOrder="RTL">alignmentFill 1111 1XXX XXXX XXXX XXXX XXXX</documentPart>
+ -->
+ <!-- 00FF 0013 3900 F8 -->
+ <!-- Record 2 -->
+ <!--
+ <documentPart type="byte" bitOrder="MSBFirst" byteOrder="LTR">00FF 0013 </documentPart>
+ <documentPart type="bits" bitOrder="LSBFirst" byteOrder="RTL">iWord Kind XXXX X001</documentPart>
+ <documentPart type="bits" bitOrder="LSBFirst" byteOrder="RTL">iWord w X000 0000 0000 0111 1XXX</documentPart>
+ <documentPart type="bits" bitOrder="LSBFirst" byteOrder="RTL">alignmentFill 1111 1XXX XXXX XXXX XXXX XXXX</documentPart>
+ -->
+ <!-- 00FF 0013 7900 F8 -->
+ </document>
+
+ </tdml:parserTestCase>
</tdml:testSuite>
diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/unparser/TestEnvelopePayload.scala b/daffodil-test/src/test/scala/org/apache/daffodil/unparser/TestEnvelopePayload.scala
index 0a68cac9d..f3344b430 100644
--- a/daffodil-test/src/test/scala/org/apache/daffodil/unparser/TestEnvelopePayload.scala
+++ b/daffodil-test/src/test/scala/org/apache/daffodil/unparser/TestEnvelopePayload.scala
@@ -34,5 +34,6 @@ class TestEnvelopePayload {
@Test def test_ep1() { runner.runOneTest("ep1") }
@Test def test_ep2() { runner.runOneTest("ep2") }
+ @Test def test_ep3() { runner.runOneTest("ep3") }
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services