From 70fb6d2173bd372cffb9b4d87b68b2f5268eb8f7 Mon Sep 17 00:00:00 2001 From: Andriy Plokhotnyuk Date: Tue, 11 Feb 2025 19:47:53 +0100 Subject: [PATCH] Fix decoding invalid JSON values as `NaN` and `Infinity` values of doubles and floats (#1304) --- .../zio/json/internal/UnsafeNumbers.scala | 21 ++++----- .../zio/json/internal/UnsafeNumbers.scala | 21 ++++----- .../zio/json/internal/UnsafeNumbers.scala | 21 ++++----- .../src/test/scala/zio/json/DecoderSpec.scala | 44 ++++++++++++++----- 4 files changed, 57 insertions(+), 50 deletions(-) diff --git a/zio-json/js/src/main/scala/zio/json/internal/UnsafeNumbers.scala b/zio-json/js/src/main/scala/zio/json/internal/UnsafeNumbers.scala index d4a3d626..96c51caa 100644 --- a/zio-json/js/src/main/scala/zio/json/internal/UnsafeNumbers.scala +++ b/zio-json/js/src/main/scala/zio/json/internal/UnsafeNumbers.scala @@ -139,12 +139,9 @@ object UnsafeNumbers { } } if (consume && current != -1) throw UnsafeNumber - if (hiM10 eq null) { - if (negate) loM10 = -loM10 - return java.math.BigInteger.valueOf(loM10) - } + if (negate) loM10 = -loM10 + if (hiM10 eq null) return java.math.BigInteger.valueOf(loM10) if (loDigits != 0) { - if (negate) loM10 = -loM10 hiM10 = hiM10.scaleByPowerOfTen(loDigits).add(java.math.BigDecimal.valueOf(loM10)) if (hiM10.unscaledValue.bitLength >= max_bits) throw UnsafeNumber } @@ -224,8 +221,6 @@ object UnsafeNumbers { if (negate) loM10 = -loM10 return java.math.BigDecimal.valueOf(loM10, -e10) } - val scale = loDigits + e10 - if (((loDigits ^ scale) & (e10 ^ scale)) < 0) throw UnsafeNumber toBigDecimal(hiM10, loM10, loDigits, e10, max_bits, negate) } @@ -241,8 +236,12 @@ object UnsafeNumbers { if (negate) loM10 = -loM10 val bd = java.math.BigDecimal.valueOf(loM10, -e10) if (hi eq null) return bd - val scale = loDigits + e10 + var scale = loDigits var hiM10 = hi + if (e10 != 0) { + scale += e10 + if (((loDigits ^ scale) & (e10 ^ scale)) < 0) throw UnsafeNumber + } if (scale != 0) hiM10 = hiM10.scaleByPowerOfTen(scale) hiM10 = hiM10.add(bd) if (hiM10.unscaledValue.bitLength >= max_bits) throw UnsafeNumber @@ -340,8 +339,6 @@ object UnsafeNumbers { if (negate) x = -x return x } - val scale = loDigits + e10 - if (((loDigits ^ scale) & (e10 ^ scale)) < 0) throw UnsafeNumber toBigDecimal(hiM10, loM10, loDigits, e10, max_bits, negate).floatValue } @@ -473,8 +470,6 @@ object UnsafeNumbers { if (negate) x = -x return x } - val scale = loDigits + e10 - if (((loDigits ^ scale) & (e10 ^ scale)) < 0) throw UnsafeNumber toBigDecimal(hiM10, loM10, loDigits, e10, max_bits, negate).doubleValue } @@ -519,7 +514,7 @@ object UnsafeNumbers { i += 1 } val current = in.read() // to be consistent read the terminator - if (consume && current != -1) throw UnsafeNumber + if (consume && current != -1 || !consume && current != '"') throw UnsafeNumber } // 64-bit unsigned multiplication was adopted from the great Hacker's Delight function diff --git a/zio-json/jvm/src/main/scala/zio/json/internal/UnsafeNumbers.scala b/zio-json/jvm/src/main/scala/zio/json/internal/UnsafeNumbers.scala index 3f263624..89341f8f 100644 --- a/zio-json/jvm/src/main/scala/zio/json/internal/UnsafeNumbers.scala +++ b/zio-json/jvm/src/main/scala/zio/json/internal/UnsafeNumbers.scala @@ -139,12 +139,9 @@ object UnsafeNumbers { } } if (consume && current != -1) throw UnsafeNumber - if (hiM10 eq null) { - if (negate) loM10 = -loM10 - return java.math.BigInteger.valueOf(loM10) - } + if (negate) loM10 = -loM10 + if (hiM10 eq null) return java.math.BigInteger.valueOf(loM10) if (loDigits != 0) { - if (negate) loM10 = -loM10 hiM10 = hiM10.scaleByPowerOfTen(loDigits).add(java.math.BigDecimal.valueOf(loM10)) if (hiM10.unscaledValue.bitLength >= max_bits) throw UnsafeNumber } @@ -224,8 +221,6 @@ object UnsafeNumbers { if (negate) loM10 = -loM10 return java.math.BigDecimal.valueOf(loM10, -e10) } - val scale = loDigits + e10 - if (((loDigits ^ scale) & (e10 ^ scale)) < 0) throw UnsafeNumber toBigDecimal(hiM10, loM10, loDigits, e10, max_bits, negate) } @@ -241,8 +236,12 @@ object UnsafeNumbers { if (negate) loM10 = -loM10 val bd = java.math.BigDecimal.valueOf(loM10, -e10) if (hi eq null) return bd - val scale = loDigits + e10 + var scale = loDigits var hiM10 = hi + if (e10 != 0) { + scale += e10 + if (((loDigits ^ scale) & (e10 ^ scale)) < 0) throw UnsafeNumber + } if (scale != 0) hiM10 = hiM10.scaleByPowerOfTen(scale) hiM10 = hiM10.add(bd) if (hiM10.unscaledValue.bitLength >= max_bits) throw UnsafeNumber @@ -340,8 +339,6 @@ object UnsafeNumbers { if (negate) x = -x return x } - val scale = loDigits + e10 - if (((loDigits ^ scale) & (e10 ^ scale)) < 0) throw UnsafeNumber toBigDecimal(hiM10, loM10, loDigits, e10, max_bits, negate).floatValue } @@ -473,8 +470,6 @@ object UnsafeNumbers { if (negate) x = -x return x } - val scale = loDigits + e10 - if (((loDigits ^ scale) & (e10 ^ scale)) < 0) throw UnsafeNumber toBigDecimal(hiM10, loM10, loDigits, e10, max_bits, negate).doubleValue } @@ -519,7 +514,7 @@ object UnsafeNumbers { i += 1 } val current = in.read() // to be consistent read the terminator - if (consume && current != -1) throw UnsafeNumber + if (consume && current != -1 || !consume && current != '"') throw UnsafeNumber } @inline private[this] def unsignedMultiplyHigh(x: Long, y: Long): Long = diff --git a/zio-json/native/src/main/scala/zio/json/internal/UnsafeNumbers.scala b/zio-json/native/src/main/scala/zio/json/internal/UnsafeNumbers.scala index 3f263624..89341f8f 100644 --- a/zio-json/native/src/main/scala/zio/json/internal/UnsafeNumbers.scala +++ b/zio-json/native/src/main/scala/zio/json/internal/UnsafeNumbers.scala @@ -139,12 +139,9 @@ object UnsafeNumbers { } } if (consume && current != -1) throw UnsafeNumber - if (hiM10 eq null) { - if (negate) loM10 = -loM10 - return java.math.BigInteger.valueOf(loM10) - } + if (negate) loM10 = -loM10 + if (hiM10 eq null) return java.math.BigInteger.valueOf(loM10) if (loDigits != 0) { - if (negate) loM10 = -loM10 hiM10 = hiM10.scaleByPowerOfTen(loDigits).add(java.math.BigDecimal.valueOf(loM10)) if (hiM10.unscaledValue.bitLength >= max_bits) throw UnsafeNumber } @@ -224,8 +221,6 @@ object UnsafeNumbers { if (negate) loM10 = -loM10 return java.math.BigDecimal.valueOf(loM10, -e10) } - val scale = loDigits + e10 - if (((loDigits ^ scale) & (e10 ^ scale)) < 0) throw UnsafeNumber toBigDecimal(hiM10, loM10, loDigits, e10, max_bits, negate) } @@ -241,8 +236,12 @@ object UnsafeNumbers { if (negate) loM10 = -loM10 val bd = java.math.BigDecimal.valueOf(loM10, -e10) if (hi eq null) return bd - val scale = loDigits + e10 + var scale = loDigits var hiM10 = hi + if (e10 != 0) { + scale += e10 + if (((loDigits ^ scale) & (e10 ^ scale)) < 0) throw UnsafeNumber + } if (scale != 0) hiM10 = hiM10.scaleByPowerOfTen(scale) hiM10 = hiM10.add(bd) if (hiM10.unscaledValue.bitLength >= max_bits) throw UnsafeNumber @@ -340,8 +339,6 @@ object UnsafeNumbers { if (negate) x = -x return x } - val scale = loDigits + e10 - if (((loDigits ^ scale) & (e10 ^ scale)) < 0) throw UnsafeNumber toBigDecimal(hiM10, loM10, loDigits, e10, max_bits, negate).floatValue } @@ -473,8 +470,6 @@ object UnsafeNumbers { if (negate) x = -x return x } - val scale = loDigits + e10 - if (((loDigits ^ scale) & (e10 ^ scale)) < 0) throw UnsafeNumber toBigDecimal(hiM10, loM10, loDigits, e10, max_bits, negate).doubleValue } @@ -519,7 +514,7 @@ object UnsafeNumbers { i += 1 } val current = in.read() // to be consistent read the terminator - if (consume && current != -1) throw UnsafeNumber + if (consume && current != -1 || !consume && current != '"') throw UnsafeNumber } @inline private[this] def unsignedMultiplyHigh(x: Long, y: Long): Long = diff --git a/zio-json/shared/src/test/scala/zio/json/DecoderSpec.scala b/zio-json/shared/src/test/scala/zio/json/DecoderSpec.scala index 0754c72b..98d33f45 100644 --- a/zio-json/shared/src/test/scala/zio/json/DecoderSpec.scala +++ b/zio-json/shared/src/test/scala/zio/json/DecoderSpec.scala @@ -80,28 +80,49 @@ object DecoderSpec extends ZIOSpecDefault { assertTrue("\"NaN\"".fromJson[Long].isLeft) }, test("float") { - assert("-1.234567e9".fromJson[Float])(isRight(equalTo(-1.234567e9f))) && assert("1.234567e9".fromJson[Float])(isRight(equalTo(1.234567e9f))) && + assert("-1.234567e9".fromJson[Float])(isRight(equalTo(-1.234567e9f))) && assert("\"-1.234567e9\"".fromJson[Float])(isRight(equalTo(-1.234567e9f))) && - assert("-1.23456789012345678901e-2147483648".fromJson[Float])(isLeft(equalTo("(expected a Float)"))) && + assert("8.3e38".fromJson[Float])(isRight(equalTo(Float.PositiveInfinity))) && + assert("-8.3e38".fromJson[Float])(isRight(equalTo(Float.NegativeInfinity))) && + assert("1.23456789012345678901e-2147483648".fromJson[Float])(isLeft(equalTo("(expected a Float)"))) && assert("123456789012345678901e+2147483647".fromJson[Float])(isLeft(equalTo("(expected a Float)"))) && - assert("-123456789012345678901e+2147483647".fromJson[Float])(isLeft(equalTo("(expected a Float)"))) && + assert("1234567890123456789.01e+2147483647".fromJson[Float])(isLeft(equalTo("(expected a Float)"))) && + assert("1.0e-2147483647".fromJson[Float])(isRight(equalTo(0.0f))) && + assert("-1.0e-2147483647".fromJson[Float])(isRight(equalTo(-0.0f))) && + assert("123456789012345678.901e+2147483647".fromJson[Float])(isRight(equalTo(Float.PositiveInfinity))) && + assert("-123456789012345678.901e+2147483647".fromJson[Float])(isRight(equalTo(Float.NegativeInfinity))) && assert("\"Infinity\"".fromJson[Float])(isRight(equalTo(Float.PositiveInfinity))) && assert("\"+Infinity\"".fromJson[Float])(isRight(equalTo(Float.PositiveInfinity))) && assert("\"-Infinity\"".fromJson[Float])(isRight(equalTo(Float.NegativeInfinity))) && assertTrue("\"NaN\"".fromJson[Float].isRight) && + assertTrue("Infinity".fromJson[Float].isLeft) && + assertTrue("+Infinity".fromJson[Float].isLeft) && + assertTrue("-Infinity".fromJson[Float].isLeft) && + assertTrue("NaN".fromJson[Float].isLeft) && assertTrue("+1.234567e9".fromJson[Float].isLeft) }, test("double") { + assert("1.23456789012345e9".fromJson[Double])(isRight(equalTo(1.23456789012345e9))) && assert("-1.23456789012345e9".fromJson[Double])(isRight(equalTo(-1.23456789012345e9))) && assert("\"-1.23456789012345e9\"".fromJson[Double])(isRight(equalTo(-1.23456789012345e9))) && - assert("-1.23456789012345678901e-2147483648".fromJson[Double])(isLeft(equalTo("(expected a Double)"))) && + assert("1.8e308".fromJson[Double])(isRight(equalTo(Double.PositiveInfinity))) && + assert("-1.8e308".fromJson[Double])(isRight(equalTo(Double.NegativeInfinity))) && + assert("1.23456789012345678901e-2147483648".fromJson[Double])(isLeft(equalTo("(expected a Double)"))) && + assert("1234567890123456789.01e+2147483647".fromJson[Double])(isLeft(equalTo("(expected a Double)"))) && assert("123456789012345678901e+2147483647".fromJson[Double])(isLeft(equalTo("(expected a Double)"))) && - assert("-123456789012345678901e+2147483647".fromJson[Double])(isLeft(equalTo("(expected a Double)"))) && + assert("1.0e-2147483647".fromJson[Double])(isRight(equalTo(0.0))) && + assert("-1.0e-2147483647".fromJson[Double])(isRight(equalTo(-0.0))) && + assert("123456789012345678.901e+2147483647".fromJson[Double])(isRight(equalTo(Double.PositiveInfinity))) && + assert("-123456789012345678.901e+2147483647".fromJson[Double])(isRight(equalTo(Double.NegativeInfinity))) && assert("\"Infinity\"".fromJson[Double])(isRight(equalTo(Double.PositiveInfinity))) && assert("\"+Infinity\"".fromJson[Double])(isRight(equalTo(Double.PositiveInfinity))) && assert("\"-Infinity\"".fromJson[Double])(isRight(equalTo(Double.NegativeInfinity))) && assertTrue("\"NaN\"".fromJson[Double].isRight) && + assertTrue("Infinity".fromJson[Double].isLeft) && + assertTrue("+Infinity".fromJson[Double].isLeft) && + assertTrue("-Infinity".fromJson[Double].isLeft) && + assertTrue("NaN".fromJson[Double].isLeft) && assertTrue("+1.23456789012345e9".fromJson[Double].isLeft) }, test("BigDecimal") { @@ -130,10 +151,10 @@ object DecoderSpec extends ZIOSpecDefault { assert("1.23456789012345678901e-2147483648".fromJson[BigDecimal])( isLeft(equalTo("(expected a BigDecimal with 256-bit mantissa)")) ) && - assert("123456789012345678901e+2147483647".fromJson[BigDecimal])( + assert("1234567890123456789.01e+2147483647".fromJson[BigDecimal])( isLeft(equalTo("(expected a BigDecimal with 256-bit mantissa)")) ) && - assert("-123456789012345678901e+2147483647".fromJson[BigDecimal])( + assert("123456789012345678901e+2147483647".fromJson[BigDecimal])( isLeft(equalTo("(expected a BigDecimal with 256-bit mantissa)")) ) }, @@ -150,13 +171,14 @@ object DecoderSpec extends ZIOSpecDefault { assertTrue("\"NaN\"".fromJson[BigInteger].isLeft) }, test("BigInteger too large") { - // this big integer consumes more than 256 bits assert( "170141183460469231731687303715884105728489465165484668486513574864654818964653168465316546851" .fromJson[java.math.BigInteger] - )( - isLeft(equalTo("(expected a 256-bit BigInteger)")) - ) + )(isLeft(equalTo("(expected a 256-bit BigInteger)"))) && + assert( + "17014118346046923173168730371588410572848946516548466848651357486465481896465316846" + .fromJson[java.math.BigInteger] + )(isLeft(equalTo("(expected a 256-bit BigInteger)"))) }, test("collections") { val arr = """[1, 2, 3]"""