Merge pull request #14223 from zhongwencool/fix-crash-websocket

fix: ensure websocket close reason is atom to avoid crash

apps/emqx/src/emqx_frame.erl

@@ -636,7 +636,7 @@ parse_property(<<16#29, Val, Bin/binary>>, Props, StrictMode) ->
 parse_property(<<16#2A, Val, Bin/binary>>, Props, StrictMode) ->
     parse_property(Bin, Props#{'Shared-Subscription-Available' => Val}, StrictMode);
 parse_property(<<Property:8, _Rest/binary>>, _Props, _StrictMode) ->
-    ?PARSE_ERR(#{invalid_property_code => Property}).
+    ?PARSE_ERR(#{cause => invalid_property_code, property_code => Property}).
 %% TODO: invalid property in specific packet.
 
 parse_variable_byte_integer(Bin) ->

apps/emqx/src/emqx_ws_connection.erl

@@ -1011,6 +1011,10 @@ classify([Cmd = {shutdown, _Reason} | More], Packets, Cmds, Events) ->
     classify(More, Packets, [Cmd | Cmds], Events);
 classify([Cmd = close | More], Packets, Cmds, Events) ->
     classify(More, Packets, [Cmd | Cmds], Events);
+%% cowboy_websocket's close reason must be an atom to avoid crashing the sender process.
+%% The cause reasons come from parse_frame_error.
+classify([{close, #{cause := Cause}} | More], Packets, Cmds, Events) when is_atom(Cause) ->
+    classify(More, Packets, [{close, Cause} | Cmds], Events);
 classify([Cmd = {close, _Reason} | More], Packets, Cmds, Events) ->
     classify(More, Packets, [Cmd | Cmds], Events);
 classify([Event | More], Packets, Cmds, Events) ->

apps/emqx/test/emqx_ws_connection_SUITE.erl

@@ -405,10 +405,19 @@ t_websocket_info_incoming(_) ->
     {[{binary, IoData2}], St2} =
         websocket_info({incoming, ?PACKET(?PINGREQ)}, St1),
     ?assertEqual(<<208, 0>>, iolist_to_binary(IoData2)),
-    %% PUBLISH
-    Publish = ?PUBLISH_PACKET(?QOS_1, <<"t">>, 1, <<"payload">>),
-    {[{binary, IoData3}], _St3} = websocket_info({incoming, Publish}, St2),
-    ?assertEqual(<<64, 4, 0, 1, 0, 0>>, iolist_to_binary(IoData3)).
+    %% PUBLISH with property
+    Publish = ?PUBLISH_PACKET(
+        ?QOS_1, <<"t">>, 1, #{'User-Property' => [{<<"k">>, <<"v">>}]}, <<"payload">>
+    ),
+    {[{binary, IoData3}], St3} = websocket_info({incoming, Publish}, St2),
+    ?assertEqual(<<64, 4, 0, 1, 0, 0>>, iolist_to_binary(IoData3)),
+    %% frame_error
+    Cause = invalid_property_code,
+    FrameError = {frame_error, #{cause => Cause, property_code => 16#2B}},
+    %% cowboy_websocket's close reason must be an atom to avoid crashing the sender process.
+    %% ensure the cause is atom
+    {[{close, CauseReq}], _St4} = websocket_info({incoming, FrameError}, St3),
+    ?assertEqual(Cause, CauseReq).
 
 t_websocket_info_check_gc(_) ->
     Stats = #{cnt => 10, oct => 1000},

changes/ce/fix-14223.en.md

@@ -0,0 +1,4 @@
+ensure websocket close reason is atom to avoid crash.
+```
+error: {{case_clause,#{invalid_property_code => 51}},[{cowboy_websocket...
+```