From f0cc75d1446582b9e2829e390eadb84d5b42f150 Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Thu, 8 Sep 2022 16:37:38 +0800 Subject: [PATCH 1/7] fix(sql): topic matching to null values should return false --- CHANGES-4.3.md | 6 +- .../src/emqx_rule_runtime.erl | 5 +- .../test/emqx_rule_engine_SUITE.erl | 142 ++++++++++++++++-- 3 files changed, 138 insertions(+), 15 deletions(-) diff --git a/CHANGES-4.3.md b/CHANGES-4.3.md index aa250b7ad..9be7de63f 100644 --- a/CHANGES-4.3.md +++ b/CHANGES-4.3.md @@ -17,7 +17,11 @@ File format: - Fix rule-engine update behaviour which may initialize actions for disabled rules. [#8849](https://github.com/emqx/emqx/pull/8849) - Fix JWT plugin don't support non-integer timestamp claims. [#8862](https://github.com/emqx/emqx/pull/8862) - Fix dashboard binding IP address not working. [#8916](https://github.com/emqx/emqx/pull/8916) - +- Fix rule SQL topic matching to null values failed. [#?](https://github.com/emqx/emqx/pull/?) + The following SQL should not fail (crash) but return `{"r": false}`: + `SELECT topic =~ 't' as r FROM "$events/client_connected"`. + The topic is a null value as there's no such field in event `$events/client_connected`, so it + should return false if mathch it to a topic. ## v4.3.19 diff --git a/apps/emqx_rule_engine/src/emqx_rule_runtime.erl b/apps/emqx_rule_engine/src/emqx_rule_runtime.erl index 794c70dfd..71775f6ee 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_runtime.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_runtime.erl @@ -242,7 +242,10 @@ do_compare('>=', L, R) -> do_compare('=', L, R) orelse do_compare('>', L, R); do_compare('<>', L, R) -> L /= R; do_compare('!=', L, R) -> L /= R; -do_compare('=~', T, F) -> emqx_topic:match(T, F). +do_compare('=~', undefined, undefined) -> true; +do_compare('=~', T, F) when T == undefined; F == undefined -> false; +do_compare('=~', T, F) -> + emqx_topic:match(T, F). number(Bin) -> try binary_to_integer(Bin) diff --git a/apps/emqx_rule_engine/test/emqx_rule_engine_SUITE.erl b/apps/emqx_rule_engine/test/emqx_rule_engine_SUITE.erl index d1e6b26a0..c8f66ea00 100644 --- a/apps/emqx_rule_engine/test/emqx_rule_engine_SUITE.erl +++ b/apps/emqx_rule_engine/test/emqx_rule_engine_SUITE.erl @@ -2505,6 +2505,13 @@ t_sqlparse_compare_undefined(_Config) -> %% no match ?assertMatch({error, nomatch}, ?TEST_SQL(Sql00)), + Sql00_1 = "select " + " * " + "from \"t/#\" " + "where dev <> undefined ", + %% no match + ?assertMatch({error, nomatch}, ?TEST_SQL(Sql00_1)), + Sql01 = "select " " 'd' as dev " "from \"t/#\" " @@ -2513,13 +2520,29 @@ t_sqlparse_compare_undefined(_Config) -> %% pass ?assertMatch(#{}, Res01), + Sql01_1 = "select " + " 'd' as dev " + "from \"t/#\" " + "where dev <> undefined ", + {ok, Res01_1} = ?TEST_SQL(Sql01_1), + %% pass + ?assertMatch(#{}, Res01_1), + Sql02 = "select " " * " "from \"t/#\" " "where dev != 'undefined' ", {ok, Res02} = ?TEST_SQL(Sql02), %% pass - ?assertMatch(#{}, Res02). + ?assertMatch(#{}, Res02), + + Sql03 = "select " + " * " + "from \"t/#\" " + "where dev =~ 'undefined' ", + Res03 = ?TEST_SQL(Sql03), + %% no match + ?assertMatch({error, nomatch}, Res03). t_sqlparse_compare_null_null(_Config) -> %% test undefined == undefined @@ -2538,6 +2561,14 @@ t_sqlparse_compare_null_null(_Config) -> ?assertMatch(#{<<"c">> := false }, Res01), + %% test undefined <> undefined + Sql01_1 = "select " + " a <> b as c " + "from \"t/#\" ", + {ok, Res01_1} = ?TEST_SQL(Sql01_1), + ?assertMatch(#{<<"c">> := false + }, Res01_1), + %% test undefined > undefined Sql02 = "select " " a > b as c " @@ -2568,10 +2599,18 @@ t_sqlparse_compare_null_null(_Config) -> "from \"t/#\" ", {ok, Res05} = ?TEST_SQL(Sql05), ?assertMatch(#{<<"c">> := true - }, Res05). + }, Res05), + + %% test undefined =~ undefined + Sql06 = "select " + " a =~ b as c " + "from \"t/#\" ", + {ok, Res06} = ?TEST_SQL(Sql06), + ?assertMatch(#{<<"c">> := true + }, Res06). t_sqlparse_compare_null_notnull(_Config) -> - %% test undefined == b + %% test undefined == 'b' Sql00 = "select " " 'b' as b, a = b as c " "from \"t/#\" ", @@ -2579,7 +2618,7 @@ t_sqlparse_compare_null_notnull(_Config) -> ?assertMatch(#{<<"c">> := false }, Res00), - %% test undefined != b + %% test undefined != 'b' Sql01 = "select " " 'b' as b, a != b as c " "from \"t/#\" ", @@ -2587,7 +2626,15 @@ t_sqlparse_compare_null_notnull(_Config) -> ?assertMatch(#{<<"c">> := true }, Res01), - %% test undefined > b + %% test undefined <> 'b' + Sql01_1 = "select " + " 'b' as b, a <> b as c " + "from \"t/#\" ", + {ok, Res01_1} = ?TEST_SQL(Sql01_1), + ?assertMatch(#{<<"c">> := true + }, Res01_1), + + %% test undefined > 'b' Sql02 = "select " " 'b' as b, a > b as c " "from \"t/#\" ", @@ -2595,7 +2642,7 @@ t_sqlparse_compare_null_notnull(_Config) -> ?assertMatch(#{<<"c">> := false }, Res02), - %% test undefined < b + %% test undefined < 'b' Sql03 = "select " " 'b' as b, a < b as c " "from \"t/#\" ", @@ -2603,7 +2650,7 @@ t_sqlparse_compare_null_notnull(_Config) -> ?assertMatch(#{<<"c">> := false }, Res03), - %% test undefined <= b + %% test undefined <= 'b' Sql04 = "select " " 'b' as b, a <= b as c " "from \"t/#\" ", @@ -2611,13 +2658,21 @@ t_sqlparse_compare_null_notnull(_Config) -> ?assertMatch(#{<<"c">> := false }, Res04), - %% test undefined >= b + %% test undefined >= 'b' Sql05 = "select " " 'b' as b, a >= b as c " "from \"t/#\" ", {ok, Res05} = ?TEST_SQL(Sql05), ?assertMatch(#{<<"c">> := false - }, Res05). + }, Res05), + + %% test undefined =~ 'b' + Sql06 = "select " + " 'b' as b, a =~ b as c " + "from \"t/#\" ", + {ok, Res06} = ?TEST_SQL(Sql06), + ?assertMatch(#{<<"c">> := false + }, Res06). t_sqlparse_compare_notnull_null(_Config) -> %% test 'a' == undefined @@ -2636,6 +2691,14 @@ t_sqlparse_compare_notnull_null(_Config) -> ?assertMatch(#{<<"c">> := true }, Res01), + %% test 'a' <> undefined + Sql01_1 = "select " + " 'a' as a, a <> b as c " + "from \"t/#\" ", + {ok, Res01_1} = ?TEST_SQL(Sql01_1), + ?assertMatch(#{<<"c">> := true + }, Res01_1), + %% test 'a' > undefined Sql02 = "select " " 'a' as a, a > b as c " @@ -2666,7 +2729,15 @@ t_sqlparse_compare_notnull_null(_Config) -> "from \"t/#\" ", {ok, Res05} = ?TEST_SQL(Sql05), ?assertMatch(#{<<"c">> := false - }, Res05). + }, Res05), + + %% test 'a' =~ undefined + Sql06 = "select " + " 'a' as a, a =~ b as c " + "from \"t/#\" ", + {ok, Res06} = ?TEST_SQL(Sql06), + ?assertMatch(#{<<"c">> := false + }, Res06). t_sqlparse_compare(_Config) -> Sql00 = "select " @@ -2676,6 +2747,13 @@ t_sqlparse_compare(_Config) -> ?assertMatch(#{<<"c">> := true }, Res00), + Sql00_1 = "select " + " 'true' as a, true as b, a = b as c " + "from \"t/#\" ", + {ok, Res00_1} = ?TEST_SQL(Sql00_1), + ?assertMatch(#{<<"c">> := true + }, Res00_1), + Sql01 = "select " " is_null(a) as c " "from \"t/#\" ", @@ -2704,7 +2782,21 @@ t_sqlparse_compare(_Config) -> ?assertMatch(#{<<"c">> := false }, Res04), - %% test 'a' >= undefined + Sql04_0 = "select " + " 1 as a, 1 as b, a = b as c " + "from \"t/#\" ", + {ok, Res04_0} = ?TEST_SQL(Sql04_0), + ?assertMatch(#{<<"c">> := true + }, Res04_0), + + Sql04_1 = "select " + " 1 as a, '1' as b, a = b as c " + "from \"t/#\" ", + {ok, Res04_1} = ?TEST_SQL(Sql04_1), + ?assertMatch(#{<<"c">> := true + }, Res04_1), + + %% test 1 >= 2 Sql05 = "select " " 1 as a, 2 as b, a >= b as c " "from \"t/#\" ", @@ -2712,13 +2804,37 @@ t_sqlparse_compare(_Config) -> ?assertMatch(#{<<"c">> := false }, Res05), - %% test 'a' >= undefined + %% test 1 <= 2 Sql06 = "select " " 1 as a, 2 as b, a <= b as c " "from \"t/#\" ", {ok, Res06} = ?TEST_SQL(Sql06), ?assertMatch(#{<<"c">> := true - }, Res06). + }, Res06), + + %% test 1 != 2 + Sql07 = "select " + " 1 as a, 2 as b, a != b as c " + "from \"t/#\" ", + {ok, Res07} = ?TEST_SQL(Sql07), + ?assertMatch(#{<<"c">> := true + }, Res07), + + %% test 1 <> 2 + Sql07_1 = "select " + " 1 as a, 2 as b, a <> b as c " + "from \"t/#\" ", + {ok, Res07_1} = ?TEST_SQL(Sql07_1), + ?assertMatch(#{<<"c">> := true + }, Res07_1), + + %% test 't' =~ 't' + Sql08 = "select " + " 't' as a, 't' as b, a =~ b as c " + "from \"t/#\" ", + {ok, Res08} = ?TEST_SQL(Sql08), + ?assertMatch(#{<<"c">> := true + }, Res08). t_sqlparse_new_map(_Config) -> %% construct a range without 'as' From b9d75181e5f74c0ecaae65776757a71052737e5c Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Thu, 8 Sep 2022 16:38:34 +0800 Subject: [PATCH 2/7] chore: update emqx_rule_engine.appup.src --- apps/emqx_rule_engine/src/emqx_rule_engine.appup.src | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src b/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src index b976a92bd..c24bead6e 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src +++ b/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src @@ -3,6 +3,7 @@ {VSN, [{"4.3.14", [{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]}, {"4.3.13", [{load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}, @@ -185,6 +186,7 @@ {<<".*">>,[]}], [{"4.3.14", [{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]}, {"4.3.13", [{load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}, From 93d10e63b04ff3c1ada172997cb0ef519052f882 Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Thu, 8 Sep 2022 16:53:24 +0800 Subject: [PATCH 3/7] chore: update CHANGES-4.3.md for #8927 --- CHANGES-4.3.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES-4.3.md b/CHANGES-4.3.md index 9be7de63f..796efcfff 100644 --- a/CHANGES-4.3.md +++ b/CHANGES-4.3.md @@ -17,11 +17,11 @@ File format: - Fix rule-engine update behaviour which may initialize actions for disabled rules. [#8849](https://github.com/emqx/emqx/pull/8849) - Fix JWT plugin don't support non-integer timestamp claims. [#8862](https://github.com/emqx/emqx/pull/8862) - Fix dashboard binding IP address not working. [#8916](https://github.com/emqx/emqx/pull/8916) -- Fix rule SQL topic matching to null values failed. [#?](https://github.com/emqx/emqx/pull/?) +- Fix rule SQL topic matching to null values failed. [#8927](https://github.com/emqx/emqx/pull/8927) The following SQL should not fail (crash) but return `{"r": false}`: `SELECT topic =~ 't' as r FROM "$events/client_connected"`. The topic is a null value as there's no such field in event `$events/client_connected`, so it - should return false if mathch it to a topic. + should return false if match it to a topic. ## v4.3.19 From 2c54190479eee500e3709b20a8febff9ee23a6a9 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Fri, 9 Sep 2022 12:27:02 +0200 Subject: [PATCH 4/7] fix: revert 'accepted' state of exproto plugin --- apps/emqx_exproto/src/emqx_exproto_channel.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/emqx_exproto/src/emqx_exproto_channel.erl b/apps/emqx_exproto/src/emqx_exproto_channel.erl index 4b354183e..6847b905e 100644 --- a/apps/emqx_exproto/src/emqx_exproto_channel.erl +++ b/apps/emqx_exproto/src/emqx_exproto_channel.erl @@ -66,7 +66,7 @@ -opaque(channel() :: #channel{}). --type(conn_state() :: idle | connecting | connected | disconnected). +-type(conn_state() :: idle | connecting | connected | disconnected | accepted). -type(reply() :: {outgoing, binary()} | {outgoing, [binary()]} @@ -159,7 +159,7 @@ init(ConnInfo = #{socktype := Socktype, Channel = #channel{gcli = #{channel => GRpcChann}, conninfo = NConnInfo1, clientinfo = ClientInfo, - conn_state = idle, + conn_state = accepted, timers = #{} }, case emqx_hooks:run_fold('client.connect', [NConnInfo], #{}) of From c463569d88c1ea1f64401adbf15334e7ddb5921c Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Fri, 9 Sep 2022 17:10:39 +0200 Subject: [PATCH 5/7] chore: bump versions in Chart --- deploy/charts/emqx/Chart.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/deploy/charts/emqx/Chart.yaml b/deploy/charts/emqx/Chart.yaml index 7667ab443..2ad425fc0 100644 --- a/deploy/charts/emqx/Chart.yaml +++ b/deploy/charts/emqx/Chart.yaml @@ -13,8 +13,8 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. -version: 4.3.19 +version: 4.3.20 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. -appVersion: 4.3.19 +appVersion: 4.3.20 From 9e122f38b9c8bd60ede4d41b1adda59b5adf3a2d Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Fri, 9 Sep 2022 12:39:53 -0300 Subject: [PATCH 6/7] ci(fix): fix check for appup coverage Before: ```elixir iex(14)> :re.run('4.3.10', "4\\.3\\.[0-4]", [{:capture, :first, :list}]) {:match, ['4.3.1']} ``` --- scripts/update_appup.escript | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/update_appup.escript b/scripts/update_appup.escript index 141fd5833..9282aa902 100755 --- a/scripts/update_appup.escript +++ b/scripts/update_appup.escript @@ -374,12 +374,12 @@ ensure_version(Version, OldInstructions) -> contains_version(Needle, Haystack) when is_list(Needle) -> lists:any( - fun(<<"*">>) -> true; %% TODO: delete after we pass esockd 5.8.4 - (Regex) when is_binary(Regex) -> + fun(Regex) when is_binary(Regex) -> + Length = length(Needle), case re:run(Needle, Regex) of - {match, _} -> + {match, [{0, Length}]} -> true; - nomatch -> + _ -> false end; (Vsn) -> From 94afb633f8715ae153a0606da2f5ba8dce939694 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Fri, 9 Sep 2022 13:37:00 -0300 Subject: [PATCH 7/7] chore: update missing appup instructions --- apps/emqx_exproto/src/emqx_exproto.appup.src | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/emqx_exproto/src/emqx_exproto.appup.src b/apps/emqx_exproto/src/emqx_exproto.appup.src index 20939ae63..fcf2fe6db 100644 --- a/apps/emqx_exproto/src/emqx_exproto.appup.src +++ b/apps/emqx_exproto/src/emqx_exproto.appup.src @@ -1,7 +1,8 @@ %% -*- mode: erlang -*- %% Unless you know what you are doing, DO NOT edit manually!! {VSN, - [{"4.3.9", + [{"4.3.10",[{load_module,emqx_exproto_channel,brutal_purge,soft_purge,[]}]}, + {"4.3.9", [{load_module,emqx_exproto_channel,brutal_purge,soft_purge,[]}, {load_module,emqx_exproto_gcli,brutal_purge,soft_purge,[]}]}, {<<"4\\.3\\.[2-8]">>, @@ -14,7 +15,8 @@ {load_module,emqx_exproto_conn,brutal_purge,soft_purge,[]}, {load_module,emqx_exproto_channel,brutal_purge,soft_purge,[]}]}, {<<".*">>,[]}], - [{"4.3.9", + [{"4.3.10",[{load_module,emqx_exproto_channel,brutal_purge,soft_purge,[]}]}, + {"4.3.9", [{load_module,emqx_exproto_channel,brutal_purge,soft_purge,[]}, {load_module,emqx_exproto_gcli,brutal_purge,soft_purge,[]}]}, {<<"4\\.3\\.[2-8]">>,