From 13f5c0b6b10e6b66c7d2e1a481f9d781eaccbad3 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Tue, 9 Aug 2022 11:22:19 +0200 Subject: [PATCH] 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. --- CHANGES-5.0.md | 1 + .../emqx_gateway/src/lwm2m/emqx_lwm2m_cmd.erl | 14 ++-- .../src/lwm2m/emqx_lwm2m_message.erl | 4 +- .../src/lwm2m/emqx_lwm2m_xml_object.erl | 9 ++- .../src/lwm2m/emqx_lwm2m_xml_object_db.erl | 7 +- apps/emqx_gateway/test/emqx_lwm2m_SUITE.erl | 69 +++++++++++++++++++ 6 files changed, 90 insertions(+), 14 deletions(-) diff --git a/CHANGES-5.0.md b/CHANGES-5.0.md index 59e1cceef..e4ba22f58 100644 --- a/CHANGES-5.0.md +++ b/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 diff --git a/apps/emqx_gateway/src/lwm2m/emqx_lwm2m_cmd.erl b/apps/emqx_gateway/src/lwm2m/emqx_lwm2m_cmd.erl index 46f7534f1..3fd78b383 100644 --- a/apps/emqx_gateway/src/lwm2m/emqx_lwm2m_cmd.erl +++ b/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. diff --git a/apps/emqx_gateway/src/lwm2m/emqx_lwm2m_message.erl b/apps/emqx_gateway/src/lwm2m/emqx_lwm2m_message.erl index 4b13015f5..7e9d418f7 100644 --- a/apps/emqx_gateway/src/lwm2m/emqx_lwm2m_message.erl +++ b/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}]. diff --git a/apps/emqx_gateway/src/lwm2m/emqx_lwm2m_xml_object.erl b/apps/emqx_gateway/src/lwm2m/emqx_lwm2m_xml_object.erl index cb1d0b1d1..53f9f0442 100644 --- a/apps/emqx_gateway/src/lwm2m/emqx_lwm2m_xml_object.erl +++ b/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); diff --git a/apps/emqx_gateway/src/lwm2m/emqx_lwm2m_xml_object_db.erl b/apps/emqx_gateway/src/lwm2m/emqx_lwm2m_xml_object_db.erl index b19e3550b..3e30cc32a 100644 --- a/apps/emqx_gateway/src/lwm2m/emqx_lwm2m_xml_object_db.erl +++ b/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() -> diff --git a/apps/emqx_gateway/test/emqx_lwm2m_SUITE.erl b/apps/emqx_gateway/test/emqx_lwm2m_SUITE.erl index da44beb91..2f5d71a43 100644 --- a/apps/emqx_gateway/test/emqx_lwm2m_SUITE.erl +++ b/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<=345&lwm2m=1", [?PORT, Epn]), + #coap_content{ + content_format = <<"text/plain">>, + payload = + <<";rt=\"oma.lwm2m\";ct=11543,,,">> + }, + [], + 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",