fix: mountpoint template render should not replace unknown as undefined

For backward compatibility, the unknown vars used in mountpoint
is kept unchanged.
e.g. '${unknown}/foo/bar' should be rendered as '${unknown}/foo/bar'
but not 'undefined/foo/bar'

apps/emqx/include/emqx_placeholder.hrl

@@ -31,12 +31,14 @@
 -define(PH_CERT_SUBJECT, ?PH(?VAR_CERT_SUBJECT)).
 -define(PH_CERT_CN_NAME, ?PH(?VAR_CERT_CN_NAME)).
 
-%% MQTT
+%% MQTT/Gateway
 -define(VAR_PASSWORD, "password").
 -define(VAR_CLIENTID, "clientid").
 -define(VAR_USERNAME, "username").
 -define(VAR_TOPIC, "topic").
+-define(VAR_ENDPOINT_NAME, "endpoint_name").
 -define(VAR_NS_CLIENT_ATTRS, {var_namespace, "client_attrs"}).
+
 -define(PH_PASSWORD, ?PH(?VAR_PASSWORD)).
 -define(PH_CLIENTID, ?PH(?VAR_CLIENTID)).
 -define(PH_FROM_CLIENTID, ?PH("from_clientid")).
@@ -90,7 +92,7 @@
 -define(PH_NODE, ?PH("node")).
 -define(PH_REASON, ?PH("reason")).
 
--define(PH_ENDPOINT_NAME, ?PH("endpoint_name")).
+-define(PH_ENDPOINT_NAME, ?PH(?VAR_ENDPOINT_NAME)).
 -define(VAR_RETAIN, "retain").
 -define(PH_RETAIN, ?PH(?VAR_RETAIN)).
 

apps/emqx/src/emqx_mountpoint.erl

@@ -28,10 +28,19 @@
 
 -export([replvar/2]).
 
+-export([lookup/2]).
+
 -export_type([mountpoint/0]).
 
 -type mountpoint() :: binary().
 
+-define(ALLOWED_VARS, [
+    ?VAR_CLIENTID,
+    ?VAR_USERNAME,
+    ?VAR_ENDPOINT_NAME,
+    ?VAR_NS_CLIENT_ATTRS
+]).
+
 -spec mount(option(mountpoint()), Any) -> Any when
     Any ::
         emqx_types:topic()
@@ -87,12 +96,29 @@ unmount_maybe_share(MountPoint, TopicFilter = #share{topic = Topic}) when
 -spec replvar(option(mountpoint()), map()) -> option(mountpoint()).
 replvar(undefined, _Vars) ->
     undefined;
-replvar(MountPoint, Vars0) ->
+replvar(MountPoint, Vars) ->
-    Allowed = [clientid, username, endpoint_name, client_attrs],
+    Template = parse(MountPoint),
-    Vars = maps:filter(
+    {String, _Errors} = emqx_template:render(Template, {?MODULE, Vars}),
-        fun(K, V) -> V =/= undefined andalso lists:member(K, Allowed) end,
-        Vars0
-    ),
-    Template = emqx_template:parse(MountPoint),
-    {String, _Errors} = emqx_template:render(Template, Vars),
     unicode:characters_to_binary(String).
+
+lookup([<<?VAR_CLIENTID>>], #{clientid := ClientId}) when is_binary(ClientId) ->
+    {ok, ClientId};
+lookup([<<?VAR_USERNAME>>], #{username := Username}) when is_binary(Username) ->
+    {ok, Username};
+lookup([<<?VAR_ENDPOINT_NAME>>], #{endpoint_name := Name}) when is_binary(Name) ->
+    {ok, Name};
+lookup([<<"client_attrs">>, AttrName], #{client_attrs := Attrs}) when is_map(Attrs) ->
+    Original = iolist_to_binary(["${client_attrs.", AttrName, "}"]),
+    {ok, maps:get(AttrName, Attrs, Original)};
+lookup(Accessor, _) ->
+    {ok, iolist_to_binary(["${", lists:join(".", Accessor), "}"])}.
+
+parse(Template) ->
+    Parsed = emqx_template:parse(Template),
+    case emqx_template:validate(?ALLOWED_VARS, Parsed) of
+        ok ->
+            Parsed;
+        {error, _Disallowed} ->
+            Escaped = emqx_template:escape_disallowed(Parsed, ?ALLOWED_VARS),
+            emqx_template:parse(Escaped)
+    end.

apps/emqx/test/emqx_mountpoint_SUITE.erl

@@ -116,4 +116,32 @@ t_replvar(_) ->
                 username => undefined
             }
         )
+    ),
+    ?assertEqual(
+        <<"mount/g1/clientid/">>,
+        replvar(
+            <<"mount/${client_attrs.group}/${clientid}/">>,
+            #{
+                clientid => <<"clientid">>,
+                client_attrs => #{<<"group">> => <<"g1">>}
+            }
+        )
+    ),
+    ?assertEqual(
+        <<"mount/${client_attrs.group}/clientid/">>,
+        replvar(
+            <<"mount/${client_attrs.group}/${clientid}/">>,
+            #{
+                clientid => <<"clientid">>
+            }
+        )
+    ),
+    ?assertEqual(
+        <<"mount/${not.allowed}/clientid/">>,
+        replvar(
+            <<"mount/${not.allowed}/${clientid}/">>,
+            #{
+                clientid => <<"clientid">>
+            }
+        )
     ).

