From 3a09fdc495058bef64335698c1de5184c24a2cad Mon Sep 17 00:00:00 2001
From: JimMoen <LnJimMoen@outlook.com>
Date: Mon, 30 Oct 2023 16:40:56 +0800
Subject: [PATCH] refactor: check match topic before do subscriptions query

---
 .../src/emqx_mgmt_api_subscriptions.erl       | 53 ++++++++++---------
 1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl b/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl
index 39ffb5972..ca0a7a625 100644
--- a/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl
+++ b/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl
@@ -142,21 +142,9 @@ parameters() ->
 
 subscriptions(get, #{query_string := QString}) ->
     Response =
-        try
-            begin
-                case maps:get(<<"match_topic">>, QString, undefined) of
-                    undefined ->
-                        do_subscriptions_query(QString);
-                    MatchTopic ->
-                        case emqx_topic:parse(MatchTopic) of
-                            {#share{}, _} -> {error, invalid_match_topic};
-                            _ -> do_subscriptions_query(QString)
-                        end
-                end
-            end
-        catch
-            error:{invalid_topic_filter, _} ->
-                {error, invalid_match_topic}
+        case check_match_topic(QString) of
+            ok -> do_subscriptions_query(QString);
+            {error, _} = Err -> Err
         end,
     case Response of
         {error, invalid_match_topic} ->
@@ -170,6 +158,31 @@ subscriptions(get, #{query_string := QString}) ->
             {200, Result}
     end.
 
+format(WhichNode, {{Topic, _Subscriber}, SubOpts}) ->
+    maps:merge(
+        #{
+            topic => emqx_topic:maybe_format_share(Topic),
+            clientid => maps:get(subid, SubOpts, null),
+            node => WhichNode
+        },
+        maps:with([qos, nl, rap, rh], SubOpts)
+    ).
+
+%%--------------------------------------------------------------------
+%% Internal functions
+%%--------------------------------------------------------------------
+
+check_match_topic(#{<<"match_topic">> := MatchTopic}) ->
+    try emqx_topic:parse(MatchTopic) of
+        {#share{}, _} -> {error, invalid_match_topic};
+        _ -> ok
+    catch
+        error:{invalid_topic_filter, _} ->
+            {error, invalid_match_topic}
+    end;
+check_match_topic(_) ->
+    ok.
+
 do_subscriptions_query(QString) ->
     Args = [?SUBOPTION, QString, ?SUBS_QSCHEMA, fun ?MODULE:qs2ms/2, fun ?MODULE:format/2],
     case maps:get(<<"node">>, QString, undefined) of
@@ -184,16 +197,6 @@ do_subscriptions_query(QString) ->
             end
     end.
 
-format(WhichNode, {{Topic, _Subscriber}, SubOpts}) ->
-    maps:merge(
-        #{
-            topic => emqx_topic:maybe_format_share(Topic),
-            clientid => maps:get(subid, SubOpts, null),
-            node => WhichNode
-        },
-        maps:with([qos, nl, rap, rh], SubOpts)
-    ).
-
 %%--------------------------------------------------------------------
 %% QueryString to MatchSpec
 %%--------------------------------------------------------------------