From b2c3d8366da17b8829516a7580519136090ac46b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=91=A8=E5=AD=90=E5=8D=9A?= <349832309@qq.com>
Date: Mon, 3 Dec 2018 13:57:37 +0800
Subject: [PATCH] Add logs for malformed acl configuration file

---
 src/emqx_access_rule.erl  | 10 +++++++---
 src/emqx_acl_internal.erl | 28 ++++++++++++++++++----------
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/src/emqx_access_rule.erl b/src/emqx_access_rule.erl
index 3fb6dd7ef..1d3154735 100644
--- a/src/emqx_access_rule.erl
+++ b/src/emqx_access_rule.erl
@@ -34,16 +34,20 @@
 -export([match/3]).
 
 -define(ALLOW_DENY(A), ((A =:= allow) orelse (A =:= deny))).
+-define(PUBSUB(A), ((A =:= subscribe) orelse (A =:= publish) orelse (A =:= pubsub))).
 
 %% @doc Compile Access Rule.
 compile({A, all}) when ?ALLOW_DENY(A) ->
     {A, all};
 
-compile({A, Who, Access, Topic}) when ?ALLOW_DENY(A), is_binary(Topic) ->
+compile({A, Who, Access, Topic}) when ?ALLOW_DENY(A), ?PUBSUB(Access), is_binary(Topic) ->
     {A, compile(who, Who), Access, [compile(topic, Topic)]};
 
-compile({A, Who, Access, TopicFilters}) when ?ALLOW_DENY(A) ->
-    {A, compile(who, Who), Access, [compile(topic, Topic) || Topic <- TopicFilters]}.
+compile({A, Who, Access, TopicFilters}) when ?ALLOW_DENY(A), ?PUBSUB(Access) ->
+    {A, compile(who, Who), Access, [compile(topic, Topic) || Topic <- TopicFilters]};
+
+compile(Rule) ->
+    emqx_logger:error("[ACCESS_RULE] Malformed rule: ~p", [Rule]).
 
 compile(who, all) ->
     all;
diff --git a/src/emqx_acl_internal.erl b/src/emqx_acl_internal.erl
index eee7e6c18..383967030 100644
--- a/src/emqx_acl_internal.erl
+++ b/src/emqx_acl_internal.erl
@@ -46,18 +46,24 @@ all_rules() ->
 -spec(init([File :: string()]) -> {ok, #{}}).
 init([File]) ->
     _ = emqx_tables:new(?ACL_RULE_TAB, [set, public, {read_concurrency, true}]),
-    true = load_rules_from_file(File),
+    ok = load_rules_from_file(File),
     {ok, #{acl_file => File}}.
 
 load_rules_from_file(AclFile) ->
-    {ok, Terms} = file:consult(AclFile),
-    Rules = [emqx_access_rule:compile(Term) || Term <- Terms],
-    lists:foreach(fun(PubSub) ->
-        ets:insert(?ACL_RULE_TAB, {PubSub,
-            lists:filter(fun(Rule) -> filter(PubSub, Rule) end, Rules)})
-        end, [publish, subscribe]),
-    ets:insert(?ACL_RULE_TAB, {all_rules, Terms}).
-
+    case file:consult(AclFile) of
+        {ok, Terms} ->
+            Rules = [emqx_access_rule:compile(Term) || Term <- Terms],
+            lists:foreach(fun(PubSub) ->
+                ets:insert(?ACL_RULE_TAB, {PubSub,
+                    lists:filter(fun(Rule) -> filter(PubSub, Rule) end, Rules)})
+                end, [publish, subscribe]),
+            ets:insert(?ACL_RULE_TAB, {all_rules, Terms}),
+            ok;
+        {error, Reason} ->
+            emqx_logger:error("[ACL_INTERNAL] Consult failed: ~p", [Reason]),
+            {error, Reason}
+    end.
+    
 filter(_PubSub, {allow, all}) ->
     true;
 filter(_PubSub, {deny, all}) ->
@@ -100,9 +106,11 @@ match(Credentials, Topic, [Rule|Rules]) ->
 -spec(reload_acl(state()) -> ok | {error, term()}).
 reload_acl(#{acl_file := AclFile}) ->
     case catch load_rules_from_file(AclFile) of
-        true ->
+        ok ->
             emqx_logger:info("Reload acl_file ~s successfully", [AclFile]),
             ok;
+        {error, Error} ->
+            {error, Error};
         {'EXIT', Error} ->
             {error, Error}
     end.