Improve validation of second-level options

5 files changed, 143 insertions, 121 deletions

diff --git a/src/gen_mod.erl b/src/gen_mod.erl
index 783e823ef..fdfd94f2a 100644
--- a/src/gen_mod.erl
+++ b/src/gen_mod.erl
@@ -441,7 +441,7 @@ get_opt_host(Host, Opts, Default) ->
     Val = get_opt(host, Opts, Default),
     ejabberd_regexp:greplace(Val, <<"@HOST@">>, Host).
 
--spec get_validators(binary(), module(), opts()) -> {ok, [{atom(), check_fun()}]} | undef.
+-spec get_validators(binary(), module(), opts()) -> dict:dict() | undef.
 get_validators(Host, Module, Opts) ->
     try Module:mod_opt_type('') of
 	L ->
@@ -453,23 +453,29 @@ get_validators(Host, Module, Opts) ->
 			   true -> [ram_db_mod(Host, Opts, Module)];
 			   false -> []
 		       end,
-	    {ok, dict:to_list(
-		   lists:foldl(
-		     fun(Mod, D) ->
-			     try Mod:mod_opt_type('') of
-				 Os ->
-				     lists:foldl(
-				       fun({Opt, SubOpt} = O, Acc) ->
-					       F = Mod:mod_opt_type(O),
-					       dict:append(Opt, {SubOpt, F}, Acc);
-					  (O, Acc) ->
-					       F = Mod:mod_opt_type(O),
-					       dict:store(O, F, Acc)
-				       end, D, Os)
-			     catch _:undef ->
-				     D
-			     end
-		     end, dict:new(), [Module|SubMods1 ++ SubMods2]))}
+	    lists:foldl(
+	      fun(Mod, D) ->
+		      try Mod:mod_opt_type('') of
+			  Os ->
+			      lists:foldl(
+				fun({Opt, SubOpt} = O, Acc) ->
+					SubF = Mod:mod_opt_type(O),
+					F = case Mod:mod_opt_type(Opt) of
+						F1 when is_function(F1) ->
+						    F1;
+						_ ->
+						    fun(X) -> X end
+					    end,
+					dict:append_list(
+					  Opt, [F, {SubOpt, [SubF]}], Acc);
+				   (O, Acc) ->
+					F = Mod:mod_opt_type(O),
+					dict:store(O, [F], Acc)
+				end, D, Os)
+		      catch _:undef ->
+			      D
+		      end
+	      end, dict:new(), [Module|SubMods1 ++ SubMods2])
     catch _:undef ->
 	    ?WARNING_MSG("module '~s' doesn't export mod_opt_type/1",
 			 [Module]),
@@ -479,26 +485,30 @@ get_validators(Host, Module, Opts) ->
 -spec validate_opts(binary(), module(), opts()) -> opts().
 validate_opts(Host, Module, Opts) ->
     case get_validators(Host, Module, Opts) of
-	{ok, Validators} ->
-	    validate_opts(Host, Module, Opts, Validators);
 	undef ->
