fix(lwm2m): write incorrect integer to device (#5425)

* fix(test): add testcase for write integer values

* fix(lwm2m): write incorrect integer to device

* fix(emqx_lwm2m): refactor the code for getting bits len of a signed int

* chore(emqx_lwm2m): bump version for emqx_lwm2m to 4.3.3

apps/emqx_lwm2m/src/emqx_lwm2m.app.src

@@ -1,6 +1,6 @@
 {application,emqx_lwm2m,
              [{description,"EMQ X LwM2M Gateway"},
-              {vsn, "4.3.2"}, % strict semver, bump manually!
+              {vsn, "4.3.3"}, % strict semver, bump manually!
               {modules,[]},
               {registered,[emqx_lwm2m_sup]},
               {applications,[kernel,stdlib,lwm2m_coap]},

apps/emqx_lwm2m/src/emqx_lwm2m.appup.src

@@ -1,13 +1,19 @@
 %% -*-: erlang -*-
-{VSN,
+{"4.3.3",
   [
     {<<"4.3.[0-1]">>, [
       {restart_application, emqx_lwm2m}
+    ]},
+    {"4.3.2", [
+      {load_module, emqx_lwm2m_message, brutal_purge, soft_purge, []}
     ]}
   ],
   [
     {<<"4.3.[0-1]">>, [
       {restart_application, emqx_lwm2m}
+    ]},
+    {"4.3.2", [
+      {load_module, emqx_lwm2m_message, brutal_purge, soft_purge, []}
     ]}
   ]
 }.

apps/emqx_lwm2m/src/emqx_lwm2m_message.erl

@@ -22,6 +22,9 @@
         , opaque_to_json/2
         , translate_json/1
         ]).
+-ifdef(TEST).
+-export([ bits/1 ]).
+-endif.
 
 -include("emqx_lwm2m.hrl").
 
@@ -364,23 +367,6 @@ encode_number(Int) when is_integer(Int) ->
 encode_number(Float) when is_float(Float) ->
     <<Float:64/float>>.
 
-encode_int(Int) when Int >= 0 ->
-    binary:encode_unsigned(Int);
-encode_int(Int) when Int < 0 ->
-    Size = byte_size_of_signed(-Int) * 8,
-    <<Int:Size/signed>>.
-
-byte_size_of_signed(UInt) ->
-    byte_size_of_signed(UInt, 0).
-
-byte_size_of_signed(UInt, N) ->
-    BitSize = (8*N - 1),
-    Max = (1 bsl BitSize),
-    if
-        UInt =< Max -> N;
-        UInt > Max -> byte_size_of_signed(UInt, N+1)
-    end.
-
 binary_to_number(NumStr) ->
     try
         binary_to_integer(NumStr)
@@ -388,3 +374,26 @@ binary_to_number(NumStr) ->
         error:badarg ->
             binary_to_float(NumStr)
     end.
+
+encode_int(Int) ->
+    Bits = bits(Int),
+    <<Int:Bits/signed>>.
+
+bits(I) when I < 0 -> bits_neg(I);
+bits(I) -> bits_pos(I).
+
+%% Quote:
+%% Integer: An 8, 16, 32 or 64-bit signed integer.
+%% The valid range of the value for a Resource SHOULD be defined.
+%% This data type is also used for the purpose of enumeration.
+%%
+%% NOTE: Integer should not be encoded to 24-bits, 40-bits, etc.
+bits_pos(I) when I < (1 bsl 7)  -> 8;
+bits_pos(I) when I < (1 bsl 15) -> 16;
+bits_pos(I) when I < (1 bsl 31) -> 32;
+bits_pos(I) when I < (1 bsl 63) -> 64.
+
+bits_neg(I) when I >= -((1 bsl 7))  -> 8;
+bits_neg(I) when I >= -((1 bsl 15)) -> 16;
+bits_neg(I) when I >= -((1 bsl 31)) -> 32;
+bits_neg(I) when I >= -((1 bsl 63)) -> 64.

apps/emqx_lwm2m/test/emqx_lwm2m_SUITE.erl

@@ -41,6 +41,7 @@ all() ->
     , {group, test_grp_5_write_attr}
     , {group, test_grp_6_observe}
     , {group, test_grp_8_object_19}
+    , {group, test_grp_bugs}
     ].
 
 suite() -> [{timetrap, {seconds, 90}}].
@@ -106,6 +107,9 @@ groups() ->
         {test_grp_9_psm_queue_mode, [RepeatOpt], [
             case90_psm_mode,
             case90_queue_mode
+        ]},
+        {test_grp_bugs, [RepeatOpt], [
+            case_bug_emqx_4989
         ]}
     ].
 
