Merge branch 'anders/diameter/app_not_configured/OTP-10972' into maint

* anders/diameter/app_not_configured/OTP-10972:
  Fix faulty sequence validation
  Add config suite
  Deal with config errors detected at transport start less brutally
  Move most transport_opt() validation into diameter_config
  Minor doc/spec fix
  Minor diameter_lib cleanup

5 files changed, 204 insertions, 152 deletions

diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl
index 490a1fa8aa..57730cad61 100644
--- a/lib/diameter/src/base/diameter.erl
+++ b/lib/diameter/src/base/diameter.erl
@@ -335,7 +335,7 @@ call(SvcName, App, Message) ->
 -type transport_opt()
    :: {transport_module, atom()}
     | {transport_config, any()}
-    | {transport_config, any(), non_neg_integer() | infinity}
+    | {transport_config, any(), 'Unsigned32'() | infinity}
     | {applications, [app_alias()]}
     | {capabilities, [capability()]}
     | {capabilities_cb, evaluable()}
diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl
index 3a2e0d2140..1282930145 100644
--- a/lib/diameter/src/base/diameter_config.erl
+++ b/lib/diameter/src/base/diameter_config.erl
@@ -103,6 +103,10 @@
 %% Time to lay low before restarting a dead service.
 -define(RESTART_SLEEP, 2000).
 