apps/emqx_auth/src/emqx_authz/emqx_authz_utils.erl

@@ -139,7 +139,7 @@ handle_disallowed_placeholders(Template, Source, Allowed) ->
                     " However, consider using `${$}` escaping for literal `$` where"
                     " needed to avoid unexpected results."
             }),
-            Result = prerender_disallowed_placeholders(Template, Allowed),
+            Result = emqx_template:escape_disallowed(Template, Allowed),
             case Source of
                 {string, _} ->
                     emqx_template:parse(Result);
@@ -148,20 +148,6 @@ handle_disallowed_placeholders(Template, Source, Allowed) ->
             end
     end.
 
-prerender_disallowed_placeholders(Template, Allowed) ->
-    {Result, _} = emqx_template:render(Template, #{}, #{
-        var_trans => fun(Name, _) ->
-            % NOTE
-            % Rendering disallowed placeholders in escaped form, which will then
-            % parse as a literal string.
-            case lists:member(Name, Allowed) of
-                true -> "${" ++ Name ++ "}";
-                false -> "${$}{" ++ Name ++ "}"
-            end
-        end
-    }),
-    Result.
-
 render_deep(Template, Values) ->
     % NOTE
     % Ignoring errors here, undefined bindings will be replaced with empty string.

apps/emqx_utils/src/emqx_template.erl

@@ -32,6 +32,7 @@
 -export([lookup/2]).
 
 -export([to_string/1]).
+-export([escape_disallowed/2]).
 
 -export_type([t/0]).
 -export_type([str/0]).
@@ -157,9 +158,31 @@ validate(Allowed, Template) ->
             {error, [{Var, disallowed} || Var <- Disallowed]}
     end.
 
+%% @doc Escape `$' with `${$}' for the variable references
+%% which are not allowed, so the original variable name
+%% can be preserved instead of rendered as `undefined'.
+%% E.g. to render `${var1}/${clientid}', if only `clientid'
+%% is allowed, the rendering result should be `${var1}/client1'
+%% but not `undefined/client1'.
+escape_disallowed(Template, Allowed) ->
+    {Result, _} = render(Template, #{}, #{
+        var_trans => fun(Name, _) ->
+            case is_allowed(Name, Allowed) of
+                true -> "${" ++ Name ++ "}";
+                false -> "${$}{" ++ Name ++ "}"
+            end
+        end
+    }),
+    Result.
+
 find_disallowed(Vars, Allowed) ->
     lists:filter(fun(Var) -> not is_allowed(Var, Allowed) end, Vars).
 
+%% @private Return 'true' if a variable reference matches
+%% at least one allowed variables.
+%% For `"${var_name}"' kind of reference, its a `=:=' compare
+%% for `{var_namespace, "namespace"}' kind of reference
+%% it matches the `"namespace."' prefix.
 is_allowed(_Var, []) ->
     false;
 is_allowed(Var, [{var_namespace, VarPrefix} | Allowed]) ->