-	    Opts
+	    Opts;
+	Validators ->
+	    validate_opts(Host, Module, Opts, dict:to_list(Validators))
     end.
 
 validate_opts(Host, Module, Opts, Validators) when is_list(Opts) ->
     lists:flatmap(
       fun({Opt, Val}) when is_atom(Opt) ->
 	      case lists:keyfind(Opt, 1, Validators) of
-		  {_, VFun} when is_function(VFun) ->
-		      validate_opt(Module, Opt, Val, VFun);
-		  {_, SubValidators} ->
-		      try validate_opts(Host, Module, Val, SubValidators) of
-			  SubOpts -> [{Opt, SubOpts}]
-		      catch _:bad_option ->
-			      ?ERROR_MSG("ignoring invalid value '~p' for "
-					 "option '~s' of module '~s'",
-					 [Val, Opt, Module]),
-			      []
+		  {_, L} ->
+		      case lists:partition(fun is_function/1, L) of
+			  {[VFun|_], []} ->
+			      validate_opt(Module, Opt, Val, VFun);
+			  {[VFun|_], SubValidators} ->
+			      try validate_opts(Host, Module, Val, SubValidators) of
+				  SubOpts ->
+				      validate_opt(Module, Opt, SubOpts, VFun)
+			      catch _:bad_option ->
+				      ?ERROR_MSG("ignoring invalid value '~p' for "
+						 "option '~s' of module '~s'",
+						 [Val, Opt, Module]),
+				      []
+			      end
 		      end;
 		  false ->
 		      ?ERROR_MSG("unknown option '~s' for module '~s' will be"
diff --git a/src/mod_muc.erl b/src/mod_muc.erl
index 56616b927..69fc2d0dc 100644
--- a/src/mod_muc.erl
+++ b/src/mod_muc.erl
@@ -337,59 +337,9 @@ init_state(Host, Opts) ->
     AccessPersistent = gen_mod:get_opt(access_persistent, Opts, all),
     HistorySize = gen_mod:get_opt(history_size, Opts, 20),
     MaxRoomsDiscoItems = gen_mod:get_opt(max_rooms_discoitems, Opts, 100),
-    DefRoomOpts1 = gen_mod:get_opt(default_room_options, Opts, []),
+    DefRoomOpts = gen_mod:get_opt(default_room_options, Opts, []),
     QueueType = gen_mod:get_opt(queue_type, Opts,
 				ejabberd_config:default_queue_type(Host)),
-    DefRoomOpts =
-	lists:flatmap(
-	  fun({Opt, Val}) ->
-		  Bool = fun(B) when is_boolean(B) -> B end,
-		  VFun = case Opt of
-			     allow_change_subj -> Bool;
-			     allow_private_messages -> Bool;
-			     allow_query_users -> Bool;
-			     allow_user_invites -> Bool;
-			     allow_visitor_nickchange -> Bool;
-			     allow_visitor_status -> Bool;
-			     anonymous -> Bool;
-			     captcha_protected -> Bool;
-			     logging -> Bool;
-			     members_by_default -> Bool;
-			     members_only -> Bool;
-			     moderated -> Bool;
-			     password_protected -> Bool;
-			     persistent -> Bool;
-			     public -> Bool;
-			     public_list -> Bool;
-			     mam -> Bool;
-			     allow_subscription -> Bool;
-			     password -> fun iolist_to_binary/1;
-			     title -> fun iolist_to_binary/1;
-			     allow_private_messages_from_visitors ->
-				 fun(anyone) -> anyone;
-				    (moderators) -> moderators;
-				    (nobody) -> nobody
-				 end;
-			     max_users ->
-				 fun(I) when is_integer(I), I > 0 -> I end;
-                             presence_broadcast ->
-                                 fun(L) ->
-                                         lists:map(
-                                           fun(moderator) -> moderator;
-                                              (participant) -> participant;
-                                              (visitor) -> visitor
-                                           end, L)
-                                 end;
-			     _ ->
-				 ?ERROR_MSG("unknown option ~p with value ~p",
-					    [Opt, Val]),
-				 fun(_) -> undefined end
-			 end,
-		  case ejabberd_config:prepare_opt_val(Opt, Val, VFun, undefined) of
-		      undefined -> [];
-		      NewVal -> [{Opt, NewVal}]
-		  end
-	  end, DefRoomOpts1),
     RoomShaper = gen_mod:get_opt(room_shaper, Opts, none),
     #state{host = MyHost,
 	   server_host = Host,
@@ -897,8 +847,6 @@ mod_opt_type(access_persistent) ->
     fun acl:access_rules_validator/1;
 mod_opt_type(db_type) -> fun(T) -> ejabberd_config:v_db(?MODULE, T) end;
 mod_opt_type(ram_db_type) -> fun(T) -> ejabberd_config:v_db(?MODULE, T) end;
-mod_opt_type(default_room_options) ->
-    fun (L) when is_list(L) -> L end;
 mod_opt_type(history_size) ->
     fun (I) when is_integer(I), I >= 0 -> I end;
 mod_opt_type(host) -> fun iolist_to_binary/1;
@@ -938,11 +886,89 @@ mod_opt_type(user_presence_shaper) ->
     fun (A) when is_atom(A) -> A end;
 mod_opt_type(queue_type) ->
     fun(ram) -> ram; (file) -> file end;
+mod_opt_type({default_room_options, allow_change_subj}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, allow_private_messages}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, allow_query_users}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, allow_user_invites}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, allow_visitor_nickchange}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, allow_visitor_status}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, anonymous}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, captcha_protected}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, logging}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, members_by_default}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, members_only}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, moderated}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, password_protected}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, persistent}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, public}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, public_list}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, mam}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, allow_subscription}) ->
+    fun(B) when is_boolean(B) -> B end;
+mod_opt_type({default_room_options, password}) ->
+    fun iolist_to_binary/1;
+mod_opt_type({default_room_options, title}) ->
+    fun iolist_to_binary/1;
+mod_opt_type({default_room_options, allow_private_messages_from_visitors}) ->
+    fun(anyone) -> anyone;
+       (moderators) -> moderators;
+       (nobody) -> nobody
+    end;
+mod_opt_type({default_room_options, max_users}) ->
+    fun(I) when is_integer(I), I > 0 -> I end;
+mod_opt_type({default_room_options, presence_broadcast}) ->
+    fun(L) ->
+	    lists:map(
+	      fun(moderator) -> moderator;
+		 (participant) -> participant;
+		 (visitor) -> visitor
+	      end, L)
+    end;
 mod_opt_type(_) ->
     [access, access_admin, access_create, access_persistent,
-     db_type, ram_db_type, default_room_options, history_size, host,
+     db_type, ram_db_type, history_size, host,
      max_room_desc, max_room_id, max_room_name,
      max_rooms_discoitems, max_user_conferences, max_users,
      max_users_admin_threshold, max_users_presence,
      min_message_interval, min_presence_interval, queue_type,
-     regexp_room_id, room_shaper, user_message_shaper, user_presence_shaper].
+     regexp_room_id, room_shaper, user_message_shaper, user_presence_shaper,
+     {default_room_options, allow_change_subj},
+     {default_room_options, allow_private_messages},
+     {default_room_options, allow_query_users},
+     {default_room_options, allow_user_invites},
+     {default_room_options, allow_visitor_nickchange},
+     {default_room_options, allow_visitor_status},
+     {default_room_options, anonymous},
+     {default_room_options, captcha_protected},
+     {default_room_options, logging},
+     {default_room_options, members_by_default},
+     {default_room_options, members_only},
+     {default_room_options, moderated},
+     {default_room_options, password_protected},
+     {default_room_options, persistent},
+     {default_room_options, public},
+     {default_room_options, public_list},
+     {default_room_options, mam},
+     {default_room_options, allow_subscription},
+     {default_room_options, password},
+     {default_room_options, title},
+     {default_room_options, allow_private_messages_from_visitors},
+     {default_room_options, max_users},
+     {default_room_options, presence_broadcast}].
diff --git a/src/mod_muc_log.erl b/src/mod_muc_log.erl
index 820cbd3bd..45ee645fc 100644
--- a/src/mod_muc_log.erl
+++ b/src/mod_muc_log.erl
@@ -1185,11 +1185,13 @@ mod_opt_type(file_format) ->
     end;
 mod_opt_type(file_permissions) ->
     fun (SubOpts) ->
