You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2021/06/03 17:37:14 UTC
[GitHub] [daffodil] stevedlawrence opened a new pull request #585: Fix max length check for xs:hexBinary
stevedlawrence opened a new pull request #585:
URL: https://github.com/apache/daffodil/pull/585
Because xs:hexBinary simple elements are stored as Java Byte arrays,
they have a max size limitation of MaxInt bytes. We do this check, but
incorrectly check against the length in bits instead of the length in
bytes. That means we really enforce a limitation of MaxInt bits for
xs:hexBinary, which is 256 MB, instead of the real max of 2GB. This
fixes that caclulation and improves the error message to mention this
maximum size and the calculated length.
DAFFODIL-2532
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [daffodil] stevedlawrence merged pull request #585: Fix max length check for xs:hexBinary
Posted by GitBox <gi...@apache.org>.
stevedlawrence merged pull request #585:
URL: https://github.com/apache/daffodil/pull/585
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [daffodil] tuxji commented on a change in pull request #585: Fix max length check for xs:hexBinary
Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #585:
URL: https://github.com/apache/daffodil/pull/585#discussion_r645554529
##########
File path: daffodil-propgen/src/main/resources/org/apache/daffodil/xsd/dafext.xsd
##########
@@ -132,12 +132,14 @@
<xs:element name="blobChunkSizeInBytes" default="4096" minOccurs="0">
<xs:annotation>
<xs:documentation>
- When reading/writing blob data, the maximum number of bytes to read/write at a time.
+ When reading/writing blob data, the maximum number of bytes to read/write
+ at a time. This is also used when parsing xs:hexBinary data.
</xs:documentation>
</xs:annotation>
<xs:simpleType>
<xs:restriction base="xs:int">
<xs:minInclusive value="1" />
+ <xs:maxInclusive value="268435455" /> <!-- This is (MaxInt / 8), needed because some uses of this need this tunable in bits as an integer -->
Review comment:
Let's make this comment both shorter and clearer:
Limit to (MaxInt / 8) because some places convert this tunable to bits
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/BlobLengthParsers.scala
##########
@@ -24,38 +24,32 @@ import org.apache.daffodil.processors.ElementRuntimeData
import org.apache.daffodil.processors.LengthInBitsEv
import org.apache.daffodil.util.MaybeULong
-sealed abstract class BlobLengthParser(override val context: ElementRuntimeData)
- extends PrimParser {
-
- protected def getLengthInBits(pstate: PState): Long
-
- override final def parse(start: PState): Unit = {
+trait ByteChunkWriter { self: PrimParser =>
+
+ /**
+ * Read nBits of data from the input source associated with pstate in chunks.
+ * For each chunk, the writeChunk function is called, which takes as
Review comment:
Let's make the comment clearer that the writeChunk function is passed as a parameter:
* For each chunk, the writeChunk function passed as a parameter is called, which takes as
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [daffodil] mbeckerle commented on a change in pull request #585: Fix max length check for xs:hexBinary
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #585:
URL: https://github.com/apache/daffodil/pull/585#discussion_r645046006
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/HexBinaryLengthParsers.scala
##########
@@ -34,11 +34,12 @@ sealed abstract class HexBinaryLengthParser(override val context: ElementRuntime
val dis = start.dataInputStream
val currentElement = start.simpleElement
val nBits = getLengthInBits(start)
- if (nBits == 0) {
+ val nBytes = (nBits + 7) / 8
+ if (nBytes == 0) {
currentElement.setDataValue(zeroLengthArray)
- } else if (nBits > Int.MaxValue) {
+ } else if (nBytes > Int.MaxValue) {
Review comment:
People should have a choice of whether to use the blob files or not.
For a while I've been wondering if the representation of a large opaque thing should not be one hexBinary element, but an array of them.
That plus our streaming would effectively make the size unlimited.
Maybe users can do this themselves anyway?
Instead of a single hexBinary element, an array of hexBinary chunks. Each "chunk" having a max length of say, 1024. That would multiply the effective size by 1024 which would make for well-behaved XML text, and would stream well.
```
<xs:element name="bigData" dfdl:lengthKind="explicit" dfdl:length='{ .... }'>
<xs:complexType>
<xs:sequence>
<xs:element name="data" maxOccurs="unbounded" dfdl:occursCountKind="implicit">
<xs:complexType>
<xs:choice>
<xs:element name="full" type="xs:hexBinary" dfdl:lengthKind="explicit" dfdl:length="1024"/>
<!-- wants to be lengthKind 'endOfParent' below, but we don't have that in Daffodil yet -->
<xs:element name="final" type="xs:hexBinary" dfdl:lengthKind="pattern"
dfdl:lengthPattern='.{1,1023}' />
</xs:choice>
</xs:complexType>
</xs:element>
</xs:sequence>
</xs:complexType>
</xs:element>
```
I'm a bit unsure about the full vs. final choice and using lengthKind pattern. We've done something like this before where things extended to the end of a enclosing length-bounded structure. I just can't recall the trick.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [daffodil] stevedlawrence commented on a change in pull request #585: Fix max length check for xs:hexBinary
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #585:
URL: https://github.com/apache/daffodil/pull/585#discussion_r645005797
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/HexBinaryLengthParsers.scala
##########
@@ -34,11 +34,12 @@ sealed abstract class HexBinaryLengthParser(override val context: ElementRuntime
val dis = start.dataInputStream
val currentElement = start.simpleElement
val nBits = getLengthInBits(start)
- if (nBits == 0) {
+ val nBytes = (nBits + 7) / 8
+ if (nBytes == 0) {
currentElement.setDataValue(zeroLengthArray)
- } else if (nBits > Int.MaxValue) {
+ } else if (nBytes > Int.MaxValue) {
Review comment:
Although MaxValue is a technical limitation imposed by java array sizes, the way our BucketingInputSource works we still have a limitation of 256MB because that's the maximum it will store in it's cache.
If we really want to support xs:hexBinary more than 256MB, I think we could maybe use similar logic as the BlobLengthParser, which reads data in chunks. The only difference is instead of writing those chunks to a file we would copy them to an array in memory, which is eventually written to the infoset.
Thoughts? Should we include this change, or should we instead suggest that blobs be used? Do we also want a tunable limit to limit the size of blobs? And if so, do we want to default that to something less than MaxInt?
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/HexBinaryLengthParsers.scala
##########
@@ -34,11 +34,12 @@ sealed abstract class HexBinaryLengthParser(override val context: ElementRuntime
val dis = start.dataInputStream
val currentElement = start.simpleElement
val nBits = getLengthInBits(start)
- if (nBits == 0) {
+ val nBytes = (nBits + 7) / 8
+ if (nBytes == 0) {
currentElement.setDataValue(zeroLengthArray)
- } else if (nBits > Int.MaxValue) {
+ } else if (nBytes > Int.MaxValue) {
Review comment:
I've pushed an update that reads large xs:hexBinary elements so it at least gets around the cache issue throwing away data before it can be read into the array. So the limit should now be 2GB. Either of your solutions are probably what's required if we want to support hexBinary larger than that. I'll leave xs:hexBinary being represented as an array of arrays to another PR if we decide that's needed. That's probably a decent amount of work to update all the locations where we use hex binary correctly, and the schema solution would probably be decent workaround until then.
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/HexBinaryLengthParsers.scala
##########
@@ -34,11 +34,12 @@ sealed abstract class HexBinaryLengthParser(override val context: ElementRuntime
val dis = start.dataInputStream
val currentElement = start.simpleElement
val nBits = getLengthInBits(start)
- if (nBits == 0) {
+ val nBytes = (nBits + 7) / 8
+ if (nBytes == 0) {
currentElement.setDataValue(zeroLengthArray)
- } else if (nBits > Int.MaxValue) {
+ } else if (nBytes > Int.MaxValue) {
// Integer overflow will occurr, recommend using blob instead of hexBinary
- PE(start, "Data is too large for xs:hexBinary. Consider using blobs instead: https://s.apache.org/daffodil-blob-feature")
+ PE(start, "Length for xs:hexBinary exceeds maximum of %s bytes: %s", Int.MaxValue, nBytes)
Review comment:
We do have a tunable to configure the blob size, and some test that crank it down to zero. So we should be able to easily test the ``hex binary > chunk size`` limit. I can add a tunable for this and default it to MaxInt.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [daffodil] stevedlawrence commented on a change in pull request #585: Fix max length check for xs:hexBinary
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #585:
URL: https://github.com/apache/daffodil/pull/585#discussion_r645080507
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/HexBinaryLengthParsers.scala
##########
@@ -34,11 +34,12 @@ sealed abstract class HexBinaryLengthParser(override val context: ElementRuntime
val dis = start.dataInputStream
val currentElement = start.simpleElement
val nBits = getLengthInBits(start)
- if (nBits == 0) {
+ val nBytes = (nBits + 7) / 8
+ if (nBytes == 0) {
currentElement.setDataValue(zeroLengthArray)
- } else if (nBits > Int.MaxValue) {
+ } else if (nBytes > Int.MaxValue) {
// Integer overflow will occurr, recommend using blob instead of hexBinary
- PE(start, "Data is too large for xs:hexBinary. Consider using blobs instead: https://s.apache.org/daffodil-blob-feature")
+ PE(start, "Length for xs:hexBinary exceeds maximum of %s bytes: %s", Int.MaxValue, nBytes)
Review comment:
We do have a tunable to configure the blob size, and some test that crank it down to zero. So we should be able to easily test the ``hex binary > chunk size`` limit. I can add a tunable for this and default it to MaxInt.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [daffodil] mbeckerle commented on a change in pull request #585: Fix max length check for xs:hexBinary
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #585:
URL: https://github.com/apache/daffodil/pull/585#discussion_r645046006
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/HexBinaryLengthParsers.scala
##########
@@ -34,11 +34,12 @@ sealed abstract class HexBinaryLengthParser(override val context: ElementRuntime
val dis = start.dataInputStream
val currentElement = start.simpleElement
val nBits = getLengthInBits(start)
- if (nBits == 0) {
+ val nBytes = (nBits + 7) / 8
+ if (nBytes == 0) {
currentElement.setDataValue(zeroLengthArray)
- } else if (nBits > Int.MaxValue) {
+ } else if (nBytes > Int.MaxValue) {
Review comment:
People should have a choice of whether to use the blob files or not.
For a while I've been wondering if the representation of a large opaque thing should not be one hexBinary element, but an array of them.
That plus our streaming would effectively make the size unlimited.
Maybe users can do this themselves anyway?
Instead of a single hexBinary element, an array of pairs of length + hexBinary of that length. Each "chunk" having a max length of say, 1024. That would multiply the effective size by 1024 which would make for well-behaved XML text, and would stream well.
```
<xs:element name="bigData" dfdl:lengthKind="explicit" dfdl:length='{ .... }'>
<xs:complexType>
<xs:sequence>
<xs:element name="data" maxOccurs="unbounded" dfdl:occursCountKind="implicit">
<xs:complexType>
<xs:choice>
<xs:element name="full" type="xs:hexBinary" dfdl:lengthKind="explicit" dfdl:length="1024"/>
<!-- wants to be lengthKind 'endOfParent' below, but we don't have that in Daffodil yet -->
<xs:element name="final" type="xs:hexBinary" dfdl:lengthKind="pattern"
dfdl:lengthPattern='.{1,1023}' />
</xs:choice>
</xs:complexType>
</xs:element>
</xs:sequence>
</xs:complexType>
</xs:element>
```
I'm a bit unsure about the full vs. final choice and using lengthKind pattern. We've done something like this before where things extended to the end of a enclosing length-bounded structure. I just can't recall the trick.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [daffodil] stevedlawrence commented on a change in pull request #585: Fix max length check for xs:hexBinary
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #585:
URL: https://github.com/apache/daffodil/pull/585#discussion_r645005797
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/HexBinaryLengthParsers.scala
##########
@@ -34,11 +34,12 @@ sealed abstract class HexBinaryLengthParser(override val context: ElementRuntime
val dis = start.dataInputStream
val currentElement = start.simpleElement
val nBits = getLengthInBits(start)
- if (nBits == 0) {
+ val nBytes = (nBits + 7) / 8
+ if (nBytes == 0) {
currentElement.setDataValue(zeroLengthArray)
- } else if (nBits > Int.MaxValue) {
+ } else if (nBytes > Int.MaxValue) {
Review comment:
Although MaxValue is a technical limitation imposed by java array sizes, the way our BucketingInputSource works we still have a limitation of 256MB because that's the maximum it will store in it's cache.
If we really want to support xs:hexBinary more than 256MB, I think we could maybe use similar logic as the BlobLengthParser, which reads data in chunks. The only difference is instead of writing those chunks to a file we would copy them to an array in memory, which is eventually written to the infoset.
Thoughts? Should we include this change, or should we instead suggest that blobs be used? Do we also want a tunable limit to limit the size of blobs? And if so, do we want to default that to something less than MaxInt?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [daffodil] mbeckerle commented on a change in pull request #585: Fix max length check for xs:hexBinary
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #585:
URL: https://github.com/apache/daffodil/pull/585#discussion_r645046006
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/HexBinaryLengthParsers.scala
##########
@@ -34,11 +34,12 @@ sealed abstract class HexBinaryLengthParser(override val context: ElementRuntime
val dis = start.dataInputStream
val currentElement = start.simpleElement
val nBits = getLengthInBits(start)
- if (nBits == 0) {
+ val nBytes = (nBits + 7) / 8
+ if (nBytes == 0) {
currentElement.setDataValue(zeroLengthArray)
- } else if (nBits > Int.MaxValue) {
+ } else if (nBytes > Int.MaxValue) {
Review comment:
People should have a choice of whether to use the blob files or not.
For a while I've been wondering if the representation of a large opaque thing should not be one hexBinary element, but an array of them.
That plus our streaming would effectively make the size unlimited.
Maybe users can do this themselves anyway?
Instead of a single hexBinary element, an array of pairs of length + hexBinary of that length. Each "chunk" having a max length of say, 1024. That would multiply the effective size by 1024 which would make for well-behaved XML text, and would stream well.
```
<xs:element name="bigData" dfdl:lengthKind="explicit" dfdl:length='{ .... }'>
<xs:complexType>
<xs:sequence>
<xs:element name="data" maxOccurs="unbounded" dfdl:occursCountKind="implicit">
<xs:complexType>
<xs:choice>
<xs:element name="full" type="xs:hexBinary" dfdl:lengthKind="explicit" dfdl:length="1024"/>
<!-- wants to be lengthKind 'endOfParent' below, but we don't have that in Daffodil yet -->
<xs:element name="final" type="xs:hexBinary" dfdl:lengthKind="pattern"
dfdl:lengthPattern='.{1,1023}' />
</xs:choice>
</xs:complexType>
</xs:element>
</xs:sequence>
</xs:complexType>
</xs:element>
```
I'm a bit unsure about the full vs. final choice and using lengthKind pattern. We've done something like this before where things extended to the end of a enclosing length-bounded structure. I just can't recall the trick.
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/HexBinaryLengthParsers.scala
##########
@@ -34,11 +34,12 @@ sealed abstract class HexBinaryLengthParser(override val context: ElementRuntime
val dis = start.dataInputStream
val currentElement = start.simpleElement
val nBits = getLengthInBits(start)
- if (nBits == 0) {
+ val nBytes = (nBits + 7) / 8
+ if (nBytes == 0) {
currentElement.setDataValue(zeroLengthArray)
- } else if (nBits > Int.MaxValue) {
+ } else if (nBytes > Int.MaxValue) {
Review comment:
People should have a choice of whether to use the blob files or not.
For a while I've been wondering if the representation of a large opaque thing should not be one hexBinary element, but an array of them.
That plus our streaming would effectively make the size unlimited.
Maybe users can do this themselves anyway?
Instead of a single hexBinary element, an array of hexBinary chunks. Each "chunk" having a max length of say, 1024. That would multiply the effective size by 1024 which would make for well-behaved XML text, and would stream well.
```
<xs:element name="bigData" dfdl:lengthKind="explicit" dfdl:length='{ .... }'>
<xs:complexType>
<xs:sequence>
<xs:element name="data" maxOccurs="unbounded" dfdl:occursCountKind="implicit">
<xs:complexType>
<xs:choice>
<xs:element name="full" type="xs:hexBinary" dfdl:lengthKind="explicit" dfdl:length="1024"/>
<!-- wants to be lengthKind 'endOfParent' below, but we don't have that in Daffodil yet -->
<xs:element name="final" type="xs:hexBinary" dfdl:lengthKind="pattern"
dfdl:lengthPattern='.{1,1023}' />
</xs:choice>
</xs:complexType>
</xs:element>
</xs:sequence>
</xs:complexType>
</xs:element>
```
I'm a bit unsure about the full vs. final choice and using lengthKind pattern. We've done something like this before where things extended to the end of a enclosing length-bounded structure. I just can't recall the trick.
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/HexBinaryLengthParsers.scala
##########
@@ -34,11 +34,12 @@ sealed abstract class HexBinaryLengthParser(override val context: ElementRuntime
val dis = start.dataInputStream
val currentElement = start.simpleElement
val nBits = getLengthInBits(start)
- if (nBits == 0) {
+ val nBytes = (nBits + 7) / 8
+ if (nBytes == 0) {
currentElement.setDataValue(zeroLengthArray)
- } else if (nBits > Int.MaxValue) {
+ } else if (nBytes > Int.MaxValue) {
Review comment:
I think the "final" element should perhaps just have lengthKind 'delimited' but no terminator specified. Then it will extend to "end of data" which is this case is bounded by the containing structure.
As usual with lengthKind pattern, I am missing a dfdl:assert that insists on the actual length being > 0. It might not be needed in this case because the "no forward progress" check would terminate the array. But it doesn't hurt.
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/HexBinaryLengthParsers.scala
##########
@@ -34,11 +34,12 @@ sealed abstract class HexBinaryLengthParser(override val context: ElementRuntime
val dis = start.dataInputStream
val currentElement = start.simpleElement
val nBits = getLengthInBits(start)
- if (nBits == 0) {
+ val nBytes = (nBits + 7) / 8
+ if (nBytes == 0) {
currentElement.setDataValue(zeroLengthArray)
- } else if (nBits > Int.MaxValue) {
+ } else if (nBytes > Int.MaxValue) {
// Integer overflow will occurr, recommend using blob instead of hexBinary
- PE(start, "Data is too large for xs:hexBinary. Consider using blobs instead: https://s.apache.org/daffodil-blob-feature")
+ PE(start, "Length for xs:hexBinary exceeds maximum of %s bytes: %s", Int.MaxValue, nBytes)
Review comment:
A good reason to have a tunable size here is so you can crank it way down for testing purposes and cover all these error cases. People will really run into them with large data, so we really should cover them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [daffodil] stevedlawrence commented on a change in pull request #585: Fix max length check for xs:hexBinary
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #585:
URL: https://github.com/apache/daffodil/pull/585#discussion_r645079475
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/HexBinaryLengthParsers.scala
##########
@@ -34,11 +34,12 @@ sealed abstract class HexBinaryLengthParser(override val context: ElementRuntime
val dis = start.dataInputStream
val currentElement = start.simpleElement
val nBits = getLengthInBits(start)
- if (nBits == 0) {
+ val nBytes = (nBits + 7) / 8
+ if (nBytes == 0) {
currentElement.setDataValue(zeroLengthArray)
- } else if (nBits > Int.MaxValue) {
+ } else if (nBytes > Int.MaxValue) {
Review comment:
I've pushed an update that reads large xs:hexBinary elements so it at least gets around the cache issue throwing away data before it can be read into the array. So the limit should now be 2GB. Either of your solutions are probably what's required if we want to support hexBinary larger than that. I'll leave xs:hexBinary being represented as an array of arrays to another PR if we decide that's needed. That's probably a decent amount of work to update all the locations where we use hex binary correctly, and the schema solution would probably be decent workaround until then.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [daffodil] mbeckerle commented on a change in pull request #585: Fix max length check for xs:hexBinary
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #585:
URL: https://github.com/apache/daffodil/pull/585#discussion_r645052263
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/HexBinaryLengthParsers.scala
##########
@@ -34,11 +34,12 @@ sealed abstract class HexBinaryLengthParser(override val context: ElementRuntime
val dis = start.dataInputStream
val currentElement = start.simpleElement
val nBits = getLengthInBits(start)
- if (nBits == 0) {
+ val nBytes = (nBits + 7) / 8
+ if (nBytes == 0) {
currentElement.setDataValue(zeroLengthArray)
- } else if (nBits > Int.MaxValue) {
+ } else if (nBytes > Int.MaxValue) {
Review comment:
I think the "final" element should perhaps just have lengthKind 'delimited' but no terminator specified. Then it will extend to "end of data" which is this case is bounded by the containing structure.
As usual with lengthKind pattern, I am missing a dfdl:assert that insists on the actual length being > 0. It might not be needed in this case because the "no forward progress" check would terminate the array. But it doesn't hurt.
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/HexBinaryLengthParsers.scala
##########
@@ -34,11 +34,12 @@ sealed abstract class HexBinaryLengthParser(override val context: ElementRuntime
val dis = start.dataInputStream
val currentElement = start.simpleElement
val nBits = getLengthInBits(start)
- if (nBits == 0) {
+ val nBytes = (nBits + 7) / 8
+ if (nBytes == 0) {
currentElement.setDataValue(zeroLengthArray)
- } else if (nBits > Int.MaxValue) {
+ } else if (nBytes > Int.MaxValue) {
// Integer overflow will occurr, recommend using blob instead of hexBinary
- PE(start, "Data is too large for xs:hexBinary. Consider using blobs instead: https://s.apache.org/daffodil-blob-feature")
+ PE(start, "Length for xs:hexBinary exceeds maximum of %s bytes: %s", Int.MaxValue, nBytes)
Review comment:
A good reason to have a tunable size here is so you can crank it way down for testing purposes and cover all these error cases. People will really run into them with large data, so we really should cover them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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