@@ -145,6 +149,56 @@ end_per_testcase(_AllTestCase, Config) ->
 %% Cases
 %%--------------------------------------------------------------------
 
+case_bug_emqx_4989(Config) ->
+    %% https://github.com/emqx/emqx/issues/4989
+    % step 1, device register ...
+    Epn = "urn:oma:lwm2m:oma:3",
+    MsgId1 = 15,
+    UdpSock = ?config(sock, Config),
+    ObjectList = <<"</1>, </2>, </3/0>, </4>, </5>">>,
+    RespTopic = list_to_binary("lwm2m/"++Epn++"/up/resp"),
+    emqtt:subscribe(?config(emqx_c, Config), RespTopic, qos0),
+    timer:sleep(200),
+
+    std_register(UdpSock, Epn, ObjectList, MsgId1, RespTopic),
+
+    % step2,  send a WRITE command to device
+    CommandTopic = <<"lwm2m/", (list_to_binary(Epn))/binary, "/dn/dm">>,
+    CmdId = 307,
+    Command = #{<<"requestID">> => CmdId, <<"cacheID">> => CmdId,
+                <<"msgType">> => <<"write">>,
+                <<"data">> => #{
+                    <<"path">> => <<"/1/0/2">>,
+                    <<"type">> => <<"Integer">>,
+                    <<"value">> => 129
+                }
+               },
+    CommandJson = emqx_json:encode(Command),
+    test_mqtt_broker:publish(CommandTopic, CommandJson, 0),
+    timer:sleep(50),
+    Request2 = test_recv_coap_request(UdpSock),
+    #coap_message{method = Method2, options=Options2, payload=Payload2} = Request2,
+    Path2 = get_coap_path(Options2),
+    ?assertEqual(put, Method2),
+    ?assertEqual(<<"/1/0/2">>, Path2),
+    ?assertMatch([#{value := 129}], emqx_lwm2m_message:tlv_to_json(Path2, Payload2)),
+
+    timer:sleep(50),
+
+    test_send_coap_response(UdpSock, "127.0.0.1", ?PORT, {ok, changed}, #coap_content{}, Request2, true),
+    timer:sleep(100),
+
+    ReadResult = emqx_json:encode(#{
+                                <<"requestID">> => CmdId, <<"cacheID">> => CmdId,
+                                <<"data">> => #{
+                                    <<"reqPath">> => <<"/1/0/2">>,
+                                    <<"code">> => <<"2.04">>,
+                                    <<"codeMsg">> => <<"changed">>
+                                },
+                                <<"msgType">> => <<"write">>
+                            }),
+    ?assertEqual(ReadResult, test_recv_mqtt_response(RespTopic)).
+
 case01_register(Config) ->
     % ----------------------------------------
     % REGISTER command

apps/emqx_lwm2m/test/emqx_lwm2m_message_tests.erl

@@ -0,0 +1,21 @@
+-module(emqx_lwm2m_message_tests).
+
+-include_lib("eunit/include/eunit.hrl").
+
+-import(emqx_lwm2m_message, [bits/1]).
+
+bits_pos_test() ->
+    ?assertEqual(8, bits(0)),
+    ?assertEqual(8, bits(1)),
+    ?assertEqual(8, bits(127)),
+    ?assertEqual(16, bits(128)),
+    ?assertEqual(16, bits(129)),
+    ok.
+
+bits_neg_test() ->
+    ?assertEqual(8, bits(-1)),
+    ?assertEqual(8, bits(-2)),
+    ?assertEqual(8, bits(-127)),
+    ?assertEqual(8, bits(-128)),
+    ?assertEqual(16, bits(-129)),
+    ok.