+%% Test for a valid timeout.
+-define(IS_UINT32(N),
+        is_integer(N) andalso 0 =< N andalso 0 == N bsr 32).
+
 %% A minimal diameter_caps for checking for valid capabilities values.
 -define(EXAMPLE_CAPS,
         #diameter_caps{origin_host = "TheHost",
@@ -490,13 +494,11 @@ stop(SvcName) ->
 %% has many.
 
 add(SvcName, Type, Opts) ->
-    %% Ensure usable capabilities. diameter_service:merge_service/2
-    %% depends on this.
-    lists:foreach(fun(Os) ->
-                          is_list(Os) orelse ?THROW({capabilities, Os}),
-                          ok = encode_CER(Os)
-                  end,
-                  [Os || {capabilities, Os} <- Opts, is_list(Os)]),
+    %% Ensure acceptable transport options. This won't catch all
+    %% possible errors (faulty callbacks for example) but it catches
+    %% many. diameter_service:merge_service/2 depends on usable
+    %% capabilities for example.
+    ok = transport_opts(Opts),
 
     Ref = make_ref(),
     T = {Ref, Type, Opts},
@@ -514,6 +516,61 @@ add(SvcName, Type, Opts) ->
             No
     end.
 
+transport_opts(Opts) ->
+    lists:foreach(fun(T) -> opt(T) orelse ?THROW({invalid, T}) end, Opts).
+
+opt({transport_module, M}) ->
+    is_atom(M);
+
+opt({transport_config, _, Tmo}) ->
+    ?IS_UINT32(Tmo) orelse Tmo == infinity;
+
+opt({applications, As}) ->
+    is_list(As);
+
+opt({capabilities, Os}) ->
+    is_list(Os) andalso ok == encode_CER(Os);
+
+opt({capx_timeout, Tmo}) ->
+    ?IS_UINT32(Tmo);
+
+opt({length_errors, T}) ->
+    lists:member(T, [exit, handle, discard]);
+
+opt({reconnect_timer, Tmo}) ->
+    ?IS_UINT32(Tmo);
+
+opt({watchdog_timer, {M,F,A}})
+  when is_atom(M), is_atom(F), is_list(A) ->
+    true;
+opt({watchdog_timer, Tmo}) ->
+    ?IS_UINT32(Tmo);
+
+opt({watchdog_config, L}) ->
+    is_list(L) andalso lists:all(fun wdopt/1, L);
+
+%% Options that we can't validate.
+opt({K, _})
+  when K == transport_config;
+       K == capabilities_cb;
+       K == disconnect_cb;
+       K == private ->
+    true;
+
+%% Anything else, which is ignored by us. This makes options sensitive
+%% to spelling mistakes but arbitrary options are passed by some users
+%% as a way to identify transports. (That is, can't just do away with
+%% it.)
+opt(_) ->
+    true.
+
+wdopt({K,N}) ->
+    (K == okay orelse K == suspect) andalso is_integer(N) andalso 0 =< N;
+wdopt(_) ->
+    false.
+
+%% start_transport/2
+
 start_transport(SvcName, T) ->
     case diameter_service:start_transport(SvcName, T) of
         {ok, _Pid} ->
@@ -636,7 +693,7 @@ opt(K, _) ->
     ?THROW({value, K}).
 
 sequence({H,N} = T)
-  when 0 =< N, N =< 32, 0 =< H, 0 == H bsr N ->
+  when 0 =< N, N =< 32, 0 =< H, 0 == H bsr (32-N) ->
     T;
 
 sequence(_) ->
diff --git a/lib/diameter/src/base/diameter_lib.erl b/lib/diameter/src/base/diameter_lib.erl
index 362d593b24..44d81e2778 100644
--- a/lib/diameter/src/base/diameter_lib.erl
+++ b/lib/diameter/src/base/diameter_lib.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2010-2011. All Rights Reserved.
+%% Copyright Ericsson AB 2010-2013. All Rights Reserved.
 %%
 %% The contents of this file are subject to the Erlang Public License,
 %% Version 1.1, (the "License"); you may not use this file except in
@@ -19,77 +19,86 @@
 
 -module(diameter_lib).
 
--export([report/2, info_report/2,
+-export([info_report/2,
          error_report/2,
          warning_report/2,
          now_diff/1,
          time/1,
          eval/1,
-         ip4address/1,
-         ip6address/1,
          ipaddr/1,
          spawn_opts/2,
          wait/1,
          fold_tuple/3,
          log/4]).
 
--include("diameter_internal.hrl").
-
 %% ---------------------------------------------------------------------------
-%% # info_report(Reason, MFA)
-%%
-%% Input: Reason = Arbitrary term indicating the reason for the report.
-%%        MFA    = {Module, Function, Args} to report.
-%%
-%% Output:  true
+%% # info_report/2
 %% ---------------------------------------------------------------------------
 
-report(Reason, MFA) ->
-    info_report(Reason, MFA).
+-spec info_report(Reason :: term(), T :: term())
+   -> true.
 
-info_report(Reason, MFA) ->
-    report(fun error_logger:info_report/1, Reason, MFA),
+info_report(Reason, T) ->
+    report(fun error_logger:info_report/1, Reason, T),
     true.
 
-%%% ---------------------------------------------------------------------------
-%%% # error_report(Reason, MFA)
-%%% # warning_report(Reason, MFA)
-%%%
-%%% Output:  false
-%%% ---------------------------------------------------------------------------
+%% ---------------------------------------------------------------------------
+%% # error_report/2
+%% # warning_report/2
+%% ---------------------------------------------------------------------------
+
+-spec error_report(Reason :: term(), T :: term())
+   -> false.
+
+error_report(Reason, T) ->
+    report(fun error_logger:error_report/1, Reason, T).
 
-error_report(Reason, MFA) ->
-    report(fun error_logger:error_report/1, Reason, MFA).
+-spec warning_report(Reason :: term(), T :: term())
+   -> false.
 
-warning_report(Reason, MFA) ->
-    report(fun error_logger:warning_report/1, Reason, MFA).
+warning_report(Reason, T) ->
+    report(fun error_logger:warning_report/1, Reason, T).
 
-report(Fun, Reason, MFA) ->
-    Fun([{why, Reason}, {who, self()}, {what, MFA}]),
+report(Fun, Reason, T) ->
+    Fun([{why, Reason}, {who, self()}, {what, T}]),
     false.
 
-%%% ---------------------------------------------------------------------------
-%%% # now_diff(Time)
-%%%
-%%% Description: Return timer:now_diff(now(), Time) as an {H, M, S, MicroS}
-%%%              tuple instead of as integer microseconds.
-%%% ---------------------------------------------------------------------------
+%% ---------------------------------------------------------------------------
+%% # now_diff/1
+%% ---------------------------------------------------------------------------
+
+-spec now_diff(NowT)
+   -> {Hours, Mins, Secs, MicroSecs}
+ when NowT  :: {non_neg_integer(), 0..999999, 0..999999},
+      Hours :: non_neg_integer(),
+      Mins  :: 0..59,
+      Secs  :: 0..59,
+      MicroSecs :: 0..999999.
+
+%% Return timer:now_diff(now(), NowT) as an {H, M, S, MicroS} tuple
+%% instead of as integer microseconds.
 
 now_diff({_,_,_} = Time) ->
-    time(timer:now_diff(erlang:now(), Time)).
-
-%%% ---------------------------------------------------------------------------
-%%% # time(Time)
-%%%
-%%% Input:  Time = {MegaSec, Sec, MicroSec}
-%%%              | MicroSec
-%%%
-%%% Output: {H, M, S, MicroS}
-%%% ---------------------------------------------------------------------------
-
-time({_,_,_} = Time) ->  %% time of day
+    time(timer:now_diff(now(), Time)).
+
+%% ---------------------------------------------------------------------------
+%% # time/1
+%%
+%% Return an elapsed time as an {H, M, S, MicroS} tuple.
+%% ---------------------------------------------------------------------------
+
+-spec time(NowT | Diff)
+   -> {Hours, Mins, Secs, MicroSecs}
+ when NowT  :: {non_neg_integer(), 0..999999, 0..999999},
+      Diff  :: non_neg_integer(),
+      Hours :: non_neg_integer(),
+      Mins  :: 0..59,
+      Secs  :: 0..59,
+      MicroSecs :: 0..999999.
+
+time({_,_,_} = NowT) ->  %% time of day
     %% 24 hours = 24*60*60*1000000 = 86400000000 microsec
-    time(timer:now_diff(Time, {0,0,0}) rem 86400000000);
+    time(timer:now_diff(NowT, {0,0,0}) rem 86400000000);
 
 time(Micro) ->  %% elapsed time
     Seconds = Micro div 1000000,
@@ -98,9 +107,21 @@ time(Micro) ->  %% elapsed time
     S = Seconds rem 60,
     {H, M, S, Micro rem 1000000}.
 
-%%% ---------------------------------------------------------------------------
-%%% # eval(Func)
-%%% ---------------------------------------------------------------------------
+%% ---------------------------------------------------------------------------
+%% # eval/1
+%%
+%% Evaluate a function in various forms.
+%% ---------------------------------------------------------------------------
+
+-type f() :: {module(), atom(), list()}
+           | nonempty_maybe_improper_list(fun(), list())
+           | fun().
+
+-spec eval(Fun)
+   -> term()
+ when Fun :: f()
+           | {f()}
+           | nonempty_maybe_improper_list(f(), list()).
 
 eval({M,F,A}) ->
     apply(M,F,A);
@@ -120,66 +141,15 @@ eval({F}) ->
 eval(F) ->
     F().
 
-%%% ---------------------------------------------------------------------------
-%%% # ip4address(Addr)
-%%%
-%%% Input:  string()   (eg. "10.0.0.1")
-%%%         | list of integer()
-%%%         | tuple of integer()
-%%%
-%%% Output: {_,_,_,_} of integer
-%%%
-%%% Exceptions: error: {invalid_address, Addr, erlang:get_stacktrace()}
-%%% ---------------------------------------------------------------------------
-
-ip4address([_,_,_,_] = Addr) -> %% Length 4 string can't be an address.
-    ipaddr(list_to_tuple(Addr));
-
-%% Be brutal.
-ip4address(Addr) ->
-    try
-        {_,_,_,_} = ipaddr(Addr)
-    catch
-        error: _ ->
-            erlang:error({invalid_address, Addr, ?STACK})
-    end.
-
-%%% ---------------------------------------------------------------------------
-%%% # ip6address(Addr)
-%%%
-%%% Input:  string()   (eg. "1080::8:800:200C:417A")
-%%%         | list of integer()
-%%%         | tuple of integer()
-%%%
-%%% Output: {_,_,_,_,_,_,_,_} of integer
-%%%
-%%% Exceptions: error: {invalid_address, Addr, erlang:get_stacktrace()}
-%%% ---------------------------------------------------------------------------
-
-ip6address([_,_,_,_,_,_,_,_] = Addr) -> %% Length 8 string can't be an address.
-    ipaddr(list_to_tuple(Addr));
-
-%% Be brutal.
-ip6address(Addr) ->
-    try
-        {_,_,_,_,_,_,_,_} = ipaddr(Addr)
-    catch
-        error: _ ->
-            erlang:error({invalid_address, Addr, ?STACK})
-    end.
-
-%%% ---------------------------------------------------------------------------
-%%% # ipaddr(Addr)
-%%%
-%%% Input:  string() | tuple of integer()
-%%%
-%%% Output: {_,_,_,_} | {_,_,_,_,_,_,_,_}
-%%%
-%%% Exceptions: error: {invalid_address, erlang:get_stacktrace()}
-%%% ---------------------------------------------------------------------------
+%% ---------------------------------------------------------------------------
+%% # ipaddr/1
+%%
+%% Parse an IP address.
+%% ---------------------------------------------------------------------------
 
--spec ipaddr(string() | tuple())
-   -> inet:ip_address().
+-spec ipaddr([byte()] | tuple())
+   -> inet:ip_address()
+    | none().
 
 %% Don't convert lists of integers since a length 8 list like
 %% [$1,$0,$.,$0,$.,$0,$.,$1] is ambiguous: is it "10.0.0.1" or
@@ -193,7 +163,7 @@ ipaddr(Addr) ->
         ip(Addr)
     catch
         error: _ ->
-            erlang:error({invalid_address, ?STACK})
+            erlang:error({invalid_address, erlang:get_stacktrace()})
     end.
 
 %% Already a tuple: ensure non-negative integers of the right size.
@@ -210,11 +180,12 @@ ip(Addr) ->
     {ok, A} = inet_parse:address(Addr),  %% documented in inet(3)
     A.
 
-%%% ---------------------------------------------------------------------------
-%%% # spawn_opts(Type, Opts)
-%%% ---------------------------------------------------------------------------
+%% ---------------------------------------------------------------------------
+%% # spawn_opts/2
+%% ---------------------------------------------------------------------------
 
-%% TODO: config variables.
+-spec spawn_opts(server|worker, list())
+   -> list().
 
 spawn_opts(server, Opts) ->
     opts(75000, Opts);
@@ -224,24 +195,32 @@ spawn_opts(worker, Opts) ->
 opts(HeapSize, Opts) ->
     [{min_heap_size, HeapSize} | lists:keydelete(min_heap_size, 1, Opts)].
 
-%%% ---------------------------------------------------------------------------
-%%% # wait(MRefs)
-%%% ---------------------------------------------------------------------------
+%% ---------------------------------------------------------------------------
+%% # wait/1
+%% ---------------------------------------------------------------------------
+
+-spec wait([pid()])
+   -> ok.
 
 wait(L) ->
-    w([erlang:monitor(process, P) || P <- L]).
+    down([erlang:monitor(process, P) || P <- L]).
 
-w([]) ->
+down([]) ->
     ok;
-w(L) ->
-    receive
-        {'DOWN', MRef, process, _, _} ->
-            w(lists:delete(MRef, L))
-    end.
+down([MRef|T]) ->
+    receive {'DOWN', MRef, process, _, _} -> ok end,
+    down(T).
 
-%%% ---------------------------------------------------------------------------
-%%% # fold_tuple(N, T0, T)
-%%% ---------------------------------------------------------------------------
+%% ---------------------------------------------------------------------------
+%% # fold_tuple/3
+%% ---------------------------------------------------------------------------
+
+-spec fold_tuple(N, T0, T)
+   -> tuple()
+ when N  :: pos_integer(),
+      T0 :: tuple(),
+      T  :: tuple()
+          | undefined.
 
 %% Replace fields in T0 by those of T starting at index N, unless the
 %% new value is 'undefined'.
@@ -262,11 +241,11 @@ ft(undefined, {_, T}) ->
 ft(Value, {Idx, T}) ->
     setelement(Idx, T, Value).
 
-%%% ----------------------------------------------------------
-%%% # log(Slogan, Mod, Line, Details)
-%%%
-%%% Called to have something to trace on for happenings of interest.
-%%% ----------------------------------------------------------
+%% ---------------------------------------------------------------------------
+%% # log/4
+%%
+%% Called to have something to trace on for happenings of interest.
+%% ---------------------------------------------------------------------------
 
-log(_, _, _, _) ->
+log(_Slogan, _Mod, _Line, _Details) ->
     ok.
diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl
index 66342f7b62..bee3e507fd 100644
--- a/lib/diameter/src/base/diameter_peer_fsm.erl
+++ b/lib/diameter/src/base/diameter_peer_fsm.erl
@@ -198,6 +198,7 @@ i({Ack, WPid, {M, Ref} = T, Opts, {Mask,
     OnLengthErr = proplists:get_value(length_errors, Opts, exit),
     lists:member(OnLengthErr, [exit, handle, discard])
         orelse ?ERROR({invalid, {length_errors, OnLengthErr}}),
+    %% Error checking is for configuration added in old code.
 
     {TPid, Addrs} = start_transport(T, Rest, Svc),
 
@@ -212,9 +213,6 @@ i({Ack, WPid, {M, Ref} = T, Opts, {Mask,
 %% transports on the same service can use different local addresses.
 %% The local addresses are put into Host-IP-Address avps here when
 %% sending capabilities exchange messages.
-%%
-%% Invalid transport config may cause us to crash but note that the
-%% watchdog start (start/2) succeeds regardless.
 
 %% Wait for the caller to have a monitor to avoid a race with our
 %% death. (Since the exit reason is used in diameter_service.)
@@ -846,8 +844,12 @@ a('DPR', #diameter_caps{origin_host = {Host, _},
 %% recv_CER/2
 
 recv_CER(CER, #state{service = Svc, dictionary = Dict}) ->
-    {ok, T} = diameter_capx:recv_CER(CER, Svc, Dict),
-    T.
+    case diameter_capx:recv_CER(CER, Svc, Dict) of
+        {ok, T} ->
+            T;
+        {error, Reason} ->
+            close({'CER', CER, Svc, Dict, Reason})
+    end.
 
 %% handle_CEA/1
 
@@ -907,8 +909,12 @@ recv_CEA(#diameter_packet{header = #diameter_header{version
                           errors = []},
          #state{service = Svc,
                 dictionary = Dict}) ->
-    {ok, T} = diameter_capx:recv_CEA(CEA, Svc, Dict),
-    T;
+    case diameter_capx:recv_CEA(CEA, Svc, Dict) of
+        {ok, T} ->
+            T;
+        {error, Reason} ->
+            close({'CEA', CEA, Svc, Dict, Reason})
+    end;
 
 recv_CEA(Pkt, S) ->
     close({'CEA', caps(S), Pkt}).
@@ -987,8 +993,17 @@ capz(#diameter_caps{} = L, #diameter_caps{} = R) ->
 %% close/1
 
 close(Reason) ->
+    report(Reason),
     throw({?MODULE, close, Reason}).
 
+%% Could possibly log more here.
+report({M, _, _, _, _} = T)
+  when M == 'CER';
+       M == 'CEA' ->
+    diameter_lib:error_report(failure, T);
+report(_) ->
+    ok.
+
 %% dwa/1
 
 dwa(#diameter_caps{origin_host = OH,
diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl
index 82ca603cf3..41c493ff20 100644
--- a/lib/diameter/src/base/diameter_watchdog.erl
+++ b/lib/diameter/src/base/diameter_watchdog.erl
@@ -158,7 +158,7 @@ wait(Ref, Pid) ->
 config(Opts) ->
     Config = proplists:get_value(watchdog_config, Opts, []),
     is_list(Config) orelse config_error({watchdog_config, Config}),
-    lists:foldl(fun config/2, #config{}, Config).
+    lists:foldl(fun config/2, #config{}, Config).      %% ^ added in old code
 
 config({suspect, N}, Rec)
   when ?IS_NATURAL(N) ->
@@ -168,7 +168,7 @@ config({okay, N}, Rec)
   when ?IS_NATURAL(N) ->
     Rec#config{okay = N};
 
-config(T, _) ->
+config(T, _) ->  %% added in old code
     config_error(T).
 
 %% start/5
@@ -225,7 +225,8 @@ dict0(_, _, Acc) ->
     Acc.
 
 config_error(T) ->
-    ?ERROR({configuration_error, T}).
+    diameter_lib:error_report(configuration_error, T),
+    exit({shutdown, {configuration_error, T}}).
 
 %% handle_call/3