Repository: eclipse/leshan (java)

Issue:
LwM2mNodeSenMLDecoder sometimes fails trying to convert from double to long
Hi,

We are using Leshan 2.0.0-M5.
In the log files I sometimes see that when reading currentTime resource from device object (3/0/13) decoding sometimes fails with the following exception:

08:50:47 [CoapServer(main)#27] ERROR c.n.s.d.s.s.d.ManagerImpl - Endpoint 94193A0407000000: Read request for path 3 failed with exception: {}
org.eclipse.leshan.core.request.exception.InvalidResponseException: Unable to decode response payload of request [ReadRequest [path=/3 format=null]] from client [94193A0407000000]
	at org.eclipse.leshan.server.californium.request.LwM2mResponseBuilder.decodeCoapResponse(LwM2mResponseBuilder.java:458)
	at org.eclipse.leshan.server.californium.request.LwM2mResponseBuilder.visit(LwM2mResponseBuilder.java:122)
	at org.eclipse.leshan.core.request.ReadRequest.accept(ReadRequest.java:187)
	at org.eclipse.leshan.server.californium.request.RequestSender$2.buildResponse(RequestSender.java:226)
	at org.eclipse.leshan.core.californium.AsyncRequestObserver$1.onResponse(AsyncRequestObserver.java:60)
	at org.eclipse.leshan.core.californium.CoapAsyncRequestObserver.onResponse(CoapAsyncRequestObserver.java:99)
	at org.eclipse.californium.core.coap.Request.setResponse(Request.java:931)
	at org.eclipse.californium.core.server.ServerMessageDeliverer.deliverResponse(ServerMessageDeliverer.java:258)
	at org.eclipse.californium.core.network.stack.BaseCoapStack$StackTopAdapter.receiveResponse(BaseCoapStack.java:217)
	at org.eclipse.californium.core.network.stack.AbstractLayer.receiveResponse(AbstractLayer.java:89)
	at org.eclipse.californium.core.network.stack.ExchangeCleanupLayer.receiveResponse(ExchangeCleanupLayer.java:105)
	at org.eclipse.californium.core.network.stack.ObserveLayer.receiveResponse(ObserveLayer.java:139)
	at org.eclipse.californium.core.network.stack.BlockwiseLayer.receiveResponse(BlockwiseLayer.java:776)
	at org.eclipse.californium.core.network.stack.ReliabilityLayer.receiveResponse(ReliabilityLayer.java:308)
	at org.eclipse.californium.core.network.stack.AbstractLayer.receiveResponse(AbstractLayer.java:89)
	at org.eclipse.californium.core.network.stack.BaseCoapStack.receiveResponse(BaseCoapStack.java:146)
	at org.eclipse.californium.core.network.CoapEndpoint$1.receiveResponse(CoapEndpoint.java:314)
	at org.eclipse.californium.core.network.UdpMatcher$4.run(UdpMatcher.java:457)
	at org.eclipse.californium.elements.util.SerialExecutor$1.run(SerialExecutor.java:290)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:834)
Caused by: org.eclipse.leshan.core.node.codec.CodecException: Invalid content [1.638435E9] for type TIME for path /3/0/13
	at org.eclipse.leshan.core.node.codec.senml.LwM2mNodeSenMLDecoder.parseResourceValue(LwM2mNodeSenMLDecoder.java:504)
	at org.eclipse.leshan.core.node.codec.senml.LwM2mNodeSenMLDecoder.extractLwM2mResources(LwM2mNodeSenMLDecoder.java:426)
	at org.eclipse.leshan.core.node.codec.senml.LwM2mNodeSenMLDecoder.parseRecords(LwM2mNodeSenMLDecoder.java:193)
	at org.eclipse.leshan.core.node.codec.senml.LwM2mNodeSenMLDecoder.decode(LwM2mNodeSenMLDecoder.java:97)
	at org.eclipse.leshan.core.node.codec.DefaultLwM2mDecoder.decode(DefaultLwM2mDecoder.java:156)
	at org.eclipse.leshan.core.node.codec.DefaultLwM2mDecoder.decode(DefaultLwM2mDecoder.java:138)
	at org.eclipse.leshan.server.californium.request.LwM2mResponseBuilder.decodeCoapResponse(LwM2mResponseBuilder.java:450)
	... 21 common frames omitted
