test: for better test coverage

apps/emqx/src/emqx_frame.erl

@@ -330,10 +330,14 @@ parse_connect2(
         <<>> ->
             ConnPacket1#mqtt_packet_connect{username = Username, password = Password};
         _ ->
-            ?PARSE_ERR(malformed_connect_data)
+            ?PARSE_ERR(#{
+                hint => malformed_connect,
+                unexpected_trailing_bytes => size(Rest7)
+            })
     end;
-parse_connect2(_ProtoName, _, _) ->
-    ?PARSE_ERR(malformed_connect_header).
+parse_connect2(_ProtoName, Bin, _StrictMode) ->
+    %% sent less than 32 bytes
+    ?PARSE_ERR(#{hint => malformed_connect, header_bytes => Bin}).
 
 parse_packet(
     #mqtt_packet_header{type = ?CONNECT},
@@ -486,7 +490,7 @@ parse_will_message(
 ) ->
     {Props, Rest} = parse_properties(Bin, Ver, StrictMode),
     {Topic, Rest1} = parse_utf8_string_with_hint(Rest, StrictMode, invalid_topic),
-    {Payload, Rest2} = parse_binary_data(Rest1),
+    {Payload, Rest2} = parse_will_payload(Rest1),
     {
         Packet#mqtt_packet_connect{
             will_props = Props,
@@ -687,20 +691,24 @@ parse_utf8_string(Bin, _) when
 ->
     ?PARSE_ERR(#{reason => malformed_utf8_string_length}).
 
-parse_binary_data(<<Len:16/big, Data:Len/binary, Rest/binary>>) ->
+parse_will_payload(<<Len:16/big, Data:Len/binary, Rest/binary>>) ->
     {Data, Rest};
-parse_binary_data(<<Len:16/big, Rest/binary>>) when
+parse_will_payload(<<Len:16/big, Rest/binary>>) when
     Len > byte_size(Rest)
 ->
     ?PARSE_ERR(#{
-        hint => malformed_binary_data,
+        hint => malformed_will_payload,
         parsed_length => Len,
-        remaining_bytes_length => byte_size(Rest)
+        remaining_bytes => byte_size(Rest)
     });
-parse_binary_data(Bin) when
+parse_will_payload(Bin) when
     2 > byte_size(Bin)
 ->
-    ?PARSE_ERR(malformed_binary_data_length).
+    ?PARSE_ERR(#{
+        hint => malformed_will_payload,
+        length_bytes => size(Bin),
+        expected_bytes => 2
+    }).
 
 %%--------------------------------------------------------------------
 %% Serialize MQTT Packet

apps/emqx/test/emqx_frame_SUITE.erl

@@ -57,11 +57,12 @@ groups() ->
             t_serialize_parse_v5_connect,
             t_serialize_parse_connect_without_clientid,
             t_serialize_parse_connect_with_will,
+            t_serialize_parse_connect_with_malformed_will,
             t_serialize_parse_bridge_connect,
             t_parse_invalid_remaining_len,
             t_parse_malformed_properties,
             t_malformed_connect_header,
-            t_malformed_connect_payload,
+            t_malformed_connect_data,
             t_reserved_connect_flag,
             t_invalid_clientid
         ]},
@@ -277,6 +278,37 @@ t_serialize_parse_connect_with_will(_) ->
     ?assertEqual(Bin, serialize_to_binary(Packet)),
     ?assertMatch({ok, Packet, <<>>, _}, emqx_frame:parse(Bin)).
 
+t_serialize_parse_connect_with_malformed_will(_) ->
+    Packet2 = #mqtt_packet{
+        header = #mqtt_packet_header{type = ?CONNECT},
+        variable = #mqtt_packet_connect{
+            proto_ver = ?MQTT_PROTO_V3,
+            proto_name = <<"MQIsdp">>,
+            clientid = <<"mosqpub/10452-iMac.loca">>,
+            clean_start = true,
+            keepalive = 60,
+            will_retain = false,
+            will_qos = ?QOS_1,
+            will_flag = true,
+            will_topic = <<"/will">>,
+            will_payload = <<>>
+        }
+    },
+    <<16, 46, Body:44/binary, 0, 0>> = serialize_to_binary(Packet2),
+    %% too short
+    BadBin1 = <<16, 45, Body/binary, 0>>,
+    ?ASSERT_FRAME_THROW(
+        #{hint := malformed_will_payload, length_bytes := 1, expected_bytes := 2},
+        emqx_frame:parse(BadBin1)
+    ),
+    %% too long
+    BadBin2 = <<16, 47, Body/binary, 0, 2, 0>>,
+    ?ASSERT_FRAME_THROW(
+        #{hint := malformed_will_payload, parsed_length := 2, remaining_bytes := 1},
+        emqx_frame:parse(BadBin2)
+    ),
+    ok.
+
 t_serialize_parse_bridge_connect(_) ->
     Bin =
         <<16, 86, 0, 6, 77, 81, 73, 115, 100, 112, 131, 44, 0, 60, 0, 19, 67, 95, 48, 48, 58, 48,
@@ -643,16 +675,14 @@ t_parse_malformed_properties(_) ->
     ).
 
 t_malformed_connect_header(_) ->
-    ?assertException(
-        throw,
-        {frame_parse_error, malformed_connect_header},
+    ?ASSERT_FRAME_THROW(
+        #{hint := malformed_connect, header_bytes := _},
         emqx_frame:parse(<<16, 11, 0, 6, 77, 81, 73, 115, 100, 112, 3, 130, 1, 6>>)
     ).
 
-t_malformed_connect_payload(_) ->
-    ?assertException(
-        throw,
-        {frame_parse_error, malformed_connect_data},
+t_malformed_connect_data(_) ->
+    ?ASSERT_FRAME_THROW(
+        #{hint := malformed_connect, unexpected_trailing_bytes := _},
         emqx_frame:parse(<<16, 15, 0, 6, 77, 81, 73, 115, 100, 112, 3, 0, 0, 0, 0, 0, 0>>)
     ).