-	    F = fun ({mode, Mode}, {_M, G}) -> {Mode, G};
-		    ({group, Group}, {M, _G}) -> {M, Group}
-		end,
-	    lists:foldl(F, {644, 33}, SubOpts)
+	    {proplists:get_value(mode, SubOpts, 644),
+	     proplists:get_value(group, SubOpts, 33)}
     end;
+mod_opt_type({file_permissions, mode}) ->
+    fun(I) when is_integer(I), I>=0 -> I end;
+mod_opt_type({file_permissions, group}) ->
+    fun(I) when is_integer(I), I>=0 -> I end;
 mod_opt_type(outdir) -> fun iolist_to_binary/1;
 mod_opt_type(spam_prevention) ->
     fun (B) when is_boolean(B) -> B end;
@@ -1203,5 +1205,5 @@ mod_opt_type(top_link) ->
     end;
 mod_opt_type(_) ->
     [access_log, cssfile, dirname, dirtype, file_format,
-     file_permissions, outdir, spam_prevention, timezone,
-     top_link].
+     {file_permissions, mode}, {file_permissions, group},
+     outdir, spam_prevention, timezone, top_link].
diff --git a/src/mod_privilege.erl b/src/mod_privilege.erl
index e2a62adb0..6ba061a53 100644
--- a/src/mod_privilege.erl
+++ b/src/mod_privilege.erl
@@ -58,31 +58,12 @@ stop(Host) ->
 reload(_Host, _NewOpts, _OldOpts) ->
     ok.
 
