-
A term passed as the third argument to the &transport_start; function of
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()}
--
cgit v1.2.3
From b6a38ba05bdd437adcb7e192c6ab0c2ca0718f76 Mon Sep 17 00:00:00 2001
From: Anders Svensson
Date: Fri, 22 Mar 2013 18:42:32 +0100
Subject: 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.
---
lib/diameter/src/base/diameter_config.erl | 71 ++++++++++++++++++++++++++---
lib/diameter/src/base/diameter_peer_fsm.erl | 4 +-
lib/diameter/src/base/diameter_watchdog.erl | 4 +-
3 files changed, 67 insertions(+), 12 deletions(-)
(limited to 'lib')
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
--
cgit v1.2.3
From 56c0af010d6da861b8e8675ba5309381d926c67e Mon Sep 17 00:00:00 2001
From: Anders Svensson
Date: Tue, 26 Mar 2013 09:51:59 +0100
Subject: Deal with config errors detected at transport start less brutally
Crashing watchdog and peer_fsm processes was somewhat unseemly. Emit an
error report and die silently instead.
---
lib/diameter/src/base/diameter_peer_fsm.erl | 25 +++++++++++++++++++++----
lib/diameter/src/base/diameter_watchdog.erl | 3 ++-
2 files changed, 23 insertions(+), 5 deletions(-)
(limited to 'lib')
diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl
index 3b8c49b7e8..bee3e507fd 100644
--- a/lib/diameter/src/base/diameter_peer_fsm.erl
+++ b/lib/diameter/src/base/diameter_peer_fsm.erl
@@ -844,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
@@ -905,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}).
@@ -985,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 3cbf91c574..41c493ff20 100644
--- a/lib/diameter/src/base/diameter_watchdog.erl
+++ b/lib/diameter/src/base/diameter_watchdog.erl
@@ -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
--
cgit v1.2.3
From 78161a2ae3f494e41883b8cf2ddf6bf2ab0b43d5 Mon Sep 17 00:00:00 2001
From: Anders Svensson
Date: Mon, 25 Mar 2013 19:15:43 +0100
Subject: Add config suite
To verify return values from diameter:start_service/2 and
diameter:add_transport/2 when passing various config.
---
lib/diameter/test/diameter_config_SUITE.erl | 261 ++++++++++++++++++++++++++++
lib/diameter/test/modules.mk | 1 +
2 files changed, 262 insertions(+)
create mode 100644 lib/diameter/test/diameter_config_SUITE.erl
(limited to 'lib')
diff --git a/lib/diameter/test/diameter_config_SUITE.erl b/lib/diameter/test/diameter_config_SUITE.erl
new file mode 100644
index 0000000000..47def9c8c9
--- /dev/null
+++ b/lib/diameter/test/diameter_config_SUITE.erl
@@ -0,0 +1,261 @@
+%% coding: utf-8
+%%
+%% %CopyrightBegin%
+%%
+%% Copyright Ericsson AB 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
+%% compliance with the License. You should have received a copy of the
+%% Erlang Public License along with this software. If not, it can be
+%% retrieved online at http://www.erlang.org/.
+%%
+%% Software distributed under the License is distributed on an "AS IS"
+%% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See
+%% the License for the specific language governing rights and limitations
+%% under the License.
+%%
+%% %CopyrightEnd%
+%%
+
+%%
+%% Test service and transport config. In particular, of the detection
+%% of config errors.
+%%
+
+-module(diameter_config_SUITE).
+
+-export([suite/0,
+ all/0]).
+
+%% testcases
+-export([start/1,
+ start_service/1,
+ add_transport/1,
+ stop/1]).
+
+-define(util, diameter_util).
+
+%% Lists of {Key, GoodConfigList, BadConfigList} with which to
+%% configure.
+
+-define(SERVICE_CONFIG,
+ [{application,
+ [[[{dictionary, diameter_gen_base_rfc6733},
+ {module, ?MODULE}]]
+ | [[[{dictionary, D},
+ {module, M},
+ {alias, A},
+ {state, S},
+ {answer_errors, AE},
+ {request_errors, RE},
+ {call_mutates_state, C}]]
+ || D <- [diameter_gen_base_rfc3588, diameter_gen_base_rfc6733],
+ M <- [?MODULE, [?MODULE, now()]],
+ A <- [0, common, make_ref()],
+ S <- [[], make_ref()],
+ AE <- [report, callback, discard],
+ RE <- [answer_3xxx, answer, callback],
+ C <- [true, false]]],
+ [[x],
+ [[]],
+ [[{dictionary, diameter_gen_base_rfc3588}]],
+ [[{module, ?MODULE}]]
+ | [[[{dictionary, diameter_gen_base_rfc6733},
+ {module, ?MODULE},
+ {K,x}]]
+ || K <- [answer_errors,
+ request_errors,
+ call_mutates_state]]]},
+ {restrict_connections,
+ [[false], [node], [nodes], [[node(), node()]]],
+ []},
+ {sequence,
+ [[{0,32}], [{1,31}]],
+ [[{2,31}]]},
+ {share_peers,
+ [[true],
+ [false],
+ [[node()]]],
+ [[x]]},
+ {use_shared_peers,
+ [[true],
+ [false],
+ [[node(), node()]]],
+ [[x]]}]).
+
+-define(TRANSPORT_CONFIG,
+ [{transport_module,
+ [[?MODULE]],
+ [[[?MODULE]]]},
+ {transport_config,
+ [[{}, 3000],
+ [{}, infinity]],
+ [[{}, x]]},
+ {applications,
+ [[[1, a, [x]]]],
+ [[x]]},
+ {capabilities,
+ [[[{'Origin-Host', "diameter.erlang.org"}]],
+ [[{'Origin-Realm', "erlang.org"}]]]
+ ++ [[[{'Host-IP-Address', L}]]
+ || L <- [[{127,0,0,1}],
+ ["127.0.0.1"],
+ ["127.0.0.1", "FFFF::1", "::1", {1,2,3,4,5,6,7,8}]]]
+ ++ [[[{'Product-Name', N}]]
+ || N <- [["Product", $-, ["Name"]],
+ "Norðurálfa",
+ "ᚠᚢᚦᚨᚱᚲ"]]
+ ++ [[[{K,V}]]
+ || K <- ['Vendor-Id',
+ 'Origin-State-Id',
+ 'Firmware-Revision'],
+ V <- [0, 256, 16#FFFF]]
+ ++ [[[{K,V}]]
+ || K <- ['Supported-Vendor-Id',
+ 'Auth-Application-Id',
+ 'Acct-Application-Id',
+ 'Inband-Security-Id'],
+ V <- [[17], [0, 256, 16#FFFF]]]
+ ++ [[[{'Vendor-Specific-Application-Id',
+ [[{'Vendor-Id', V},
+ {'Auth-Application-Id', [0]},
+ {'Acct-Application-Id', [4]}]]}]]
+ || V <- [1, [1]]],
+ [[x], [[{'Origin-Host', "ᚠᚢᚦᚨᚱᚲ"}]]]
+ ++ [[[{'Host-IP-Address', A}]]
+ || A <- [{127,0,0,1}]]
+ ++ [[[{'Product-Name', N}]]
+ || N <- [x, 1]]
+ ++ [[[{K,V}]]
+ || K <- ['Vendor-Id',
+ 'Origin-State-Id',
+ 'Firmware-Revision'],
+ V <- [x, [0], -1, 1 bsl 32]]
+ ++ [[[{K,V}]]
+ || K <- ['Supported-Vendor-Id',
+ 'Auth-Application-Id',
+ 'Acct-Application-Id',
+ 'Inband-Security-Id'],
+ V <- [x, 17, [-1], [1 bsl 32]]]
+ ++ [[[{'Vendor-Specific-Application-Id', V}]]
+ || V <- [x,
+ [[{'Vendor-Id', 1 bsl 32}]],
+ [[{'Auth-Application-Id', 1}]]]]},
+ {capabilities_cb,
+ [[x]],
+ []},
+ {capx_timeout,
+ [[3000]],
+ [[{?MODULE, tmo, []}]]},
+ {disconnect_cb,
+ [[x]],
+ []},
+ {length_errors,
+ [[exit], [handle], [discard]],
+ [[x]]},
+ {reconnect_timer,
+ [[3000]],
+ [[infinity]]},
+ {watchdog_timer,
+ [[3000],
+ [{?MODULE, tmo, []}]],
+ [[infinity],
+ [-1]]},
+ {watchdog_config,
+ [[[{okay, 0}, {suspect, 0}]],
+ [[{okay, 1}]],
+ [[{suspect, 2}]]],
+ [[x],
+ [[{open, 0}]]]}]).
+
+%% ===========================================================================
+
+suite() ->
+ [{timetrap, {seconds, 60}}].
+
+all() ->
+ [start,
+ start_service,
+ add_transport,
+ stop].
+
+%% ===========================================================================
+
+start(_) ->
+ ok = diameter:start().
+
+start_service(T)
+ when is_tuple(T) ->
+ do(fun start/3, T);
+
+start_service(_) ->
+ [] = ?util:run([{?MODULE, start_service, [T]}
+ || T <- [lists:keyfind(capabilities, 1, ?TRANSPORT_CONFIG)
+ | ?SERVICE_CONFIG]]).
+
+add_transport(T)
+ when is_tuple(T) ->
+ do(fun add/3, T);
+
+add_transport(_) ->
+ [] = ?util:run([{?MODULE, add_transport, [T]}
+ || T <- ?TRANSPORT_CONFIG]).
+
+stop(_) ->
+ ok = diameter:stop().
+
+%% ===========================================================================
+
+%% do/2
+
+do(F, {Key, Good, Bad}) ->
+ F(Key, Good, Bad).
+
+%% add/3
+
+add(Key, Good, Bad) ->
+ {[],[]} = {[{Vs,T} || Vs <- Good,
+ T <- [add(Key, Vs)],
+ [T] /= [T || {ok,_} <- [T]]],
+ [{Vs,T} || Vs <- Bad,
+ T <- [add(Key, Vs)],
+ [T] /= [T || {error,_} <- [T]]]}.
+
+add(Key, Vs) ->
+ T = list_to_tuple([Key | Vs]),
+ diameter:add_transport(make_ref(), {connect, [T]}).
+
+%% start/3
+
+start(Key, Good, Bad) ->
+ {[],[]} = {[{Vs,T} || Vs <- Good,
+ T <- [start(Key, Vs)],
+ T /= ok],
+ [{Vs,T} || Vs <- Bad,
+ T <- [start(Key, Vs)],
+ [T] /= [T || {error,_} <- [T]]]}.
+
+start(capabilities = K, [Vs]) ->
+ if is_list(Vs) ->
+ start(make_ref(), Vs ++ apps(K));
+ true ->
+ {error, Vs}
+ end;
+
+start(Key, Vs)
+ when is_atom(Key) ->
+ start(make_ref(), [list_to_tuple([Key | Vs]) | apps(Key)]);
+
+start(SvcName, Opts) ->
+ try
+ diameter:start_service(SvcName, Opts)
+ after
+ diameter:stop_service(SvcName)
+ end.
+
+apps(application) ->
+ [];
+apps(_) ->
+ [{application, [{dictionary, diameter_gen_base_rfc6733},
+ {module, ?MODULE}]}].
diff --git a/lib/diameter/test/modules.mk b/lib/diameter/test/modules.mk
index beff588a02..f6a49d36ab 100644
--- a/lib/diameter/test/modules.mk
+++ b/lib/diameter/test/modules.mk
@@ -29,6 +29,7 @@ MODULES = \
diameter_capx_SUITE \
diameter_codec_SUITE \
diameter_codec_test \
+ diameter_config_SUITE \
diameter_compiler_SUITE \
diameter_dict_SUITE \
diameter_distribution_SUITE \
--
cgit v1.2.3
From 49040f8be249501331abc3830dfe91344fd0f988 Mon Sep 17 00:00:00 2001
From: Anders Svensson
Date: Tue, 26 Mar 2013 13:39:10 +0100
Subject: Fix faulty sequence validation
The validation of {sequence, {H,N}} incorrectly checked that H was an
N-bit integer, instead of the intended 32-N.
---
lib/diameter/src/base/diameter_config.erl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
(limited to 'lib')
diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl
index 899c909ebe..1282930145 100644
--- a/lib/diameter/src/base/diameter_config.erl
+++ b/lib/diameter/src/base/diameter_config.erl
@@ -693,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(_) ->
--
cgit v1.2.3