Merge pull request #7439 from lafirest/fix/empty_topic_5

fix(frame): forbidden empty topic in strict mode

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,14 @@ 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
 %%--------------------------------------------------------------------
@@ -757,9 +765,9 @@ serialize_variable(
 ) ->
     [
         serialize_utf8_string(TopicName),
-        if
+        case PacketId of
-            PacketId =:= undefined -> <<>>;
+            undefined -> <<>>;
-            true -> <<PacketId:16/big-unsigned-integer>>
+            _ -> <<PacketId:16/big-unsigned-integer>>
         end,
         serialize_properties(Properties, Ver)
     ];

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,