You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "mike-mcgann (via GitHub)" <gi...@apache.org> on 2023/02/02 15:18:49 UTC
[GitHub] [daffodil] mike-mcgann opened a new pull request, #945: Normalize whitespace in simple type constructors with string values
mike-mcgann opened a new pull request, #945:
URL: https://github.com/apache/daffodil/pull/945
Daffodil currently throws an exception if a constructor for a simple type has leading whitespace (e.g., xs:unsignedInt(' 1')). This update applies whitespace normalization before parsing.
[DAFFODIL-2676](https://issues.apache.org/jira/browse/DAFFODIL-2676)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] mbeckerle commented on a diff in pull request #945: Normalize whitespace in simple type constructors with string values
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #945:
URL: https://github.com/apache/daffodil/pull/945#discussion_r1096347811
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/dpath/NodeInfo.scala:
##########
@@ -535,15 +535,16 @@ object NodeInfo extends Enum {
protected def fromString(s: String): DataValueNumber
def fromXMLString(s: String): DataValueNumber = {
Review Comment:
I have seen pretty printers do this:
```
<foo>
6847
</foo>
```
And in schema-aware XML, that doesn't change the infoset. This insertion of whitespace only causes problems when the type is xs:string. (Which is what the CDATA bracketing or escaping all whitespace is for)
So for numbers I think it's not a problem to tolerate whitespace when reading from XML.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] stevedlawrence commented on a diff in pull request #945: Normalize whitespace in simple type constructors with string values
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #945:
URL: https://github.com/apache/daffodil/pull/945#discussion_r1094679109
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/dpath/NodeInfo.scala:
##########
@@ -535,15 +535,16 @@ object NodeInfo extends Enum {
protected def fromString(s: String): DataValueNumber
def fromXMLString(s: String): DataValueNumber = {
Review Comment:
`fromXMLString` is also used to convert values from an infoset to their primitives types (and maybe a couple other places as well). So this change would allow whitespace in a infoset numeric value, for example `<foo> 123 </foo>` would be allowed.
I'm not sure what DFDL or XML says about that? Maybe that should be allowed and this fixes more than just our DPath stuff and makes infoset parsing a little more lax. I think XML schema does allow the whitespace when validating numerics, so maybe this is a good change?
But if either says it shouldn't be allowed, maybe the trim should only be called in the StringTo* functions in ConverterOps.scala, which is specific to DPath?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] tuxji commented on a diff in pull request #945: Normalize whitespace in simple type constructors with string values
Posted by "tuxji (via GitHub)" <gi...@apache.org>.
tuxji commented on code in PR #945:
URL: https://github.com/apache/daffodil/pull/945#discussion_r1094710130
##########
daffodil-test/src/test/resources/org/apache/daffodil/section23/dfdl_functions/Functions.tdml:
##########
@@ -6805,7 +6812,8 @@
<xs:element name="dfdlULong09" type="xs:unsignedLong" dfdl:inputValueCalc="{ dfdl:unsignedLong(dfdl:hexBinary(9223372036854775807)) }"/> <!-- valid -->
<xs:element name="dfdlULong11" type="xs:unsignedLong" dfdl:inputValueCalc="{ dfdl:unsignedLong('xFFFFFFFFFFFFFFFG') }"/> <!-- invalid -->
<xs:element name="dfdlULong12" type="xs:unsignedLong" dfdl:inputValueCalc="{ dfdl:unsignedLong(-1) }"/> <!-- invalid -->
-
+ <xs:element name="dfdlULong13" type="xs:unsignedLong" dfdl:inputValueCalc="{ dfdl:unsignedLong(' 18446744073709551615') }"/> <!-- valid -->
Review Comment:
The JIRA issue says XPath performs whitespace normalization which removes leading AND trailing whitespace. None of your new tests have any trailing whitespace, even though the trim method will remove it too. Other than that, I'll let Mike and Steve review.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] stevedlawrence merged pull request #945: Normalize whitespace in simple type constructors with string values
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence merged PR #945:
URL: https://github.com/apache/daffodil/pull/945
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org