The problem lies in the following code:
|
@Override |
|
public void writeNumber(String encodedValue) |
|
throws IOException, JsonGenerationException, UnsupportedOperationException |
|
{ |
|
// There is a room to improve this API's performance while the implementation is robust. |
|
// If users can use other MessagePackGenerator#writeNumber APIs that accept |
|
// proper numeric types not String, it's better to use the other APIs instead. |
|
try { |
|
long l = Long.parseLong(encodedValue); |
|
addValueToStackTop(l); |
|
return; |
|
} |
|
catch (NumberFormatException e) { |
|
} |
|
|
|
try { |
|
double d = Double.parseDouble(encodedValue); |
|
addValueToStackTop(d); |
|
return; |
|
} |
|
catch (NumberFormatException e) { |
|
} |
|
|
|
try { |
|
BigInteger bi = new BigInteger(encodedValue); |
|
addValueToStackTop(bi); |
|
return; |
|
} |
|
catch (NumberFormatException e) { |
|
} |
|
|
|
try { |
|
BigDecimal bc = new BigDecimal(encodedValue); |
|
addValueToStackTop(bc); |
|
return; |
|
} |
|
catch (NumberFormatException e) { |
|
} |
|
|
|
throw new NumberFormatException(encodedValue); |
|
} |
I will provide an example. Take the following input: "9223372036854775808" (2^63)
Since Javas Long type max value is 2^63-1, the first try will fail, continuing to the next try. Double will succeed resulting in the input being encoded as a float64. As a user of the library I would've assumed that the input got encoded as a uint64.
I'm not sure, but it may also be a problem that integers can silently lose precision. I think an IllegalArgumentException would've been preferred in that case, unless there are decimals in the number (even if just a trailing zero).
Double#parseDouble will succeed to parse all integers, although integers past Double.MAX_VALUE will yield Infinity, so the try to parse BigInteger will either never be reached or never succeed for any input.
Unless I'm mistaken, these issues could be solved by swapping the ordering of the parse tries, placing BigInteger before Double, and in that case I don't think it's even necessary to try parse Long and Double rather than just BigInteger and BigDecimal. Maybe it's less performant?
I realize that the other writeNumber methods are preferred over this one. I'm using a library that has a dependency on this library (dd-trace-java) where the same assumption that a string of a 64-bit integer would be encoded as a uint64 was made.
I will make a pull request shortly.
The problem lies in the following code:
msgpack-java/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackGenerator.java
Lines 437 to 477 in 87d38a0
I will provide an example. Take the following input: "9223372036854775808" (2^63)
Since Javas
Longtype max value is 2^63-1, the first try will fail, continuing to the next try.Doublewill succeed resulting in the input being encoded as afloat64. As a user of the library I would've assumed that the input got encoded as auint64.I'm not sure, but it may also be a problem that integers can silently lose precision. I think an
IllegalArgumentExceptionwould've been preferred in that case, unless there are decimals in the number (even if just a trailing zero).Double#parseDoublewill succeed to parse all integers, although integers pastDouble.MAX_VALUEwill yieldInfinity, so the try to parseBigIntegerwill either never be reached or never succeed for any input.Unless I'm mistaken, these issues could be solved by swapping the ordering of the parse tries, placing
BigIntegerbeforeDouble, and in that case I don't think it's even necessary to try parseLongandDoublerather than just BigInteger andBigDecimal. Maybe it's less performant?I realize that the other
writeNumbermethods are preferred over this one. I'm using a library that has a dependency on this library (dd-trace-java) where the same assumption that a string of a 64-bit integer would be encoded as a uint64 was made.I will make a pull request shortly.