Merge pull request #10710 from terry-xiaoyu/webhook-crash

fix: webhook crash at on_client_connack
emqx · May 17, 2023 · 1aceb7a · 1aceb7a
2 parents 4a476be + b19136a
commit 1aceb7a
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 22 deletions.
diff --git a/apps/emqx_web_hook/src/emqx_web_hook.app.src b/apps/emqx_web_hook/src/emqx_web_hook.app.src
@@ -1,6 +1,6 @@
 {application, emqx_web_hook,
  [{description, "EMQX WebHook Plugin"},
-  {vsn, "4.3.18"}, % strict semver, bump manually!
+  {vsn, "4.3.19"}, % strict semver, bump manually!
   {modules, []},
   {registered, [emqx_web_hook_sup]},
   {applications, [kernel,stdlib,ehttpc]},

diff --git a/apps/emqx_web_hook/src/emqx_web_hook.appup.src b/apps/emqx_web_hook/src/emqx_web_hook.appup.src
@@ -1,13 +1,20 @@
 %% -*- mode: erlang -*-
 %% Unless you know what you are doing, DO NOT edit manually!!
 {VSN,
-  [{"4.3.17",[{load_module,emqx_web_hook_actions,brutal_purge,soft_purge,[]}]},
-   {"4.3.16",[{load_module,emqx_web_hook_actions,brutal_purge,soft_purge,[]}]},
+  [{"4.3.18",[{load_module,emqx_web_hook,brutal_purge,soft_purge,[]}]},
+   {"4.3.17",
+    [{load_module,emqx_web_hook,brutal_purge,soft_purge,[]},
+     {load_module,emqx_web_hook_actions,brutal_purge,soft_purge,[]}]},
+   {"4.3.16",
+    [{load_module,emqx_web_hook,brutal_purge,soft_purge,[]},
+     {load_module,emqx_web_hook_actions,brutal_purge,soft_purge,[]}]},
    {"4.3.15",
-    [{load_module,emqx_web_hook_app,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_web_hook,brutal_purge,soft_purge,[]},
+     {load_module,emqx_web_hook_app,brutal_purge,soft_purge,[]},
      {load_module,emqx_web_hook_actions,brutal_purge,soft_purge,[]}]},
    {"4.3.14",
-    [{load_module,emqx_web_hook_app,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_web_hook,brutal_purge,soft_purge,[]},
+     {load_module,emqx_web_hook_app,brutal_purge,soft_purge,[]},
      {load_module,emqx_web_hook_actions,brutal_purge,soft_purge,[]}]},
    {<<"4\\.3\\.[0-7]">>,
     [{apply,{application,stop,[emqx_web_hook]}},
@@ -31,16 +38,24 @@
      {load_module,emqx_web_hook_actions,brutal_purge,soft_purge,[]},
      {load_module,emqx_web_hook,brutal_purge,soft_purge,[]}]},
    {<<"4\\.3\\.1[2-4]">>,
-    [{load_module,emqx_web_hook_app,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_web_hook,brutal_purge,soft_purge,[]},
+     {load_module,emqx_web_hook_app,brutal_purge,soft_purge,[]},
      {load_module,emqx_web_hook_actions,brutal_purge,soft_purge,[]}]},
    {<<".*">>,[]}],
-  [{"4.3.17",[{load_module,emqx_web_hook_actions,brutal_purge,soft_purge,[]}]},
-   {"4.3.16",[{load_module,emqx_web_hook_actions,brutal_purge,soft_purge,[]}]},
+  [{"4.3.18",[{load_module,emqx_web_hook,brutal_purge,soft_purge,[]}]},
+   {"4.3.17",
+    [{load_module,emqx_web_hook,brutal_purge,soft_purge,[]},
+     {load_module,emqx_web_hook_actions,brutal_purge,soft_purge,[]}]},
+   {"4.3.16",
+    [{load_module,emqx_web_hook,brutal_purge,soft_purge,[]},
+     {load_module,emqx_web_hook_actions,brutal_purge,soft_purge,[]}]},
    {"4.3.15",
-    [{load_module,emqx_web_hook_app,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_web_hook,brutal_purge,soft_purge,[]},
+     {load_module,emqx_web_hook_app,brutal_purge,soft_purge,[]},
      {load_module,emqx_web_hook_actions,brutal_purge,soft_purge,[]}]},
    {"4.3.14",
-    [{load_module,emqx_web_hook_app,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_web_hook,brutal_purge,soft_purge,[]},
+     {load_module,emqx_web_hook_app,brutal_purge,soft_purge,[]},
      {load_module,emqx_web_hook_actions,brutal_purge,soft_purge,[]}]},
    {<<"4\\.3\\.[0-7]">>,
     [{apply,{application,stop,[emqx_web_hook]}},
@@ -64,6 +79,7 @@
      {load_module,emqx_web_hook_actions,brutal_purge,soft_purge,[]},
      {load_module,emqx_web_hook,brutal_purge,soft_purge,[]}]},
    {<<"4\\.3\\.1[2-4]">>,
-    [{load_module,emqx_web_hook_app,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_web_hook,brutal_purge,soft_purge,[]},
+     {load_module,emqx_web_hook_app,brutal_purge,soft_purge,[]},
      {load_module,emqx_web_hook_actions,brutal_purge,soft_purge,[]}]},
    {<<".*">>,[]}]}.
diff --git a/apps/emqx_web_hook/src/emqx_web_hook.erl b/apps/emqx_web_hook/src/emqx_web_hook.erl
@@ -91,10 +91,9 @@ on_client_connect(ConnInfo = #{clientid := ClientId, username := Username, peern
               , clientid => ClientId
               , username => maybe(Username)
               , ipaddress => iolist_to_binary(ntoa(Peerhost))
-              , keepalive => maps:get(keepalive, ConnInfo)
-              , proto_ver => maps:get(proto_ver, ConnInfo)
               },
-    send_http_request(ClientId, Params).
+    OptFields = [keepalive, proto_ver],
+    send_http_request(ClientId, add_optional_conn_fields(Params, OptFields, ConnInfo)).
 
 %%--------------------------------------------------------------------
 %% Client connack
@@ -107,12 +106,10 @@ on_client_connack(ConnInfo = #{clientid := ClientId, username := Username, peern
               , clientid => ClientId
               , username => maybe(Username)
               , ipaddress => iolist_to_binary(ntoa(Peerhost))
-              , keepalive => maps:get(keepalive, ConnInfo)
-              , proto_ver => maps:get(proto_ver, ConnInfo)
-              , connected_at => maps:get(connected_at, ConnInfo)
               , conn_ack => Rc
               },
-    send_http_request(ClientId, Params).
+    OptFields = [keepalive, proto_ver, connected_at],
+    send_http_request(ClientId, add_optional_conn_fields(Params, OptFields, ConnInfo)).
 
 %%--------------------------------------------------------------------
 %% Client connected
@@ -125,11 +122,9 @@ on_client_connected(#{clientid := ClientId, username := Username, peerhost := Pe
               , clientid => ClientId
               , username => maybe(Username)
               , ipaddress => iolist_to_binary(ntoa(Peerhost))
-              , keepalive => maps:get(keepalive, ConnInfo)
-              , proto_ver => maps:get(proto_ver, ConnInfo)
-              , connected_at => maps:get(connected_at, ConnInfo)
               },
-    send_http_request(ClientId, Params).
+    OptFields = [keepalive, proto_ver, connected_at],
+    send_http_request(ClientId, add_optional_conn_fields(Params, OptFields, ConnInfo)).
 
 %%--------------------------------------------------------------------
 %% Client disconnected
@@ -387,3 +382,6 @@ stringfy(Term) ->
 
 maybe(undefined) -> null;
 maybe(Str) -> Str.
+
+add_optional_conn_fields(Params, OptFields, ConnInfo) ->
+    maps:merge(Params, maps:with(OptFields, ConnInfo)).
diff --git a/changes/v4.4.19-en.md b/changes/v4.4.19-en.md
@@ -35,3 +35,7 @@
   Prior to the fix, the `date` variable exported by `FOREACH` could not be accessed in the `DO` clause of the above SQL, resulting in the following output for the SQL statement:
   `[{"elem": "a","date": "undefined"}]`.
   After the fix, the output of the SQL statement is: `[{"elem": "a","date": "2023-05-06"}]`
+
+- Fixed an issue where the WebHook plugin failed to execute the `on_client_connack` hook [#10710](https://github.com/emqx/emqx/pull/10710).
+
+  See https://github.com/emqx/emqx/issues/10628 for more details.
diff --git a/changes/v4.4.19-zh.md b/changes/v4.4.19-zh.md
@@ -35,3 +35,7 @@
   修复前，以上 SQL 语句中 `FOREACH` 导出的 `date` 变量无法在 `DO` 子句中访问，导致以上 SQL 的输出为：
   `[{"elem": "a","date": "undefined"}]`。
   修复后，SQL 的输出为：`[{"elem": "a","date": "2023-05-06"}]`
+
+- 修复 WebHook 插件执行 `on_client_connack` 钩子失败的问题 [#10710](https://github.com/emqx/emqx/pull/10710)。
+
+  详见 https://github.com/emqx/emqx/issues/10628