Caused by: java.lang.IllegalStateException: Can not convert 1.638435E9 to long safely : Unsupported number java.lang.Double
	at org.eclipse.leshan.core.util.datatype.NumberUtil.numberToLong(NumberUtil.java:59)
	at org.eclipse.leshan.core.node.codec.senml.LwM2mNodeSenMLDecoder.numberToLong(LwM2mNodeSenMLDecoder.java:527)
	at org.eclipse.leshan.core.node.codec.senml.LwM2mNodeSenMLDecoder.parseResourceValue(LwM2mNodeSenMLDecoder.java:493)
	... 27 common frames omitted

Best regards,
Sonny

Pull request description:
Support safe conversion from double to long in NumberUtils
This aims to fix #1170.

For now just a test is added and show that : 
`[{"bn":"/3/0/13","v":1.638435E9}]` senml+json payload failed as reported : 
```
org.eclipse.leshan.core.node.codec.CodecException: Invalid content [1.638435E9] for type TIME for path /3/0/13
	at org.eclipse.leshan.core.node.codec.senml.LwM2mNodeSenMLDecoder.parseResourceValue(LwM2mNodeSenMLDecoder.java:504)
	at org.eclipse.leshan.core.node.codec.senml.LwM2mNodeSenMLDecoder.extractLwM2mResources(LwM2mNodeSenMLDecoder.java:426)
	at org.eclipse.leshan.core.node.codec.senml.LwM2mNodeSenMLDecoder.parseRecords(LwM2mNodeSenMLDecoder.java:229)
	at org.eclipse.leshan.core.node.codec.senml.LwM2mNodeSenMLDecoder.decode(LwM2mNodeSenMLDecoder.java:97)
	at org.eclipse.leshan.core.node.codec.DefaultLwM2mDecoder.decode(DefaultLwM2mDecoder.java:156)
	at org.eclipse.leshan.core.node.codec.LwM2mNodeDecoderTest.senml_json_decode_single_resource_using_exponantial_notation(LwM2mNodeDecoderTest.java:915)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:89)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:41)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:541)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:763)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:463)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:209)
Caused by: java.lang.IllegalStateException: Can not convert 1.638435E9 to long safely : Unsupported number java.lang.Double
	at org.eclipse.leshan.core.util.datatype.NumberUtil.numberToLong(NumberUtil.java:59)
	at org.eclipse.leshan.core.node.codec.senml.LwM2mNodeSenMLDecoder.numberToLong(LwM2mNodeSenMLDecoder.java:527)
	at org.eclipse.leshan.core.node.codec.senml.LwM2mNodeSenMLDecoder.parseResourceValue(LwM2mNodeSenMLDecoder.java:493)
	... 32 more
```

