diff --git a/apps/emqx/src/emqx_frame.erl b/apps/emqx/src/emqx_frame.erl index a86bb0155..10f0aa135 100644 --- a/apps/emqx/src/emqx_frame.erl +++ b/apps/emqx/src/emqx_frame.erl @@ -288,11 +288,15 @@ parse_connect(FrameBin, StrictMode) -> parse_connect2( ProtoName, <>, + CleanStart:1, Reserved:1, KeepAlive:16/big, Rest2/binary>>, StrictMode ) -> + case Reserved of + 0 -> ok; + 1 -> ?PARSE_ERR(reserved_connect_flag) + end, {Properties, Rest3} = parse_properties(Rest2, ProtoVer, StrictMode), - {ClientId, Rest4} = parse_utf8_string_with_hint(Rest3, StrictMode, invalid_username), + {ClientId, Rest4} = parse_utf8_string_with_hint(Rest3, StrictMode, invalid_clientid), ConnPacket = #mqtt_packet_connect{ proto_name = ProtoName, proto_ver = ProtoVer, @@ -324,7 +328,7 @@ parse_connect2( <<>> -> ConnPacket1#mqtt_packet_connect{username = Username, password = Password}; _ -> - ?PARSE_ERR(malformed_connect_payload) + ?PARSE_ERR(malformed_connect_data) end; parse_connect2(_ProtoName, _, _) -> ?PARSE_ERR(malformed_connect_header). @@ -340,6 +344,7 @@ parse_packet( <>, #{version := Ver, strict_mode := StrictMode} ) -> + %% Not possible for broker to receive! case parse_properties(Rest, Ver, StrictMode) of {Properties, <<>>} -> #mqtt_packet_connack{ diff --git a/apps/emqx/test/emqx_frame_SUITE.erl b/apps/emqx/test/emqx_frame_SUITE.erl index f6296359f..158318b07 100644 --- a/apps/emqx/test/emqx_frame_SUITE.erl +++ b/apps/emqx/test/emqx_frame_SUITE.erl @@ -57,7 +57,13 @@ groups() -> t_serialize_parse_v5_connect, t_serialize_parse_connect_without_clientid, t_serialize_parse_connect_with_will, - t_serialize_parse_bridge_connect + t_serialize_parse_bridge_connect, + t_parse_invalid_remaining_len, + t_parse_malformed_properties, + t_malformed_connect_header, + t_malformed_connect_payload, + t_reserved_connect_flag, + t_invalid_clientid ]}, {connack, [parallel], [ t_serialize_parse_connack, @@ -636,18 +642,32 @@ t_parse_malformed_properties(_) -> emqx_frame:parse(<<2:4, 0:4, 3:8, 1:8, 0:8, 0:8>>) ). -t_parse_malformed_connect(_) -> +t_malformed_connect_header(_) -> ?assertException( throw, {frame_parse_error, malformed_connect_header}, - emqx_frame:parse(<<16, 11, 0, 6, 77, 81, 73, 115, 110, 112, 3, 130, 1, 6>>) - ), + 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_payload}, - emqx_frame:parse( - <<16, 21, 0, 6, 77, 81, 73, 115, 110, 112, 3, 130, 1, 6, 0, 0, 2, 67, 49.49>> - ) + {frame_parse_error, malformed_connect_data}, + emqx_frame:parse(<<16, 15, 0, 6, 77, 81, 73, 115, 100, 112, 3, 0, 0, 0, 0, 0, 0>>) + ). + +t_reserved_connect_flag(_) -> + ?assertException( + throw, + {frame_parse_error, reserved_connect_flag}, + emqx_frame:parse(<<16, 15, 0, 6, 77, 81, 73, 115, 100, 112, 3, 1, 0, 0, 1, 0, 0>>) + ). + +t_invalid_clientid(_) -> + ?assertException( + throw, + {frame_parse_error, #{hint := invalid_clientid}}, + emqx_frame:parse(<<16, 15, 0, 6, 77, 81, 73, 115, 100, 112, 3, 0, 0, 0, 1, 0, 0>>) ). parse_serialize(Packet) ->