From c2b1571134f99bd780114d5abcc770a170dfd767 Mon Sep 17 00:00:00 2001 From: firest Date: Mon, 21 Mar 2022 18:32:42 +0800 Subject: [PATCH 1/4] fix(auto_subscribe): make log if the topic is empty when auto subscribe --- .../src/emqx_mod_subscription.erl | 57 ++++++++++++++----- 1 file changed, 44 insertions(+), 13 deletions(-) diff --git a/lib-ce/emqx_modules/src/emqx_mod_subscription.erl b/lib-ce/emqx_modules/src/emqx_mod_subscription.erl index 06178aee7..76b4b1ac9 100644 --- a/lib-ce/emqx_modules/src/emqx_mod_subscription.erl +++ b/lib-ce/emqx_modules/src/emqx_mod_subscription.erl @@ -20,6 +20,7 @@ -include_lib("emqx/include/emqx.hrl"). -include_lib("emqx/include/emqx_mqtt.hrl"). +-include_lib("emqx/include/logger.hrl"). %% emqx_gen_mod callbacks -export([ load/1 @@ -38,14 +39,29 @@ load(Topics) -> emqx_hooks:add('client.connected', {?MODULE, on_client_connected, [Topics]}). on_client_connected(#{clientid := ClientId, username := Username}, _ConnInfo = #{proto_ver := ProtoVer}, Topics) -> - Replace = fun(Topic) -> - rep(<<"%u">>, Username, rep(<<"%c">>, ClientId, Topic)) + + OptFun = case ProtoVer of + ?MQTT_PROTO_V5 -> fun(X) -> X end; + _ -> fun(#{qos := Qos}) -> #{qos => Qos} end end, - TopicFilters = case ProtoVer of - ?MQTT_PROTO_V5 -> [{Replace(Topic), SubOpts} || {Topic, SubOpts} <- Topics]; - _ -> [{Replace(Topic), #{qos => Qos}} || {Topic, #{qos := Qos}} <- Topics] - end, - self() ! {subscribe, TopicFilters}. + + Fold = fun({Topic, SubOpts}, Acc) -> + case rep(Topic, ClientId, Username) of + {error, _} -> + Acc; + <<>> -> + ?LOG(warning, "Topic can't be empty when auto subscribe"), + Acc; + NewTopic -> + [{NewTopic, OptFun(SubOpts)} | Acc] + end + end, + + case lists:foldl(Fold, [], Topics) of + [] -> ok; + TopicFilters -> + self() ! {subscribe, TopicFilters} + end. unload(_) -> emqx_hooks:del('client.connected', {?MODULE, on_client_connected}). @@ -56,10 +72,25 @@ description() -> %% Internal functions %%-------------------------------------------------------------------- -rep(<<"%c">>, ClientId, Topic) -> - emqx_topic:feed_var(<<"%c">>, ClientId, Topic); -rep(<<"%u">>, undefined, Topic) -> - Topic; -rep(<<"%u">>, Username, Topic) -> - emqx_topic:feed_var(<<"%u">>, Username, Topic). +rep(Topic, ClientId, Username) -> + Words = emqx_topic:words(Topic), + rep(Words, ClientId, Username, []). +rep([<<"%c">> | T], ClientId, Username, Acc) -> + rep(T, + ClientId, + Username, + [ClientId | Acc]); +rep([<<"%u">> | _], _, undefined, _) -> + ?LOG(error, "Username undefined when auto subscribe"), + {error, username_undefined}; +rep([<<"%u">> | T], ClientId, Username, Acc) -> + rep(T, + ClientId, + Username, + [Username | Acc]); +rep([H | T], ClientId, UserName, Acc) -> + rep(T, ClientId, UserName, [H | Acc]); + +rep([], _, _, Acc) -> + emqx_topic:join(lists:reverse(Acc)). From 4fe9275103429316d37120974d895c88f160e80b Mon Sep 17 00:00:00 2001 From: firest Date: Tue, 22 Mar 2022 10:08:52 +0800 Subject: [PATCH 2/4] chore(modules): update appup file --- .../emqx_modules/src/emqx_modules.appup.src | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/lib-ce/emqx_modules/src/emqx_modules.appup.src b/lib-ce/emqx_modules/src/emqx_modules.appup.src index a82421aec..01b9c6651 100644 --- a/lib-ce/emqx_modules/src/emqx_modules.appup.src +++ b/lib-ce/emqx_modules/src/emqx_modules.appup.src @@ -1,29 +1,39 @@ %% -*- mode: erlang -*- %% Unless you know what you are doing, DO NOT edit manually!! {VSN, - [{"4.3.4",[{load_module,emqx_mod_delayed,brutal_purge,soft_purge,[]}]}, + [{"4.3.4", + [{load_module,emqx_mod_subscription,brutal_purge,soft_purge,[]}, + {load_module,emqx_mod_delayed,brutal_purge,soft_purge,[]}]}, {<<"4\\.3\\.[2-3]">>, - [{load_module,emqx_mod_delayed,brutal_purge,soft_purge,[]}, + [{load_module,emqx_mod_subscription,brutal_purge,soft_purge,[]}, + {load_module,emqx_mod_delayed,brutal_purge,soft_purge,[]}, {load_module,emqx_mod_presence,brutal_purge,soft_purge,[]}]}, {"4.3.1", - [{load_module,emqx_mod_delayed,brutal_purge,soft_purge,[]}, + [{load_module,emqx_mod_subscription,brutal_purge,soft_purge,[]}, + {load_module,emqx_mod_delayed,brutal_purge,soft_purge,[]}, {load_module,emqx_mod_presence,brutal_purge,soft_purge,[]}, {load_module,emqx_mod_api_topic_metrics,brutal_purge,soft_purge,[]}]}, {"4.3.0", - [{update,emqx_mod_delayed,{advanced,[]}}, + [{load_module,emqx_mod_subscription,brutal_purge,soft_purge,[]}, + {update,emqx_mod_delayed,{advanced,[]}}, {load_module,emqx_mod_presence,brutal_purge,soft_purge,[]}, {load_module,emqx_mod_api_topic_metrics,brutal_purge,soft_purge,[]}]}, {<<".*">>,[]}], - [{"4.3.4",[{load_module,emqx_mod_delayed,brutal_purge,soft_purge,[]}]}, + [{"4.3.4", + [{load_module,emqx_mod_subscription,brutal_purge,soft_purge,[]}, + {load_module,emqx_mod_delayed,brutal_purge,soft_purge,[]}]}, {<<"4\\.3\\.[2-3]">>, - [{load_module,emqx_mod_delayed,brutal_purge,soft_purge,[]}, + [{load_module,emqx_mod_subscription,brutal_purge,soft_purge,[]}, + {load_module,emqx_mod_delayed,brutal_purge,soft_purge,[]}, {load_module,emqx_mod_presence,brutal_purge,soft_purge,[]}]}, {"4.3.1", - [{load_module,emqx_mod_delayed,brutal_purge,soft_purge,[]}, + [{load_module,emqx_mod_subscription,brutal_purge,soft_purge,[]}, + {load_module,emqx_mod_delayed,brutal_purge,soft_purge,[]}, {load_module,emqx_mod_presence,brutal_purge,soft_purge,[]}, {load_module,emqx_mod_api_topic_metrics,brutal_purge,soft_purge,[]}]}, {"4.3.0", - [{update,emqx_mod_delayed,{advanced,[]}}, + [{load_module,emqx_mod_subscription,brutal_purge,soft_purge,[]}, + {update,emqx_mod_delayed,{advanced,[]}}, {load_module,emqx_mod_presence,brutal_purge,soft_purge,[]}, {load_module,emqx_mod_api_topic_metrics,brutal_purge,soft_purge,[]}]}, {<<".*">>,[]}]}. From c43f17920469eabc1b3a366369b345cefbcf622b Mon Sep 17 00:00:00 2001 From: firest Date: Tue, 22 Mar 2022 18:09:06 +0800 Subject: [PATCH 3/4] fix(mod_subscription): improve the log information --- lib-ce/emqx_modules/src/emqx_mod_subscription.erl | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib-ce/emqx_modules/src/emqx_mod_subscription.erl b/lib-ce/emqx_modules/src/emqx_mod_subscription.erl index 76b4b1ac9..1b6a2c1c7 100644 --- a/lib-ce/emqx_modules/src/emqx_mod_subscription.erl +++ b/lib-ce/emqx_modules/src/emqx_mod_subscription.erl @@ -47,10 +47,14 @@ on_client_connected(#{clientid := ClientId, username := Username}, _ConnInfo = # Fold = fun({Topic, SubOpts}, Acc) -> case rep(Topic, ClientId, Username) of - {error, _} -> + {error, Reason} -> + ?LOG(warning, "auto subscribe ignored, topic filter:~ts reason:~p~n", + [Topic, Reason]), Acc; <<>> -> - ?LOG(warning, "Topic can't be empty when auto subscribe"), + ?LOG(warning, "auto subscribe ignored, topic filter:~ts" + " reason: topic can't be empty~n", + [Topic]), Acc; NewTopic -> [{NewTopic, OptFun(SubOpts)} | Acc] @@ -82,7 +86,6 @@ rep([<<"%c">> | T], ClientId, Username, Acc) -> Username, [ClientId | Acc]); rep([<<"%u">> | _], _, undefined, _) -> - ?LOG(error, "Username undefined when auto subscribe"), {error, username_undefined}; rep([<<"%u">> | T], ClientId, Username, Acc) -> rep(T, From b12b72df994590a56590d25dd996ea2f220c9d54 Mon Sep 17 00:00:00 2001 From: firest Date: Wed, 23 Mar 2022 10:55:19 +0800 Subject: [PATCH 4/4] chore: update changes-4.3.md --- CHANGES-4.3.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES-4.3.md b/CHANGES-4.3.md index a7e161915..25760be08 100644 --- a/CHANGES-4.3.md +++ b/CHANGES-4.3.md @@ -50,6 +50,7 @@ File format: * Fix user or appid created, name only allow `^[A-Za-z]+[A-Za-z0-9-_]*$` * Fix subscribe http api crash by bad_qos `/mqtt/subscribe`,`/mqtt/subscribe_batch`. * Send DISCONNECT packet with reason code 0x98 if connection has been kicked [#7309] +* Auto subscribe to an empty topic will be simply ignored now ## v4.3.12 ### Important changes