aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnders Svensson <[email protected]>2013-03-22 18:42:32 +0100
committerAnders Svensson <[email protected]>2013-03-26 17:13:29 +0100
commitb6a38ba05bdd437adcb7e192c6ab0c2ca0718f76 (patch)
tree1bd47885515842eb468216066b7caaae26ee6ea7
parentbcbaf5e1bcc47cb31782a6b36bc8a538b209ebc1 (diff)
downloadotp-b6a38ba05bdd437adcb7e192c6ab0c2ca0718f76.tar.gz
otp-b6a38ba05bdd437adcb7e192c6ab0c2ca0718f76.tar.bz2
otp-b6a38ba05bdd437adcb7e192c6ab0c2ca0718f76.zip
Move most transport_opt() validation into diameter_config
Faulty configuration was previously passed directly on to watchdog and peer_fsm processes, diameter:add_transport/2 happily returning ok and the error resulting on failure of watchdog and/or peer_fsm processes. Now check for errors before getting this far, returning {error, Reason} from diameter:add_transport/2 when one is detected. There are still some errors that can only be detected after transport start (eg. a misbehaving callback) but most will be caught early.
-rw-r--r--lib/diameter/src/base/diameter_config.erl71
-rw-r--r--lib/diameter/src/base/diameter_peer_fsm.erl4
-rw-r--r--lib/diameter/src/base/diameter_watchdog.erl4
3 files changed, 67 insertions, 12 deletions
diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl
index 3a2e0d2140..899c909ebe 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} ->
diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl
index 66342f7b62..3b8c49b7e8 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.)
diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl
index 82ca603cf3..3cbf91c574 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