Procházet zdrojové kódy

fix(lwm2m): better logging for unknown object IDs

Prior to this change, unknown object IDs would result in a tuple
{error, no_xml_definition}, and this tuple is passed down to xmerl
lib to get XML node data and eventually crash with function_clause.

With this fix, the unknown object ID exception is caught and logged
properly.
Zaiming (Stone) Shi před 3 roky
rodič
revize
13f5c0b6b1

+ 1 - 0
CHANGES-5.0.md

@@ -9,6 +9,7 @@
 * The license is now copied to all nodes in the cluster when it's reloaded. [#8598](https://github.com/emqx/emqx/pull/8598)
 * Added a HTTP API to manage licenses. [#8610](https://github.com/emqx/emqx/pull/8610)
 * Updated `/nodes` API node_status from `Running/Stopped` to `running/stopped`. [#8642](https://github.com/emqx/emqx/pull/8642)
+* Better logging on unknown object IDs. [#8670](https://github.com/emqx/emqx/pull/8670)
 
 # 5.0.4
 

+ 8 - 6
apps/emqx_gateway/src/lwm2m/emqx_lwm2m_cmd.erl

@@ -221,14 +221,16 @@ read_resp_to_mqtt({ok, SuccessCode}, CoapPayload, Format, Ref) ->
         Result = content_to_mqtt(CoapPayload, Format, Ref),
         make_response(SuccessCode, Ref, Format, Result)
     catch
-        error:not_implemented ->
-            make_response(not_implemented, Ref);
-        _:Ex:_ST ->
+        throw:{bad_request, Reason} ->
+            ?SLOG(error, #{msg => "bad_request", payload => CoapPayload, reason => Reason}),
+            make_response(bad_request, Ref);
+        E:R:ST ->
             ?SLOG(error, #{
-                msg => "bad_payload_format",
+                msg => "bad_request",
                 payload => CoapPayload,
-                reason => Ex,
-                stacktrace => _ST
+                exception => E,
+                reason => R,
+                stacktrace => ST
             }),
             make_response(bad_request, Ref)
     end.

+ 2 - 2
apps/emqx_gateway/src/lwm2m/emqx_lwm2m_message.erl

@@ -29,7 +29,7 @@
 tlv_to_json(BaseName, TlvData) ->
     DecodedTlv = emqx_lwm2m_tlv:parse(TlvData),
     ObjectId = object_id(BaseName),
-    ObjDefinition = emqx_lwm2m_xml_object:get_obj_def(ObjectId, true),
+    ObjDefinition = emqx_lwm2m_xml_object:get_obj_def_assertive(ObjectId, true),
     case DecodedTlv of
         [#{tlv_resource_with_value := Id, value := Value}] ->
             TrueBaseName = basename(BaseName, undefined, undefined, Id, 3),
@@ -318,7 +318,7 @@ path([H | T], Acc) ->
 
 text_to_json(BaseName, Text) ->
     {ObjectId, ResourceId} = object_resource_id(BaseName),
-    ObjDefinition = emqx_lwm2m_xml_object:get_obj_def(ObjectId, true),
+    ObjDefinition = emqx_lwm2m_xml_object:get_obj_def_assertive(ObjectId, true),
     Val = text_value(Text, ResourceId, ObjDefinition),
     [#{path => BaseName, value => Val}].
 

+ 8 - 1
apps/emqx_gateway/src/lwm2m/emqx_lwm2m_xml_object.erl

@@ -21,6 +21,7 @@
 
 -export([
     get_obj_def/2,
+    get_obj_def_assertive/2,
     get_object_id/1,
     get_object_name/1,
     get_object_and_resource_id/2,
@@ -29,7 +30,13 @@
     get_resource_operations/2
 ]).
 
-% This module is for future use. Disabled now.
+get_obj_def_assertive(ObjectId, IsInt) ->
+    case get_obj_def(ObjectId, IsInt) of
+        {error, no_xml_definition} ->
+            erlang:throw({bad_request, {unknown_object_id, ObjectId}});
+        Xml ->
+            Xml
+    end.
 
 get_obj_def(ObjectIdInt, true) ->
     emqx_lwm2m_xml_object_db:find_objectid(ObjectIdInt);

+ 2 - 5
apps/emqx_gateway/src/lwm2m/emqx_lwm2m_xml_object_db.erl

@@ -76,12 +76,9 @@ find_name(Name) ->
         end,
     case ets:lookup(?LWM2M_OBJECT_NAME_TO_ID_TAB, NameBinary) of
         [] ->
-            undefined;
+            {error, no_xml_definition};
         [{NameBinary, ObjectId}] ->
-            case ets:lookup(?LWM2M_OBJECT_DEF_TAB, ObjectId) of
-                [] -> undefined;
-                [{ObjectId, Xml}] -> Xml
-            end
+            find_objectid(ObjectId)
     end.
 
 stop() ->

+ 69 - 0
apps/emqx_gateway/test/emqx_lwm2m_SUITE.erl

@@ -850,6 +850,75 @@ case10_read(Config) ->
     ),
     ?assertEqual(ReadResult, test_recv_mqtt_response(RespTopic)).
 
+case10_read_bad_request(Config) ->
+    UdpSock = ?config(sock, Config),
+    Epn = "urn:oma:lwm2m:oma:3",
+    MsgId1 = 15,
+    RespTopic = list_to_binary("lwm2m/" ++ Epn ++ "/up/resp"),
+    emqtt:subscribe(?config(emqx_c, Config), RespTopic, qos0),
+    timer:sleep(200),
+    % step 1, device register ...
+    test_send_coap_request(
+        UdpSock,
+        post,
+        sprintf("coap://127.0.0.1:~b/rd?ep=~s&lt=345&lwm2m=1", [?PORT, Epn]),
+        #coap_content{
+            content_format = <<"text/plain">>,
+            payload =
+                <<"</lwm2m>;rt=\"oma.lwm2m\";ct=11543,</lwm2m/1/0>,</lwm2m/2/0>,</lwm2m/3/0>">>
+        },
+        [],
+        MsgId1
+    ),
+    #coap_message{method = Method1} = test_recv_coap_response(UdpSock),
+    ?assertEqual({ok, created}, Method1),
+    test_recv_mqtt_response(RespTopic),
+
+    % step2,  send a READ command to device
+    CmdId = 206,
+    CommandTopic = <<"lwm2m/", (list_to_binary(Epn))/binary, "/dn/dm">>,
+    Command = #{
+        <<"requestID">> => CmdId,
+        <<"cacheID">> => CmdId,
+        <<"msgType">> => <<"read">>,
+        <<"data">> => #{
+            <<"path">> => <<"/3333/0/0">>
+        }
+    },
+    CommandJson = emqx_json:encode(Command),
+    ?LOGT("CommandJson=~p", [CommandJson]),
+    test_mqtt_broker:publish(CommandTopic, CommandJson, 0),
+    timer:sleep(50),
+    Request2 = test_recv_coap_request(UdpSock),
+    #coap_message{method = Method2, payload = Payload2} = Request2,
+    ?LOGT("LwM2M client got ~p", [Request2]),
+    ?assertEqual(get, Method2),
+    ?assertEqual(<<>>, Payload2),
+    timer:sleep(50),
+
+    test_send_coap_response(
+        UdpSock,
+        "127.0.0.1",
+        ?PORT,
+        {ok, content},
+        #coap_content{content_format = <<"text/plain">>, payload = <<"EMQ">>},
+        Request2,
+        true
+    ),
+    timer:sleep(100),
+
+    ReadResult = emqx_json:encode(#{
+        <<"requestID">> => CmdId,
+        <<"cacheID">> => CmdId,
+        <<"msgType">> => <<"read">>,
+        <<"data">> => #{
+            <<"code">> => <<"4.00">>,
+            <<"codeMsg">> => <<"bad_request">>,
+            <<"reqPath">> => <<"/3333/0/0">>
+        }
+    }),
+    ?assertEqual(ReadResult, test_recv_mqtt_response(RespTopic)).
+
 case10_read_separate_ack(Config) ->
     UdpSock = ?config(sock, Config),
     Epn = "urn:oma:lwm2m:oma:3",