From 0f5282487287715e2c7d3397d6413a27ae15724d Mon Sep 17 00:00:00 2001
From: Zaiming Shi <zmstone@gmail.com>
Date: Mon, 22 Nov 2021 16:49:58 +0100
Subject: [PATCH] refactor(trace): hash non-printable or too long names

---
 src/emqx_trace_handler.erl        | 42 ++++++++++++++---------------
 test/emqx_trace_handler_tests.erl | 44 +++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 21 deletions(-)
 create mode 100644 test/emqx_trace_handler_tests.erl

diff --git a/src/emqx_trace_handler.erl b/src/emqx_trace_handler.erl
index 7006fc815..328d734f0 100644
--- a/src/emqx_trace_handler.erl
+++ b/src/emqx_trace_handler.erl
@@ -160,22 +160,16 @@ filter_ip_address(#{meta := #{peername := Peername}} = Log, {IP, _Name}) ->
 filter_ip_address(_Log, _ExpectId) -> ignore.
 
 install_handler(Who = #{name := Name, type := Type}, Level, LogFile) ->
-    case handler_id(Name, Type) of
-        {ok, HandlerId} ->
-            Config = #{
-                level => Level,
+    HandlerId = handler_id(Name, Type),
+    Config = #{ level => Level,
                 formatter => ?FORMAT,
                 filter_default => stop,
                 filters => filters(Who),
                 config => ?CONFIG(LogFile)
-            },
-            Res = logger:add_handler(HandlerId, logger_disk_log_h, Config),
-            show_prompts(Res, Who, "Start trace"),
-            Res;
-        {error, _Reason} = Error ->
-            show_prompts(Error, Who, "Start trace"),
-            Error
-    end.
+                },
+    Res = logger:add_handler(HandlerId, logger_disk_log_h, Config),
+    show_prompts(Res, Who, "Start trace"),
+    Res.
 
 filters(#{type := clientid, filter := Filter, name := Name}) ->
     [{clientid, {fun ?MODULE:filter_clientid/2, {ensure_list(Filter), Name}}}];
@@ -197,17 +191,23 @@ filter_traces(#{id := Id, level := Level, dst := Dst, filters := Filters}, Acc)
     end.
 
 handler_id(Name, Type) ->
-    TypeBin = atom_to_binary(Type),
-    case io_lib:printable_unicode_list(binary_to_list(Name)) of
-        true when byte_size(Name) < 256 ->
-            NameHash = base64:encode(crypto:hash(sha, Name)),
-            {ok, binary_to_atom(<<"trace_", TypeBin/binary, "_", NameHash/binary>>)};
-        true ->
-            {error, <<"Name length must < 256">>};
-        false ->
-            {error, <<"Name must printable unicode">>}
+    try
+        do_handler_id(Name, Type)
+    catch
+        _ : _ ->
+            Hash = emqx_misc:bin2hexstr_a_f_lower(crypto:hash(md5, Name)),
+            do_handler_id(Hash, Type)
     end.
 
+%% Handler ID must be an atom.
+do_handler_id(Name, Type) ->
+    TypeStr = atom_to_list(Type),
+    NameStr = unicode:characters_to_list(Name, utf8),
+    FullNameStr = "trace_" ++ TypeStr ++ "_" ++  NameStr,
+    true = io_lib:printable_unicode_list(FullNameStr),
+    FullNameBin = unicode:characters_to_binary(FullNameStr, utf8),
+    binary_to_atom(FullNameBin, utf8).
+
 ensure_bin(List) when is_list(List) -> iolist_to_binary(List);
 ensure_bin(Bin) when is_binary(Bin) -> Bin.
 
diff --git a/test/emqx_trace_handler_tests.erl b/test/emqx_trace_handler_tests.erl
new file mode 100644
index 000000000..40586810e
--- /dev/null
+++ b/test/emqx_trace_handler_tests.erl
@@ -0,0 +1,44 @@
+%%--------------------------------------------------------------------
+%% Copyright (c) 2021 EMQ Technologies Co., Ltd. All Rights Reserved.
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%%--------------------------------------------------------------------
+
+-module(emqx_trace_handler_tests).
+
+-include_lib("eunit/include/eunit.hrl").
+
+handler_id_test_() ->
+    [{"normal_printable",
+     fun() ->
+             ?assertEqual(trace_topic_t1, emqx_trace_handler:handler_id("t1", topic))
+     end},
+     {"normal_unicode",
+      fun() ->
+             ?assertEqual('trace_topic_主题', emqx_trace_handler:handler_id("主题", topic))
+      end
+     },
+     {"not_printable",
+      fun() ->
+             ?assertEqual('trace_topic_93b885adfe0da089cdf634904fd59f71',
+                          emqx_trace_handler:handler_id("\0", topic))
+      end
+     },
+     {"too_long",
+      fun() ->
+            T = lists:duplicate(250, $a),
+            ?assertEqual('trace_topic_1bdbdf1c9087c796394bcda5789f7206',
+                         emqx_trace_handler:handler_id(T, topic))
+      end
+     }
+    ].