From d4dd4a124c5c08e728e6c40b89c3ead2eafef034 Mon Sep 17 00:00:00 2001
From: zhongwencool <zhongwencool@gmail.com>
Date: Mon, 6 Dec 2021 18:24:47 +0800
Subject: [PATCH] fix: trace_name format [A-Za-z0-9-_];waiting status if create
 time to closed"

---
 .../src/emqx_trace/emqx_trace.erl             | 15 +++++++------
 .../src/emqx_trace/emqx_trace_api.erl         | 21 ++++++++-----------
 .../test/emqx_trace_SUITE.erl                 |  2 +-
 test/emqx_trace_handler_SUITE.erl             |  1 +
 4 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/apps/emqx_plugin_libs/src/emqx_trace/emqx_trace.erl b/apps/emqx_plugin_libs/src/emqx_trace/emqx_trace.erl
index 135a1f53b..252da8c0f 100644
--- a/apps/emqx_plugin_libs/src/emqx_trace/emqx_trace.erl
+++ b/apps/emqx_plugin_libs/src/emqx_trace/emqx_trace.erl
@@ -398,15 +398,13 @@ fill_default(Trace = #?TRACE{end_at = undefined, start_at = StartAt}) ->
     fill_default(Trace#?TRACE{end_at = StartAt + 10 * 60});
 fill_default(Trace) -> Trace.
 
+-define(NAME_RE, "^[A-Za-z]+[A-Za-z0-9-_]*$").
+
 to_trace([], Rec) -> {ok, Rec};
 to_trace([{name, Name} | Trace], Rec) ->
-    case io_lib:printable_unicode_list(unicode:characters_to_list(Name, utf8)) of
-        true ->
-            case binary:match(Name, [<<"/">>], []) of
-                nomatch -> to_trace(Trace, Rec#?TRACE{name = Name});
-                _ -> {error, "name cannot contain /"}
-            end;
-        false -> {error, "name must printable unicode"}
+    case re:run(Name, ?NAME_RE) of
+        nomatch -> {error, "Name should be " ?NAME_RE};
+        _ -> to_trace(Trace, Rec#?TRACE{name = Name})
     end;
 to_trace([{type, Type} | Trace], Rec) ->
     case lists:member(Type, [<<"clientid">>, <<"topic">>, <<"ip_address">>]) of
@@ -453,7 +451,8 @@ validate_topic(TopicName) ->
 to_system_second(At) ->
     try
         Sec = calendar:rfc3339_to_system_time(binary_to_list(At), [{unit, second}]),
-        {ok, Sec}
+        Now = erlang:system_time(second),
+        {ok, erlang:max(Now, Sec)}
     catch error: {badmatch, _} ->
         {error, ["The rfc3339 specification not satisfied: ", At]}
     end.
diff --git a/apps/emqx_plugin_libs/src/emqx_trace/emqx_trace_api.erl b/apps/emqx_plugin_libs/src/emqx_trace/emqx_trace_api.erl
index d243235a4..d2bca542b 100644
--- a/apps/emqx_plugin_libs/src/emqx_trace/emqx_trace_api.erl
+++ b/apps/emqx_plugin_libs/src/emqx_trace/emqx_trace_api.erl
@@ -135,7 +135,7 @@ stream_log_file(#{name := Name}, Params) ->
         {ok, Node} ->
             Position = binary_to_integer(Position0),
             Bytes = binary_to_integer(Bytes0),
-            case rpc:call(Node, ?MODULE, read_trace_file, [Name, Position, Bytes]) of
+            case rpc:call(Node, ?MODULE, read_trace_file, [Name, Position, Bytes])  of
                 {ok, Bin} ->
                     Meta = #{<<"position">> => Position + byte_size(Bin), <<"bytes">> => Bytes},
                     {ok, #{meta => Meta, items => Bin}};
@@ -143,7 +143,7 @@ stream_log_file(#{name := Name}, Params) ->
                     Meta = #{<<"position">> => Size, <<"bytes">> => Bytes},
                     {ok, #{meta => Meta, items => <<"">>}};
                 {error, Reason} ->
-                    logger:log(error, "read_file_failed by ~p", [{Name, Reason, Position, Bytes}]),
+                    logger:log(error, "read_file_failed by ~p", [{Node, Name, Reason, Position, Bytes}]),
                     {error, Reason};
                 {badrpc, nodedown} ->
                     {error, "BadRpc node down"}
@@ -165,15 +165,12 @@ get_trace_size() ->
 
 %% this is an rpc call for stream_log_file/2
 read_trace_file(Name, Position, Limit) ->
-    TraceDir = emqx_trace:trace_dir(),
-    {ok, AllFiles} = file:list_dir(TraceDir),
-    TracePrefix = "trace_" ++ binary_to_list(Name) ++ "_",
-    Filter = fun(FileName) -> nomatch =/= string:prefix(FileName, TracePrefix) end,
-    case lists:filter(Filter, AllFiles) of
-        [TraceFile] ->
+    case emqx_trace:get_trace_filename(Name) of
+        {error, _} = Error -> Error;
+        {ok, TraceFile} ->
+            TraceDir = emqx_trace:trace_dir(),
             TracePath = filename:join([TraceDir, TraceFile]),
-            read_file(TracePath, Position, Limit);
-        [] -> {error, not_found}
+            read_file(TracePath, Position, Limit)
     end.
 
 read_file(Path, Offset, Bytes) ->
@@ -206,8 +203,8 @@ collect_file_size(Nodes, FileName, AllFiles) ->
         Acc#{Node => Size}
                 end, #{}, Nodes).
 
-%% status(false, _Start, End, Now) when End > Now -> <<"stopped">>;
 status(false, _Start, _End, _Now) -> <<"stopped">>;
-status(true, Start, _End, Now) when Now < Start -> <<"waiting">>;
+%% asynchronously create trace, we should wait 1 seconds
+status(true, Start, _End, Now) when Now < Start + 2 -> <<"waiting">>;
 status(true, _Start, End, Now) when Now >= End -> <<"stopped">>;
 status(true, _Start, _End, _Now) -> <<"running">>.
diff --git a/apps/emqx_plugin_libs/test/emqx_trace_SUITE.erl b/apps/emqx_plugin_libs/test/emqx_trace_SUITE.erl
index 94ab71270..4f33e5b7f 100644
--- a/apps/emqx_plugin_libs/test/emqx_trace_SUITE.erl
+++ b/apps/emqx_plugin_libs/test/emqx_trace_SUITE.erl
@@ -130,7 +130,7 @@ t_create_failed(_Config) ->
     InvalidPackets4 = [{<<"name">>, <<"/test">>}, {<<"clientid">>, <<"t">>},
         {<<"type">>, <<"clientid">>}],
     {error, Reason9} = emqx_trace:create(InvalidPackets4),
-    ?assertEqual(<<"name cannot contain /">>, iolist_to_binary(Reason9)),
+    ?assertEqual(<<"Name should be ^[A-Za-z]+[A-Za-z0-9-_]*$">>, iolist_to_binary(Reason9)),
 
     ?assertEqual({error, "type=[topic,clientid,ip_address] required"},
         emqx_trace:create([{<<"name">>, <<"test-name">>}, {<<"clientid">>, <<"good">>}])),
diff --git a/test/emqx_trace_handler_SUITE.erl b/test/emqx_trace_handler_SUITE.erl
index b42b1a591..cbf455d2d 100644
--- a/test/emqx_trace_handler_SUITE.erl
+++ b/test/emqx_trace_handler_SUITE.erl
@@ -200,6 +200,7 @@ t_trace_ip_address(_Config) ->
 
 
 filesync(Name, Type) ->
+    ct:sleep(50),
     filesync(Name, Type, 3).
 
 %% sometime the handler process is not started yet.