Using the payload `[{"bn":"/3/0/13","v":1638435000}]` works as expected. (like guessed in https://github.com/eclipse/leshan/issues/1170#issuecomment-984631579)

Patch:
diff --git a/leshan-core/src/main/java/org/eclipse/leshan/core/util/datatype/NumberUtil.java b/leshan-core/src/main/java/org/eclipse/leshan/core/util/datatype/NumberUtil.java
index 10965acc..654fff1d 100644
--- a/leshan-core/src/main/java/org/eclipse/leshan/core/util/datatype/NumberUtil.java
+++ b/leshan-core/src/main/java/org/eclipse/leshan/core/util/datatype/NumberUtil.java
@@ -15,6 +15,7 @@
  *******************************************************************************/
 package org.eclipse.leshan.core.util.datatype;
 
+import java.math.BigDecimal;
 import java.math.BigInteger;
 
 import com.upokecenter.numbers.EInteger;
@@ -29,7 +30,7 @@ public class NumberUtil {
      * 
      * @throws IllegalArgumentException if the number can not be store in a long.
      */
-    public static Long numberToLong(Number number) throws IllegalStateException {
+    public static Long numberToLong(Number number) throws IllegalArgumentException {
         // handle INTEGER
         if (number instanceof Byte || number instanceof Short || number instanceof Integer || number instanceof Long) {
             return number.longValue();
@@ -39,24 +40,36 @@ public class NumberUtil {
             BigInteger bigInt = (BigInteger) number;
             if (bigInt.compareTo(BigInteger.valueOf(Long.MIN_VALUE)) < 0
                     || bigInt.compareTo(BigInteger.valueOf(Long.MAX_VALUE)) > 0) {
-                throw new IllegalStateException(String.format("%s  : can not be store in a long", bigInt));
+                throw new IllegalArgumentException(String.format("%s  : can not be store in a long", bigInt));
             }
             return bigInt.longValue();
         }
 
         // handle FLOATING-POINT
-        // TODO should we support a safe floating-point conversion
+        BigDecimal bigDec = null;
+        if (number instanceof Float || number instanceof Double) {
+            bigDec = new BigDecimal(number.doubleValue());
+        } else if (number instanceof BigDecimal) {
+            bigDec = (BigDecimal) number;
+        }
+        if (bigDec != null) {
+            try {
+                return bigDec.longValueExact();
+            } catch (ArithmeticException e) {
+                throw new IllegalArgumentException(String.format("%s  : can not be store in a long", bigDec));
+            }
+        }
 
         // handle UNSIGNED
         if (number instanceof ULong) {
             long longValue = number.longValue();
             // if long value is negative this means that this is a too long unsigned long
             if (longValue < 0) {
-                throw new IllegalStateException(String.format("%s  : can not be store in a long", number));
+                throw new IllegalArgumentException(String.format("%s  : can not be store in a long", number));
             }
             return longValue;
         }
-        throw new IllegalStateException(String.format("Can not convert %s to long safely : Unsupported number %s",
+        throw new IllegalArgumentException(String.format("Can not convert %s to long safely : Unsupported number %s",
                 number, number.getClass().getCanonicalName()));
     }
 
@@ -68,12 +81,12 @@ public class NumberUtil {
      * 
      * @throws IllegalArgumentException if the number can not be store in a long.
      */
-    public static ULong numberToULong(Number number) throws IllegalStateException {
+    public static ULong numberToULong(Number number) throws IllegalArgumentException {
         // handle INTEGER
         if (number instanceof Byte || number instanceof Short || number instanceof Integer || number instanceof Long) {
             long longValue = number.longValue();
             if (longValue < 0) {
-                throw new IllegalStateException(
+                throw new IllegalArgumentException(
                         String.format("%s  : can not convert negative number to an unsigned long", number));
             }
             return ULong.valueOf(longValue);
@@ -82,19 +95,40 @@ public class NumberUtil {
             // check big integer is not too big for a long
             BigInteger bigInt = (BigInteger) number;
             if (bigInt.signum() == -1 || bigInt.compareTo(ULong.MAX_VALUE) > 0) {
-                throw new IllegalStateException(String.format("%s  : can not be store in an unsigned long", bigInt));
+                throw new IllegalArgumentException(String.format("%s  : can not be store in an unsigned long", bigInt));
             }
             return ULong.valueOf(bigInt);
         }
 
         // handle FLOATING-POINT
-        // TODO should we support a safe floating-point conversion
+        BigDecimal bigDec = null;
+        if (number instanceof Float || number instanceof Double) {
+            bigDec = new BigDecimal(number.doubleValue());
+        } else if (number instanceof BigDecimal) {
+            bigDec = (BigDecimal) number;
+        }
+        if (bigDec != null) {
+            if (bigDec.signum() == -1) {
+                throw new IllegalArgumentException(String.format("%s  : can not be store in an unsigned long", bigDec));
+            } else {
+                try {
+                    BigInteger bigInt = bigDec.toBigIntegerExact();
+                    if (bigInt.compareTo(ULong.MAX_VALUE) > 0) {
+                        throw new IllegalArgumentException(
+                                String.format("%s  : can not be store in an unsigned long", bigInt));
+                    }
+                    return ULong.valueOf(bigInt);
+                } catch (ArithmeticException e) {
+                    throw new IllegalArgumentException(String.format("%s  : can not be store in a long", bigDec));
+                }
+            }
+        }
 
         // handle UNSIGNED
         if (number instanceof ULong) {
             return (ULong) number;
         }
-        throw new IllegalStateException(String.format("Can not convert %s to long safely : Unsupported number %s",
+        throw new IllegalArgumentException(String.format("Can not convert %s to long safely : Unsupported number %s",
                 number, number.getClass().getCanonicalName()));
     }
 


Test patch:
diff --git a/leshan-core/src/test/java/org/eclipse/leshan/core/datatype/NumberUtilTest.java b/leshan-core/src/test/java/org/eclipse/leshan/core/datatype/NumberUtilTest.java
index 73a1809e..6a823660 100644
--- a/leshan-core/src/test/java/org/eclipse/leshan/core/datatype/NumberUtilTest.java
+++ b/leshan-core/src/test/java/org/eclipse/leshan/core/datatype/NumberUtilTest.java
@@ -18,6 +18,7 @@ package org.eclipse.leshan.core.datatype;
 import static org.eclipse.leshan.core.util.datatype.NumberUtil.*;
 import static org.junit.Assert.assertEquals;
 
+import java.math.BigDecimal;
 import java.math.BigInteger;
 
 import org.eclipse.leshan.core.util.datatype.ULong;
@@ -44,23 +45,78 @@ public class NumberUtilTest {
 
         assertEquals(Long.valueOf(0l), numberToLong(ULong.valueOf("0")));
         assertEquals(Long.valueOf(9223372036854775807l), numberToLong(ULong.valueOf("9223372036854775807")));
+
+        // floating point
+        assertEquals(Long.valueOf(-9223371487098961920l), numberToLong(Float.valueOf("-9223371487098961920")));
+        assertEquals(Long.valueOf(9223371487098961920l), numberToLong(Float.valueOf("9223371487098961920")));
+
+        assertEquals(Long.valueOf(-9223372036854775808l), numberToLong(Double.valueOf("-9223372036854775808")));
+        assertEquals(Long.valueOf(9223372036854774784l), numberToLong(Double.valueOf("9223372036854774800")));
+
+        assertEquals(Long.valueOf(-9223372036854775808l), numberToLong(new BigDecimal("-9223372036854775808")));
+        assertEquals(Long.valueOf(9223372036854775807l), numberToLong(new BigDecimal("9223372036854775807")));
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void biginteger_too_small_for_long() {
         numberToLong(new BigInteger("-9223372036854775809"));
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void biginteger_too_big_for_long() {
         numberToLong(new BigInteger("9223372036854775808"));
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void ulong_too_big_for_long() {
         numberToLong(ULong.valueOf("9223372036854775808"));
     }
 
+    @Test(expected = IllegalArgumentException.class)
+    public void float_too_small_for_long() {
+        numberToLong(Float.valueOf("-9223373136366403584"));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void float_too_big_for_long() {
+        numberToLong(Float.valueOf("9223372036854775808"));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void float_with_decimal_for_long() {
+        numberToLong(Float.valueOf(30.50f));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void double_too_small_for_long() {
+        numberToLong(Double.valueOf("-9223373136366403584"));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void double_too_big_for_long() {
+        numberToLong(Double.valueOf("9223372036854775808"));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void double_with_decimal() {
+        numberToLong(Double.valueOf(30.50d));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void bigdecimal_too_small_for_long() {
+        numberToLong(new BigDecimal("-9223372036854775809"));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void bigdecimal_too_big_for_long() {
+        numberToLong(new BigDecimal("9223372036854775808"));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void bigdecimal_with_decimal_for_long() {
+        numberToLong(new BigDecimal(30.50f));
+    }
+
     @Test
     public void convert_number_to_ulong() {
         assertEquals(ULong.valueOf("0"), numberToULong(Byte.valueOf("0")));
@@ -80,35 +136,80 @@ public class NumberUtilTest {
 
         assertEquals(ULong.valueOf("0"), numberToULong(ULong.valueOf("0")));
         assertEquals(ULong.valueOf("18446744073709551615"), numberToULong(ULong.valueOf("18446744073709551615")));
+
+        // floating point
+        assertEquals(ULong.valueOf("0"), numberToULong(Float.valueOf("0")));
+        assertEquals(ULong.valueOf("18446742974197923840"), numberToULong(Float.valueOf("18446742974197923840")));
+
+        assertEquals(ULong.valueOf("0"), numberToULong(Double.valueOf("0")));
+        assertEquals(ULong.valueOf("18446742974197923840"), numberToULong(Double.valueOf("18446742974197923840")));
+
+        assertEquals(ULong.valueOf("0"), numberToULong(new BigDecimal("0")));
+        assertEquals(ULong.valueOf("18446744073709551615"), numberToULong(new BigDecimal("18446744073709551615")));
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void byte_negative_for_ulong() {
         numberToULong(Byte.valueOf("-1"));
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void short_negative_for_ulong() {
         numberToULong(Short.valueOf("-1"));
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void integer_negative_for_ulong() {
         numberToULong(Integer.valueOf("-1"));
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void long_negative_for_ulong() {
         numberToULong(Long.valueOf("-1"));
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void biginteger_negative_for_ulong() {
         numberToULong(new BigInteger("-1"));
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
+    public void float_negative_for_ulong() {
+        numberToULong(Float.valueOf("-1"));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void double_negative_for_ulong() {
+        numberToULong(Double.valueOf("-1"));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void bigdecimal_negative_for_ulong() {
+        numberToULong(new BigDecimal("-1"));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void float_with_decimal_for_ulong() {
+        numberToULong(Float.valueOf(30.50f));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
     public void biginteger_to_big_for_ulong() {
-        numberToLong(new BigInteger("18446744073709551616"));
+        numberToULong(new BigInteger("18446744073709551616"));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void float_too_big_for_ulong() {
+        numberToULong(Float.valueOf("18446744073709551616"));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void double_too_big_for_ulong() {
+        numberToULong(Double.valueOf("18446744073709551616"));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void bigdecimal_to_big_for_ulong() {
+        numberToULong(new BigDecimal("18446744073709551616"));
     }
 }
diff --git a/leshan-core/src/test/java/org/eclipse/leshan/core/node/codec/LwM2mNodeDecoderTest.java b/leshan-core/src/test/java/org/eclipse/leshan/core/node/codec/LwM2mNodeDecoderTest.java
index 3dde2e06..11ed0a16 100644
--- a/leshan-core/src/test/java/org/eclipse/leshan/core/node/codec/LwM2mNodeDecoderTest.java
+++ b/leshan-core/src/test/java/org/eclipse/leshan/core/node/codec/LwM2mNodeDecoderTest.java
@@ -909,6 +909,18 @@ public class LwM2mNodeDecoderTest {
         assertEquals(20.0, resource.getValue());
     }
 
+    @Test
+    public void senml_json_decode_single_resource_using_exponantial_notation() {
+        String payload = "[{\"bn\":\"/3/0/13\",\"v\":1.638435E9}]";
+        LwM2mResource resource = decoder.decode(payload.getBytes(), ContentFormat.SENML_JSON, new LwM2mPath("/3/0/13"),
+                model, LwM2mResource.class);
+
+        assertNotNull(resource);
+        assertTrue(!resource.isMultiInstances());
+        assertEquals(13, resource.getId());
+        assertEquals(new Date(1638435000000l), resource.getValue());
+    }
+
     @Test
     public void senml_json_decode_multiple_resource() {
         StringBuilder payload = new StringBuilder();