Fix handling of unknown options to diameter:start_service/2

{error, Reason} is now returned, instead of the options being ignored.

Note that diameter:add_transport/2 purposely ignores unknown options and
that the behaviour is documented. This is historic: some users depend on
it in order to store their own options for identifying transport config,
instead of using the reference returned by add_transport.

lib/diameter/src/base/diameter_config.erl

@@ -614,30 +614,38 @@ stop_transport(SvcName, Refs) ->
 %% make_config/2
 
 make_config(SvcName, Opts) ->
-    Apps = init_apps(Opts),
+    AppOpts = [T || {application, _} = T <- Opts],
+    Apps = init_apps(AppOpts),
+
     [] == Apps andalso ?THROW(no_apps),
 
     %% Use the fact that diameter_caps has the same field names as CER.
     Fields = ?BASE:'#info-'(diameter_base_CER) -- ['AVP'],
 
-    COpts = [T || {K,_} = T <- Opts, lists:member(K, Fields)],
-    Caps = make_caps(#diameter_caps{}, COpts),
+    CapOpts = [T || {K,_} = T <- Opts, lists:member(K, Fields)],
+    Caps = make_caps(#diameter_caps{}, CapOpts),
 
-    ok = encode_CER(COpts),
+    ok = encode_CER(CapOpts),
 
-    Os = split(Opts, fun opt/2, [{false, share_peers},
-                                 {false, use_shared_peers},
-                                 {false, monitor},
-                                 {?NOMASK, sequence},
-                                 {nodes, restrict_connections}]),
+    SvcOpts = make_opts((Opts -- AppOpts) -- CapOpts,
+                        [{false, share_peers},
+                         {false, use_shared_peers},
+                         {false, monitor},
+                         {?NOMASK, sequence},
+                         {nodes, restrict_connections}]),
 
     #service{name = SvcName,
              rec = #diameter_service{applications = Apps,
                                      capabilities = Caps},
-             options = Os}.
+             options = SvcOpts}.
+
+make_opts(Opts, Defs) ->
+    Known = [{K, get_opt(K, Opts, D)} || {D,K} <- Defs],
+    Unknown = Opts -- Known,
+
+    [] == Unknown orelse ?THROW({invalid, hd(Unknown)}),
 
-split(Opts, F, Defs) ->
-    [{K, F(K, get_opt(K, Opts, D))} || {D,K} <- Defs].
+    [{K, opt(K,V)} || {K,V} <- Known].
 
 opt(K, false = B)
   when K /= sequence ->
@@ -728,8 +736,8 @@ encode_CER(Opts) ->
 init_apps(Opts) ->
     lists:foldl(fun app_acc/2, [], lists:reverse(Opts)).
 
-app_acc({application, Opts}, Acc) ->
-    is_list(Opts) orelse ?THROW({application, Opts}),
+app_acc({application, Opts} = T, Acc) ->
+    is_list(Opts) orelse ?THROW(T),
 
     [Dict, Mod] = get_opt([dictionary, module], Opts),
     Alias = get_opt(alias, Opts, Dict),
@@ -745,9 +753,7 @@ app_acc({application, Opts}, Acc) ->
                    mutable = M,
                    options = [{answer_errors, A},
                               {request_errors, P}]}
-     | Acc];
-app_acc(_, Acc) ->
-    Acc.
+     | Acc].
 
 init_mod(#diameter_callback{} = R) ->
     init_mod([diameter_callback, R]);

lib/diameter/test/diameter_config_SUITE.erl

@@ -82,7 +82,11 @@
           [[true],
            [false],
            [[node(), node()]]],
-          [[x]]}]).
+          [[x]]},
+         {invalid_option,  %% invalid service options are rejected
+          [],
+          [[x],
+           [x,x]]}]).
 
 -define(TRANSPORT_CONFIG,
         [{transport_module,
@@ -167,7 +171,14 @@
            [[{okay, 1}]],
            [[{suspect, 2}]]],
           [[x],
-           [[{open, 0}]]]}]).
+           [[{open, 0}]]]},
+         {private,
+          [[x]],
+          []},
+         {invalid_option,  %% invalid transport options are silently ignored
+          [[x],
+           [x,x]],
+          []}]).
 
 %% ===========================================================================