From 5febf480a24e97b6f4bb507daa7c2ce2755a8712 Mon Sep 17 00:00:00 2001 From: firest Date: Tue, 29 Mar 2022 10:34:50 +0800 Subject: [PATCH 1/3] fix(frame): forbidden empty topic in strict mode --- apps/emqx/src/emqx_frame.erl | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/apps/emqx/src/emqx_frame.erl b/apps/emqx/src/emqx_frame.erl index 9b34152e7..c7a09f874 100644 --- a/apps/emqx/src/emqx_frame.erl +++ b/apps/emqx/src/emqx_frame.erl @@ -301,7 +301,7 @@ parse_packet( Bin, #{strict_mode := StrictMode, version := Ver} ) -> - {TopicName, Rest} = parse_utf8_string(Bin, StrictMode), + {TopicName, Rest} = parse_topic_name(Bin, StrictMode), {PacketId, Rest1} = case QoS of ?QOS_0 -> {undefined, Rest}; @@ -422,7 +422,7 @@ parse_will_message( StrictMode ) -> {Props, Rest} = parse_properties(Bin, Ver, StrictMode), - {Topic, Rest1} = parse_utf8_string(Rest, StrictMode), + {Topic, Rest1} = parse_topic_name(Rest, StrictMode), {Payload, Rest2} = parse_binary_data(Rest1), { Packet#mqtt_packet_connect{ @@ -621,6 +621,15 @@ parse_binary_data(Bin) when -> ?PARSE_ERR(malformed_binary_data_length). +parse_topic_name(Bin, false) -> + parse_utf8_string(Bin, false); + +parse_topic_name(Bin, true) -> + case parse_utf8_string(Bin, true) of + {<<>>, _Rest} -> ?PARSE_ERR(empty_topic_name); + Result -> Result + end. + %%-------------------------------------------------------------------- %% Serialize MQTT Packet %%-------------------------------------------------------------------- From 978350b6434f224f5d420f51b2c1e292f20df4b5 Mon Sep 17 00:00:00 2001 From: firest Date: Tue, 29 Mar 2022 10:42:22 +0800 Subject: [PATCH 2/3] fix(frame): make elvis && erlfmt happy --- apps/emqx/src/emqx_frame.erl | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/apps/emqx/src/emqx_frame.erl b/apps/emqx/src/emqx_frame.erl index c7a09f874..c55c4d275 100644 --- a/apps/emqx/src/emqx_frame.erl +++ b/apps/emqx/src/emqx_frame.erl @@ -623,7 +623,6 @@ parse_binary_data(Bin) when parse_topic_name(Bin, false) -> parse_utf8_string(Bin, false); - parse_topic_name(Bin, true) -> case parse_utf8_string(Bin, true) of {<<>>, _Rest} -> ?PARSE_ERR(empty_topic_name); @@ -766,9 +765,9 @@ serialize_variable( ) -> [ serialize_utf8_string(TopicName), - if - PacketId =:= undefined -> <<>>; - true -> <> + case PacketId of + undefined -> <<>>; + _ -> <> end, serialize_properties(Properties, Ver) ]; From ed7ce7078a7eab484ff2c90e765acd040c85576c Mon Sep 17 00:00:00 2001 From: firest Date: Tue, 29 Mar 2022 17:06:07 +0800 Subject: [PATCH 3/3] test(frame): add test case for empty topic check --- apps/emqx/test/emqx_frame_SUITE.erl | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apps/emqx/test/emqx_frame_SUITE.erl b/apps/emqx/test/emqx_frame_SUITE.erl index d8d823630..488cf89f6 100644 --- a/apps/emqx/test/emqx_frame_SUITE.erl +++ b/apps/emqx/test/emqx_frame_SUITE.erl @@ -157,6 +157,14 @@ t_parse_malformed_utf8_string(_) -> ParseState = emqx_frame:initial_parse_state(#{strict_mode => true}), ?ASSERT_FRAME_THROW(utf8_string_invalid, emqx_frame:parse(MalformedPacket, ParseState)). +t_parse_empty_topic_name(_) -> + Packet = <<48, 4, 0, 0, 0, 1>>, + NormalState = emqx_frame:initial_parse_state(#{strict_mode => false}), + ?assertMatch({_, _}, emqx_frame:parse(Packet, NormalState)), + + StrictState = emqx_frame:initial_parse_state(#{strict_mode => true}), + ?ASSERT_FRAME_THROW(empty_topic_name, emqx_frame:parse(Packet, StrictState)). + t_serialize_parse_v3_connect(_) -> Bin = <<16, 37, 0, 6, 77, 81, 73, 115, 100, 112, 3, 2, 0, 60, 0, 23, 109, 111, 115, 113, 112, 117,