From 9fd8e930be9bcf5df8446c2e086a1a37143dee87 Mon Sep 17 00:00:00 2001
From: Kjell Winblad <kjellwinblad@gmail.com>
Date: Thu, 16 May 2024 11:19:48 +0200
Subject: [PATCH] fix(best_effort_json): only do tuple list to map without
 losing pairs

---
 apps/emqx/src/emqx_logger_jsonfmt.erl         | 11 +++++++
 .../test/emqx_mgmt_api_trace_SUITE.erl        | 29 +++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/apps/emqx/src/emqx_logger_jsonfmt.erl b/apps/emqx/src/emqx_logger_jsonfmt.erl
index 776c8f753..8c760f96c 100644
--- a/apps/emqx/src/emqx_logger_jsonfmt.erl
+++ b/apps/emqx/src/emqx_logger_jsonfmt.erl
@@ -219,6 +219,8 @@ best_effort_unicode(Input, Config) ->
 
 best_effort_json_obj(List, Config) when is_list(List) ->
     try
+        %% We should only do this if there are no duplicated keys
+        check_no_dup_tuple_list(List),
         json_obj(maps:from_list(List), Config)
     catch
         _:_ ->
@@ -232,6 +234,15 @@ best_effort_json_obj(Map, Config) ->
             do_format_msg("~p", [Map], Config)
     end.
 
+check_no_dup_tuple_list(List) ->
+    %% Crash if this is not a tuple list
+    lists:foreach(fun({_, _}) -> ok end, List),
+    Items = [K || {K, _} <- List],
+    NumberOfItems = length(Items),
+    %% Crash if there are duplicates
+    NumberOfItems = maps:size(maps:from_keys(Items, true)),
+    ok.
+
 json(A, _) when is_atom(A) -> A;
 json(I, _) when is_integer(I) -> I;
 json(F, _) when is_float(F) -> F;
diff --git a/apps/emqx_management/test/emqx_mgmt_api_trace_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_trace_SUITE.erl
index 2a3e1d18a..ce29a67f6 100644
--- a/apps/emqx_management/test/emqx_mgmt_api_trace_SUITE.erl
+++ b/apps/emqx_management/test/emqx_mgmt_api_trace_SUITE.erl
@@ -271,6 +271,16 @@ t_http_test_json_formatter(_Config) ->
     }),
     %% We should handle report style logging
     ?SLOG(error, #{msg => "recursive_republish_detected"}, #{topic => Topic}),
+    ?TRACE("CUSTOM", "my_log_msg", #{
+        topic => Topic,
+        %% This will be converted to map
+        map_key => [{a, a}, {b, b}]
+    }),
+    ?TRACE("CUSTOM", "my_log_msg", #{
+        topic => Topic,
+        %% We should not convert this to a map as we will lose information
+        map_key => [{a, a}, {a, b}]
+    }),
     ok = emqx_trace_handler_SUITE:filesync(Name, topic),
     {ok, _Detail2} = request_api(get, api_path("trace/" ++ binary_to_list(Name) ++ "/log_detail")),
     {ok, Bin} = request_api(get, api_path("trace/" ++ binary_to_list(Name) ++ "/download")),
@@ -425,6 +435,25 @@ t_http_test_json_formatter(_Config) ->
         },
         NextFun()
     ),
+    ?assertMatch(
+        #{
+            <<"meta">> := #{
+                <<"map_key">> := #{
+                    <<"a">> := <<"a">>,
+                    <<"b">> := <<"b">>
+                }
+            }
+        },
+        NextFun()
+    ),
+    ?assertMatch(
+        #{
+            <<"meta">> := #{
+                <<"map_key">> := [_, _]
+            }
+        },
+        NextFun()
+    ),
     {ok, Delete} = request_api(delete, api_path("trace/" ++ binary_to_list(Name))),
     ?assertEqual(<<>>, Delete),