-mod_opt_type(roster) ->
-    fun(Props) ->
-	    lists:map(
-	      fun({both, ACL}) -> {both, acl:access_rules_validator(ACL)};
-		 ({get, ACL}) -> {get, acl:access_rules_validator(ACL)};
-		 ({set, ACL}) -> {set, acl:access_rules_validator(ACL)}
-	      end, Props)
-    end;
-mod_opt_type(message) ->
-    fun(Props) ->
-	    lists:map(
-	      fun({outgoing, ACL}) -> {outgoing, acl:access_rules_validator(ACL)}
-	      end, Props)
-    end;
-mod_opt_type(presence) ->
-    fun(Props) ->
-	    lists:map(
-	      fun({managed_entity, ACL}) ->
-		      {managed_entity, acl:access_rules_validator(ACL)};
-		 ({roster, ACL}) ->
-		      {roster, acl:access_rules_validator(ACL)}
-	      end, Props)
-    end;
+mod_opt_type({roster, _}) -> fun acl:access_rules_validator/1;
+mod_opt_type({message, _}) -> fun acl:access_rules_validator/1;
+mod_opt_type({presence, _}) -> fun acl:access_rules_validator/1;
 mod_opt_type(_) ->
-    [roster, message, presence].
+    [{roster, both}, {roster, get}, {roster, set},
+     {message, outgoing}, {presence, managed_entity}, {presence, roster}].
 
 depends(_, _) ->
     [].
diff --git a/src/mod_register.erl b/src/mod_register.erl
index 1c4de1301..893a16c7a 100644
--- a/src/mod_register.erl
+++ b/src/mod_register.erl
@@ -599,15 +599,18 @@ mod_opt_type(registration_watchers) ->
 	    [jid:decode(iolist_to_binary(S)) || S <- Ss]
     end;
 mod_opt_type(welcome_message) ->
-    fun (Opts) ->
-	    S = proplists:get_value(subject, Opts, <<>>),
-	    B = proplists:get_value(body, Opts, <<>>),
-	    {iolist_to_binary(S), iolist_to_binary(B)}
+    fun(L) ->
+	    {proplists:get_value(subject, L, <<"">>),
+	     proplists:get_value(body, L, <<"">>)}
     end;
+mod_opt_type({welcome_message, subject}) ->
+    fun iolist_to_binary/1;
+mod_opt_type({welcome_message, body}) ->
+    fun iolist_to_binary/1;
 mod_opt_type(_) ->
     [access, access_from, captcha_protected, ip_access,
      iqdisc, password_strength, registration_watchers,
-     welcome_message].
+     {welcome_message, subject}, {welcome_message, body}].
 
 opt_type(registration_timeout) ->
     fun (TO) when is_integer(TO), TO > 0 -> TO;