[GitHub] [daffodil] stevedlawrence commented on a diff in pull request #934: Fix handling of NaN/Infinifty when parsing text integers

["stevedlawrence (via GitHub)" <gi...@apache.org>]

stevedlawrence commented on code in PR #934:
URL: https://github.com/apache/daffodil/pull/934#discussion_r1087024338


##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ConvertTextStandardNumberParser.scala:
##########
@@ -173,27 +173,32 @@ case class ConvertTextStandardNumberParser(
         val df = textNumberFormatEv.evaluate(start).get
         val strCheckPolicy = if (df.isParseStrict) str else str.trim
         val pos = new ParsePosition(0)
-        val icuNum: Number = df.parse(strCheckPolicy, pos)
-
-
-        if (icuNum == null) {
-          PE(start, "Unable to parse %s from text: %s",
-            context.optPrimType.get.globalQName, str)
-          return
-        }
-
-        // sometimes ICU will return their own custom BigDecimal, even if the
-        // value could be represented as a BigInteger. We only want Java types,
-        // so detect this and convert it to the appropriate type
-        val num1 = icuNum match {
+        val icuNum: Number = df.parse(strCheckPolicy, pos) match {
+          case null => {
+            PE(start, "Unable to parse %s from text: %s", context.optPrimType.get.globalQName, str)
+            return
+          }
+          case d: JDouble if primNumeric.isInteger => {
+            // If ICU returns a Double when only integers are expected, it means the
+            // string must have been NaN, -Infinity, or Infinity using the locales
+            // default symbols. There does not seem to be a way to disable this even
+            // with setParseBigDecimal to false and setParseIntegerOnly set to true.
+            // So just create the same PE as if it failed to parse it, which is what
+            // we really want ICU to do
+            Assert.invariant(d.isNaN || d.isInfinite)
+            PE(start, "Unable to parse %s from text: %s", context.optPrimType.get.globalQName, str)
+            return
+          }
           case bd: ICUBigDecimal => {
+            // sometimes ICU will return their own custom BigDecimal, even if the
+            // value could be represented as a BigInteger. We only want Java types,
+            // so detect this and convert it to the appropriate type
             if (bd.scale == 0) bd.unscaledValue
             else bd.toBigDecimal
           }
-          case _ => icuNum
+          case num => num

Review Comment:
   Scala has no problem with it and I guess is able to detect all cases are covered. Will add.



-- 
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