diff --git a/artio-codecs/src/main/java/uk/co/real_logic/artio/fields/DecimalFloat.java b/artio-codecs/src/main/java/uk/co/real_logic/artio/fields/DecimalFloat.java index 2d6a1775ec..d28a174794 100644 --- a/artio-codecs/src/main/java/uk/co/real_logic/artio/fields/DecimalFloat.java +++ b/artio-codecs/src/main/java/uk/co/real_logic/artio/fields/DecimalFloat.java @@ -81,7 +81,7 @@ public DecimalFloat(final long value, final int scale) } /** - * Resets the encoder to the NAN value. This can checked using the {@link #isNaNValue()} method. + * Resets the encoder to the NAN value. This can be checked using the {@link #isNaNValue()} method. */ public void reset() { diff --git a/artio-codecs/src/main/java/uk/co/real_logic/artio/util/float_parsing/DecimalFloatParser.java b/artio-codecs/src/main/java/uk/co/real_logic/artio/util/float_parsing/DecimalFloatParser.java index 3318da6954..f4a33b2419 100644 --- a/artio-codecs/src/main/java/uk/co/real_logic/artio/util/float_parsing/DecimalFloatParser.java +++ b/artio-codecs/src/main/java/uk/co/real_logic/artio/util/float_parsing/DecimalFloatParser.java @@ -20,7 +20,7 @@ public static DecimalFloat extract( final int offset, final int length) { - // Throw away trailing spaces or zeros + // Throw away trailing spaces int workingOffset = offset; int end = workingOffset + length; for (int index = end - 1; charReader.isSpace(data, index) && index > workingOffset; index--) @@ -28,19 +28,20 @@ public static DecimalFloat extract( end--; } - int endDiff = 0; - for (int index = end - 1; charReader.isZero(data, index) && index > workingOffset; index--) - { - endDiff++; - } + int endOfSignificand = findEndOfSignificand(charReader, workingOffset, end, data); + final int startOfExponent = endOfSignificand + 1; - if (isFloatingPoint(charReader, workingOffset, end, endDiff, data)) + if (isFloatingPoint(charReader, workingOffset, endOfSignificand, data)) { - end -= endDiff; + // Throw away trailing zeros + for (int index = endOfSignificand - 1; charReader.isZero(data, index) && index > workingOffset; index--) + { + endOfSignificand--; + } } // Throw away leading spaces - for (int index = workingOffset; charReader.isSpace(data, index) && index < end; index++) + for (int index = workingOffset; index < endOfSignificand && charReader.isSpace(data, index); index++) { workingOffset++; } @@ -53,78 +54,117 @@ public static DecimalFloat extract( } // Throw away leading zeros - for (int index = workingOffset; index < end && charReader.isZero(data, index); index++) + for (int index = workingOffset; index < endOfSignificand && charReader.isZero(data, index); index++) { workingOffset++; } int workingScale = 0; long value = 0; - int base10exponent = 0; - boolean isScientificNotation = false; - short scaleDecrementValue = 0; - short scientificExponentMultiplier = -1; - for (int index = workingOffset; index < end; index++) + for (int index = workingOffset; index < endOfSignificand; index++) { final char charValue = charReader.charAt(data, index); if (charValue == DOT) { // number of digits after the dot - workingScale = end - (index + 1); - scaleDecrementValue = 1; - } - else if (charValue == LOWER_CASE_E || charValue == UPPER_CASE_E) - { - isScientificNotation = true; - - workingScale -= scaleDecrementValue; - } - else if (isScientificNotation && charValue == PLUS) - { - workingScale -= scaleDecrementValue; - } - else if (isScientificNotation && charValue == MINUS) - { - workingScale -= scaleDecrementValue; - scientificExponentMultiplier = 1; + workingScale = endOfSignificand - (index + 1); } else { final int digit = charReader.getDigit(data, index, charValue); - if (isScientificNotation) - { - base10exponent = base10exponent * 10 + digit; - workingScale -= scaleDecrementValue; - } - else + value = value * 10 + digit; + if (value < 0) { - value = value * 10 + digit; - if (value < 0) - { - throw new ArithmeticException( - "Out of range: when parsing " + charReader.asString(data, offset, length)); - } + throw new ArithmeticException( + "Out of range: when parsing " + charReader.asString(data, offset, length)); } } } - final int scale = workingScale + (scientificExponentMultiplier * base10exponent); + int exponent = 0; + final int exponentLength = end - startOfExponent; + if (exponentLength > 0) + { + // scientific notation + exponent = parseExponent(charReader, data, offset, length, startOfExponent, end); + } + else if (exponentLength == 0) + { + throw new NumberFormatException(charReader.asString(data, offset, length).toString()); + } + + final int scale = workingScale - exponent; final long signedValue = negative ? -1 * value : value; return number.set( - (scale >= 0) ? signedValue : signedValue * pow10(-scale), - Math.max(scale, 0) + (scale >= 0) ? signedValue : signedValue * pow10(-scale), + Math.max(scale, 0) ); } + private static int parseExponent( + final CharReader charReader, + final Data data, + final int offset, + final int length, + final int startOfExponent, + final int end) + { + int exponent = 0; + boolean negative = false; + int position = startOfExponent; + + final char firstChar = charReader.charAt(data, position); + if (firstChar == MINUS) + { + position++; + negative = true; + } + else if (firstChar == PLUS) + { + position++; + } + + while (position < end) + { + final char charValue = charReader.charAt(data, position); + final int digit = charReader.getDigit(data, position, charValue); + position++; + exponent = exponent * 10 + digit; + if (exponent > 1000) // overflow and arbitrary limit check + { + throw new NumberFormatException(charReader.asString(data, offset, length).toString()); + } + } + + return negative ? -exponent : exponent; + } + + private static int findEndOfSignificand( + final CharReader dataExtractor, + final int offset, + final int end, + final Data data + ) + { + for (int index = end - 1; index > offset; index--) + { + final char charValue = dataExtractor.charAt(data, index); + if (charValue == LOWER_CASE_E || charValue == UPPER_CASE_E) + { + return index; + } + } + return end; + } + private static boolean isFloatingPoint( final CharReader dataExtractor, final int offset, final int end, - final int endDiff, final Data data ) { - for (int index = end - endDiff - 1; index > offset; index--) + for (int index = end - 1; index >= offset; index--) { if (dataExtractor.charAt(data, index) == '.') { diff --git a/artio-codecs/src/test/java/uk/co/real_logic/artio/fields/DecimalFloatDecodingTest.java b/artio-codecs/src/test/java/uk/co/real_logic/artio/fields/DecimalFloatDecodingTest.java index 0b52a5d3f1..52c92dd71f 100644 --- a/artio-codecs/src/test/java/uk/co/real_logic/artio/fields/DecimalFloatDecodingTest.java +++ b/artio-codecs/src/test/java/uk/co/real_logic/artio/fields/DecimalFloatDecodingTest.java @@ -62,6 +62,7 @@ public static Iterable decimalFloatCodecData() {"-0.9950", -995L, 3}, {"-25", -25L, 0}, {".6", 6L, 1}, + {".600", 6L, 1}, {".6e0", 6L, 1}, {".6e2", 60L, 0}, {".06", 6L, 2}, @@ -92,6 +93,22 @@ public static Iterable decimalFloatCodecData() {"0.00000001", 1, 8}, {"6456.123456789", 6456123456789L, 9}, {"6456.000000001", 6456000000001L, 9}, + + {"0", 0L, 0}, + {"00", 0L, 0}, + {"0.", 0L, 0}, + {".0", 0L, 0}, + {"0.0", 0L, 0}, + {"00.00", 0L, 0}, + {"0e0", 0L, 0}, + {"00e00", 0L, 0}, + {"00.00e00", 0L, 0}, + + {"1.0e0", 1L, 0}, + {"1.0e10", 10_000_000_000L, 0}, + {"1.0e+00010", 10_000_000_000L, 0}, + {"1.0e-10", 1L, 10}, + {"1.0e-100", 1L, 100}, }); } diff --git a/artio-codecs/src/test/java/uk/co/real_logic/artio/fields/DecimalFloatTest.java b/artio-codecs/src/test/java/uk/co/real_logic/artio/fields/DecimalFloatTest.java index 6f8a0b1f9d..ae70a2cc41 100644 --- a/artio-codecs/src/test/java/uk/co/real_logic/artio/fields/DecimalFloatTest.java +++ b/artio-codecs/src/test/java/uk/co/real_logic/artio/fields/DecimalFloatTest.java @@ -15,21 +15,18 @@ */ package uk.co.real_logic.artio.fields; -import org.junit.Test; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.comparesEqualTo; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.lessThan; - - +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import uk.co.real_logic.artio.util.AsciiBuffer; import uk.co.real_logic.artio.util.MutableAsciiBuffer; import static java.nio.charset.StandardCharsets.US_ASCII; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.jupiter.api.Assertions.assertThrows; -public class DecimalFloatTest +class DecimalFloatTest { private static final DecimalFloat ZERO = new DecimalFloat(0, 0); private static final DecimalFloat FIVE = new DecimalFloat(5, 0); @@ -40,7 +37,7 @@ public class DecimalFloatTest private static final DecimalFloat MINUS_FIVE_POINT_FIVE = new DecimalFloat(-55, 1); @Test - public void compareToDetectsEqualIntegers() + void compareToDetectsEqualIntegers() { assertThat(ZERO, comparesEqualTo(ZERO)); assertThat(FIVE, comparesEqualTo(new DecimalFloat(5, 0))); @@ -51,14 +48,14 @@ public void compareToDetectsEqualIntegers() } @Test - public void compareToOrdersIntegers() + void compareToOrdersIntegers() { assertOrderWithNegatives(ZERO, FIVE); assertOrderWithNegatives(MINUS_FIVE, FIVE); } @Test - public void compareToOrdersFloatsOfSameScale() + void compareToOrdersFloatsOfSameScale() { assertOrderWithNegatives(POINT_ONE, FIVE_POINT_FIVE); assertOrderWithNegatives(MINUS_FIVE_POINT_FIVE, POINT_ONE); @@ -66,7 +63,7 @@ public void compareToOrdersFloatsOfSameScale() } @Test - public void compareToOrdersFloatsWithIntegers() + void compareToOrdersFloatsWithIntegers() { assertOrderWithNegatives(ZERO, POINT_ONE); assertOrderWithNegatives(MINUS_FIVE_POINT_FIVE, ZERO); @@ -74,28 +71,28 @@ public void compareToOrdersFloatsWithIntegers() } @Test - public void compareToOrdersFloatsOfDifferentScale() + void compareToOrdersFloatsOfDifferentScale() { assertOrderWithNegatives(new DecimalFloat(45, 2), new DecimalFloat(45, 1)); assertOrderWithNegatives(new DecimalFloat(9, 2), POINT_ONE); } @Test - public void compareToOrdersFloatsOfDifferentScaleVsZero() + void compareToOrdersFloatsOfDifferentScaleVsZero() { assertOrderWithNegatives(ZERO, new DecimalFloat(45, 1)); assertOrderWithNegatives(ZERO, new DecimalFloat(45, 2)); } @Test - public void compareToOrderFloatsOfDifferentScaleWithMultiDigitValues() + void compareToOrderFloatsOfDifferentScaleWithMultiDigitValues() { assertOrderWithNegatives(new DecimalFloat(54321, 2), new DecimalFloat(543219, 3)); assertOrderWithNegatives(new DecimalFloat(54321, 2), new DecimalFloat(5433, 1)); } @Test - public void normaliseValuesDuringConstruction() + void normaliseValuesDuringConstruction() { assertThat(new DecimalFloat(0, 0), equalTo(new DecimalFloat(0, 0))); assertThat(new DecimalFloat(0, 0), equalTo(new DecimalFloat(0, 25))); @@ -105,54 +102,54 @@ public void normaliseValuesDuringConstruction() assertThat(new DecimalFloat(1234, 2), equalTo(new DecimalFloat(123400, 4))); } - @Test(expected = NumberFormatException.class) - public void shouldNotConvertInvalidStringIntoANumber() + @Test + void shouldNotConvertInvalidStringIntoANumber() { - new DecimalFloat().fromString("ABC"); + assertThrows(NumberFormatException.class, () -> new DecimalFloat().fromString("ABC")); } - @Test(expected = ArithmeticException.class) - public void shouldNotParseValueOutOfRange() + @Test + void shouldNotParseValueOutOfRange() { // Valid decimal floats have max 18 digits, could also have e, E, -, + or . - new DecimalFloat().fromString("10000000000000000000000"); + assertThrows(ArithmeticException.class, () -> new DecimalFloat().fromString("10000000000000000000000")); } // Bug reproduction testcase - @Test(expected = ArithmeticException.class) - public void shouldNotParseOverflowingValue() + @Test + void shouldNotParseOverflowingValue() { - new DecimalFloat().fromString("99999999999999990000000"); + assertThrows(ArithmeticException.class, () -> new DecimalFloat().fromString("99999999999999990000000")); } @Test - public void parseZeroDecimalFloat() + void parseZeroDecimalFloat() { assertThat(new DecimalFloat(0, 0), equalTo(new DecimalFloat().fromString("0"))); } // Bug reproduction testcase - @Test(expected = ArithmeticException.class) - public void shouldNotDecodeOverflowingValue() + @Test + void shouldNotDecodeOverflowingValue() { - parseNumberFromBuffer("99999999999999990000000"); + assertThrows(ArithmeticException.class, () -> parseNumberFromBuffer("99999999999999990000000")); } - @Test(expected = ArithmeticException.class) - public void shouldNotDecodeValueOutOfRange() + @Test + void shouldNotDecodeValueOutOfRange() { - parseNumberFromBuffer("10000000000000000000000"); + assertThrows(ArithmeticException.class, () -> parseNumberFromBuffer("10000000000000000000000")); } - @Test(expected = IllegalArgumentException.class) - public void shouldNotEncodeAnInvalidValue() + @Test + void shouldNotEncodeAnInvalidValue() { final MutableAsciiBuffer buffer = new MutableAsciiBuffer(new byte[1000]); - buffer.putFloatAscii(0, DecimalFloat.NAN); + assertThrows(IllegalArgumentException.class, () -> buffer.putFloatAscii(0, DecimalFloat.NAN)); } @Test - public void shouldNotBeAbleToRedefineConstantValues() + void shouldNotBeAbleToRedefineConstantValues() { final DecimalFloat value; final DecimalFloat zero = DecimalFloat.ZERO.mutableCopy(); @@ -168,6 +165,22 @@ public void shouldNotBeAbleToRedefineConstantValues() assertThat(DecimalFloat.ZERO, equalTo(zero)); } + @ParameterizedTest + @ValueSource(strings = { + "e1", + "1e", + "1e2.3", + "1e2147483647", + "1e2147483648", + "1e-2147483648", + "1e-2147483649", + }) + void shouldThrowWhenDecodedStringIsInvalid(final String invalidFloat) + { + final DecimalFloat df = new DecimalFloat(); + assertThrows(NumberFormatException.class, () -> df.fromString(invalidFloat)); + } + private void parseNumberFromBuffer(final String number) { final AsciiBuffer buffer = new MutableAsciiBuffer(number.getBytes(US_ASCII));