From e693596e7d73ce438659e54b2e5cf72b2e7173f3 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 15 Aug 2017 14:24:50 +0200 Subject: Fix regexp match of accept tuple in diameter_tcp/sctp Matching of remote addresses when accepting connections in a listening transport was case-sensitive, causing the semantics to change as a consequence of 95ebfa0b, which made inet:ntoa/1 return lowercase. --- lib/diameter/src/base/diameter_peer.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/diameter/src/base/diameter_peer.erl b/lib/diameter/src/base/diameter_peer.erl index 2759f17e64..dfbf350346 100644 --- a/lib/diameter/src/base/diameter_peer.erl +++ b/lib/diameter/src/base/diameter_peer.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2010-2015. All Rights Reserved. +%% Copyright Ericsson AB 2010-2017. All Rights Reserved. %% %% Licensed under the Apache License, Version 2.0 (the "License"); %% you may not use this file except in compliance with the License. @@ -202,7 +202,7 @@ match1(Addr, Match) -> match(Addr, {ok, A}, _) -> Addr == A; match(Addr, {error, _}, RE) -> - match == re:run(inet_parse:ntoa(Addr), RE, [{capture, none}]). + match == re:run(inet_parse:ntoa(Addr), RE, [{capture, none}, caseless]). addr([_|_] = A) -> inet_parse:address(A); -- cgit v1.2.3 From d10abde1e0f948400a6f14128b5b54443edbffd3 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 15 Aug 2017 14:34:42 +0200 Subject: Replace calls to inet_parse(3) With the documented calls to inet(3). --- lib/diameter/src/base/diameter_lib.erl | 2 +- lib/diameter/src/base/diameter_peer.erl | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/diameter/src/base/diameter_lib.erl b/lib/diameter/src/base/diameter_lib.erl index 8792e97621..1c1ea42cb5 100644 --- a/lib/diameter/src/base/diameter_lib.erl +++ b/lib/diameter/src/base/diameter_lib.erl @@ -283,7 +283,7 @@ ip(T) %% Or not: convert from '.'/':'-separated decimal/hex. ip(Addr) -> - {ok, A} = inet_parse:address(Addr), %% documented in inet(3) + {ok, A} = inet:parse_address(Addr), A. %% --------------------------------------------------------------------------- diff --git a/lib/diameter/src/base/diameter_peer.erl b/lib/diameter/src/base/diameter_peer.erl index dfbf350346..4cb5a57a54 100644 --- a/lib/diameter/src/base/diameter_peer.erl +++ b/lib/diameter/src/base/diameter_peer.erl @@ -202,10 +202,10 @@ match1(Addr, Match) -> match(Addr, {ok, A}, _) -> Addr == A; match(Addr, {error, _}, RE) -> - match == re:run(inet_parse:ntoa(Addr), RE, [{capture, none}, caseless]). + match == re:run(inet:ntoa(Addr), RE, [{capture, none}, caseless]). addr([_|_] = A) -> - inet_parse:address(A); + inet:parse_address(A); addr(A) -> {ok, A}. -- cgit v1.2.3 From bc86d93f4c842487e1c1ea37ba743d5fe9169c58 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sat, 19 Aug 2017 12:36:27 +0200 Subject: Use map decoding in example client As introduced in commit 1b3b64af and adjusted in commit e0603ba1. There's nothing client-specific about it, but keep the record format in the server example for the sake of coverage. --- lib/diameter/examples/code/client.erl | 10 +++++----- lib/diameter/examples/code/client_cb.erl | 29 +++++++++++++---------------- 2 files changed, 18 insertions(+), 21 deletions(-) (limited to 'lib') diff --git a/lib/diameter/examples/code/client.erl b/lib/diameter/examples/code/client.erl index 6fb90b1c09..0864919cdd 100644 --- a/lib/diameter/examples/code/client.erl +++ b/lib/diameter/examples/code/client.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2010-2015. All Rights Reserved. +%% Copyright Ericsson AB 2010-2017. All Rights Reserved. %% %% Licensed under the Apache License, Version 2.0 (the "License"); %% you may not use this file except in compliance with the License. @@ -39,7 +39,6 @@ -module(client). -include_lib("diameter/include/diameter.hrl"). --include_lib("diameter/include/diameter_gen_base_rfc6733.hrl"). -export([start/1, %% start a service start/2, %% @@ -71,6 +70,7 @@ {'Product-Name', "Client"}, {'Auth-Application-Id', [0]}, {string_decode, false}, + {decode_format, map}, {application, [{alias, common}, {dictionary, diameter_gen_base_rfc6733}, {module, client_cb}]}]). @@ -108,9 +108,9 @@ connect(T) -> call(Name) -> SId = diameter:session_id(?L(Name)), - RAR = #diameter_base_RAR{'Session-Id' = SId, - 'Auth-Application-Id' = 0, - 'Re-Auth-Request-Type' = 0}, + RAR = ['RAR' | #{'Session-Id' => SId, + 'Auth-Application-Id' => 0, + 'Re-Auth-Request-Type' => 0}], diameter:call(Name, common, RAR, []). call() -> diff --git a/lib/diameter/examples/code/client_cb.erl b/lib/diameter/examples/code/client_cb.erl index ed1d3b9b7b..af2d4d6da7 100644 --- a/lib/diameter/examples/code/client_cb.erl +++ b/lib/diameter/examples/code/client_cb.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2010-2016. All Rights Reserved. +%% Copyright Ericsson AB 2010-2017. All Rights Reserved. %% %% Licensed under the Apache License, Version 2.0 (the "License"); %% you may not use this file except in compliance with the License. @@ -55,21 +55,18 @@ prepare_request(#diameter_packet{msg = ['RAR' = T | Avps]}, _, {_, Caps}) -> origin_realm = {OR, DR}} = Caps, - {send, [T, {'Origin-Host', OH}, - {'Origin-Realm', OR}, - {'Destination-Host', DH}, - {'Destination-Realm', DR} - | Avps]}; - -prepare_request(#diameter_packet{msg = Rec}, _, {_, Caps}) -> - #diameter_caps{origin_host = {OH, DH}, - origin_realm = {OR, DR}} - = Caps, - - {send, Rec#diameter_base_RAR{'Origin-Host' = OH, - 'Origin-Realm' = OR, - 'Destination-Host' = DH, - 'Destination-Realm' = DR}}. + {send, [T | if is_map(Avps) -> + Avps#{'Origin-Host' => OH, + 'Origin-Realm' => OR, + 'Destination-Host' => DH, + 'Destination-Realm' => DR}; + is_list(Avps) -> + [{'Origin-Host', OH}, + {'Origin-Realm', OR}, + {'Destination-Host', DH}, + {'Destination-Realm', DR} + | Avps] + end]}. %% prepare_retransmit/3 -- cgit v1.2.3 From 2a4ec066046a5953527e496226f10171e229ae3e Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sat, 19 Aug 2017 14:56:15 +0200 Subject: Handle loopback/any as local address in diameter_tcp/sctp The support is implied by documentation, but wasn't handled in code. Be consistent in retrieving the address from the sock rather than the configuration, and in accepting both ip and ifaddr for a local address. --- lib/diameter/doc/src/diameter_sctp.xml | 3 +- lib/diameter/doc/src/diameter_tcp.xml | 13 ++-- lib/diameter/src/transport/diameter_sctp.erl | 53 +++++++++------ lib/diameter/src/transport/diameter_tcp.erl | 90 ++++++++++++++------------ lib/diameter/test/diameter_transport_SUITE.erl | 39 ++++++----- 5 files changed, 109 insertions(+), 89 deletions(-) (limited to 'lib') diff --git a/lib/diameter/doc/src/diameter_sctp.xml b/lib/diameter/doc/src/diameter_sctp.xml index 9b6d629f79..c9b74a9ec5 100644 --- a/lib/diameter/doc/src/diameter_sctp.xml +++ b/lib/diameter/doc/src/diameter_sctp.xml @@ -16,7 +16,7 @@
2011 -2016 +2017 Ericsson AB. All Rights Reserved. @@ -116,7 +116,6 @@ and port respectively.

Multiple ip options can be specified for a multihomed peer. If none are specified then the values of Host-IP-Address in the diameter_service record are used. -(In particular, one of these must be specified.) Option port defaults to 3868 for a listening transport and 0 for a connecting transport.

diff --git a/lib/diameter/doc/src/diameter_tcp.xml b/lib/diameter/doc/src/diameter_tcp.xml index 6ca280c52b..1d65d14257 100644 --- a/lib/diameter/doc/src/diameter_tcp.xml +++ b/lib/diameter/doc/src/diameter_tcp.xml @@ -170,14 +170,11 @@ that will not be forthcoming, which will eventually cause the RFC 3539 watchdog to take down the connection.

-If an ip option is not specified then the first element of a -non-empty Host-IP-Address list in Svc provides the local -IP address. -If neither is specified then the default address selected by &gen_tcp; -is used. -In all cases, the selected address is either returned from -&start; or passed in a connected message over the transport -interface.

+The first element of a non-empty Host-IP-Address list in +Svc provides the local IP address if an ip option is not +specified. +The local address is either returned from&start; or passed in a +connected message over the transport interface.

diff --git a/lib/diameter/src/transport/diameter_sctp.erl b/lib/diameter/src/transport/diameter_sctp.erl index c150252b78..91010105a3 100644 --- a/lib/diameter/src/transport/diameter_sctp.erl +++ b/lib/diameter/src/transport/diameter_sctp.erl @@ -156,12 +156,7 @@ start(T, Svc, Opts) = Svc, diameter_sctp_sup:start(), %% start supervisors on demand Addrs = Caps#diameter_caps.host_ip_address, - s(T, Addrs, Pid, lists:map(fun ip/1, Opts)). - -ip({ifaddr, A}) -> - {ip, A}; -ip(T) -> - T. + s(T, Addrs, Pid, Opts). %% A listener spawns transports either as a consequence of this call %% when there is not yet an association to assign it, or at comm_up on @@ -354,23 +349,35 @@ l([], Ref, T) -> %% open/3 open(Addrs, Opts, PortNr) -> - {LAs, Os} = addrs(Addrs, Opts), - {LAs, case gen_sctp:open(gen_opts(portnr(Os, PortNr))) of - {ok, Sock} -> - Sock; - {error, Reason} -> - x({open, Reason}) - end}. + case gen_sctp:open(gen_opts(portnr(addrs(Addrs, Opts), PortNr))) of + {ok, Sock} -> + {addrs(Sock), Sock}; + {error, Reason} -> + x({open, Reason}) + end. addrs(Addrs, Opts) -> - case proplists:split(Opts, [ip]) of - {[[]], _} -> - {Addrs, Opts ++ [{ip, A} || A <- Addrs]}; - {[As], Os} -> - LAs = [diameter_lib:ipaddr(A) || {ip, A} <- As], - {LAs, Os ++ [{ip, A} || A <- LAs]} + case lists:mapfoldl(fun ipaddr/2, false, Opts) of + {Os, true} -> + Os; + {_, false} -> + Opts ++ [{ip, A} || A <- Addrs] end. +ipaddr({K,A}, _) + when K == ifaddr; + K == ip -> + {{ip, ipaddr(A)}, true}; +ipaddr(T, B) -> + {T, B}. + +ipaddr(A) + when A == loopback; + A == any -> + A; +ipaddr(A) -> + diameter_lib:ipaddr(A). + portnr(Opts, PortNr) -> case proplists:get_value(port, Opts) of undefined -> @@ -379,6 +386,14 @@ portnr(Opts, PortNr) -> Opts end. +addrs(Sock) -> + case inet:socknames(Sock) of + {ok, As} -> + [A || {A,_} <- As]; + {error, Reason} -> + x({socknames, Reason}) + end. + %% x/1 x(Reason) -> diff --git a/lib/diameter/src/transport/diameter_tcp.erl b/lib/diameter/src/transport/diameter_tcp.erl index aa09a261a3..f1536f0865 100644 --- a/lib/diameter/src/transport/diameter_tcp.erl +++ b/lib/diameter/src/transport/diameter_tcp.erl @@ -143,8 +143,7 @@ -> {ok, pid(), [inet:ip_address()]} when Ref :: diameter:transport_ref(); ({connect, Ref}, #diameter_service{}, [connect_option()]) - -> {ok, pid(), [inet:ip_address()]} - | {ok, pid()} + -> {ok, pid()} when Ref :: diameter:transport_ref(). start({T, Ref}, Svc, Opts) -> @@ -259,22 +258,14 @@ i(#monitor{parent = Pid, transport = TPid} = S) -> i({listen, Ref, {Mod, Opts, Addrs}}) -> [_] = diameter_config:subscribe(Ref, transport), %% assert existence - {[LA, LP], Rest} = proplists:split(Opts, [ip, port]), - LAddrOpt = get_addr(LA, Addrs), - LPort = get_port(LP), - {ok, LSock} = Mod:listen(LPort, gen_opts(LAddrOpt, Rest)), - LAddr = laddr(LAddrOpt, Mod, LSock), + {[LP], Rest} = proplists:split(Opts, [port]), + {ok, LSock} = Mod:listen(get_port(LP), gen_opts(Addrs, Rest)), + {ok, {LAddr, _}} = sockname(Mod, LSock), true = diameter_reg:add_new({?MODULE, listener, {Ref, {LAddr, LSock}}}), proc_lib:init_ack({ok, self(), {LAddr, LSock}}), #listener{socket = LSock, module = Mod}. -laddr([], Mod, Sock) -> - {ok, {Addr, _Port}} = sockname(Mod, Sock), - Addr; -laddr([{ip, Addr}], _, _) -> - Addr. - ssl_opts([]) -> false; ssl_opts([{ssl_options, true}]) -> @@ -309,24 +300,16 @@ init(accept = T, Ref, Mod, Pid, Opts, Addrs, SvcPid) -> Sock; init(connect = T, Ref, Mod, Pid, Opts, Addrs, _SvcPid) -> - {[LA, RA, RP], Rest} = proplists:split(Opts, [ip, raddr, rport]), - LAddrOpt = get_addr(LA, Addrs), + {[RA, RP], Rest} = proplists:split(Opts, [raddr, rport]), RAddr = get_addr(RA), RPort = get_port(RP), - proc_lib:init_ack(init_rc(LAddrOpt)), - Sock = ok(connect(Mod, RAddr, RPort, gen_opts(LAddrOpt, Rest))), + proc_lib:init_ack({ok, self()}), + Sock = ok(connect(Mod, RAddr, RPort, gen_opts(Addrs, Rest))), publish(Mod, T, Ref, Sock), - up(Pid, {RAddr, RPort}, LAddrOpt, Mod, Sock), + up(Pid, {RAddr, RPort}, Mod, Sock), Sock. -init_rc([{ip, Addr}]) -> - {ok, self(), [Addr]}; -init_rc([]) -> - {ok, self()}. - -up(Pid, Remote, [{ip, _Addr}], _, _) -> - diameter_peer:up(Pid, Remote); -up(Pid, Remote, [], Mod, Sock) -> +up(Pid, Remote, Mod, Sock) -> {Addr, _Port} = ok(sockname(Mod, Sock)), diameter_peer:up(Pid, Remote, [Addr]). @@ -383,25 +366,41 @@ l([{{?MODULE, listener, {_, AS}}, LPid}], _, _) -> l([], Ref, T) -> diameter_tcp_sup:start_child({listen, Ref, T}). -%% get_addr/1 +%% addrs/2 +%% +%% Take the first address from the service if several are specified +%% and not address is configured. + +addrs(Addrs, Opts) -> + case lists:mapfoldr(fun ipaddr/2, [], Opts) of + {Os, [_]} -> + Os; + {_, []} -> + Opts ++ [{ip, A} || [A|_] <- [Addrs]]; + {_, As} -> + ?ERROR({invalid_addrs, As, Addrs}) + end. -get_addr(As) -> - diameter_lib:ipaddr(addr(As, [])). +ipaddr({K,A}, As) + when K == ifaddr; + K == ip -> + {{ip, ipaddr(A)}, [A | As]}; +ipaddr(T, B) -> + {T, B}. -%% get_addr/2 +ipaddr(A) + when A == loopback; + A == any -> + A; +ipaddr(A) -> + diameter_lib:ipaddr(A). -get_addr([], []) -> - []; -get_addr(As, Def) -> - [{ip, diameter_lib:ipaddr(addr(As, Def))}]. +%% get_addr/1 -%% Take the first address from the service if several are unspecified. -addr([], [Addr | _]) -> - Addr; -addr([{_, Addr}], _) -> - Addr; -addr(As, Addrs) -> - ?ERROR({invalid_addrs, As, Addrs}). +get_addr([{_, Addr}]) -> + diameter_lib:ipaddr(Addr); +get_addr(Addrs) -> + ?ERROR({invalid_addrs, Addrs}). %% get_port/1 @@ -414,10 +413,15 @@ get_port(Ps) -> %% gen_opts/2 -gen_opts(LAddrOpt, Opts) -> +gen_opts(Addrs, Opts) -> + gen_opts(addrs(Addrs, Opts)). + +%% gen_opts/1 + +gen_opts(Opts) -> {L,_} = proplists:split(Opts, [binary, packet, active]), [[],[],[]] == L orelse ?ERROR({reserved_options, Opts}), - [binary, {packet, 0}, {active, false}] ++ LAddrOpt ++ Opts. + [binary, {packet, 0}, {active, false} | Opts]. %% --------------------------------------------------------------------------- %% # ports/1 diff --git a/lib/diameter/test/diameter_transport_SUITE.erl b/lib/diameter/test/diameter_transport_SUITE.erl index 9d981d0a2b..284d2b9566 100644 --- a/lib/diameter/test/diameter_transport_SUITE.erl +++ b/lib/diameter/test/diameter_transport_SUITE.erl @@ -349,35 +349,40 @@ rand_bytes(N) -> %% start_connect/3 start_connect(Prot, PortNr, Ref) -> - {ok, TPid, [?ADDR]} = start_connect(Prot, - {connect, Ref}, - ?SVC([]), - [{raddr, ?ADDR}, - {rport, PortNr}, - {ip, ?ADDR}, - {port, 0}]), - ?RECV(?TMSG({TPid, connected, _})), + {ok, TPid} = start_connect(Prot, + {connect, Ref}, + ?SVC([]), + [{raddr, ?ADDR}, + {rport, PortNr}, + {ip, ?ADDR}, + {port, 0}]), + connected(Prot, TPid), TPid. +connected(sctp, TPid) -> + ?RECV(?TMSG({TPid, connected, _})); +connected(tcp, TPid) -> + ?RECV(?TMSG({TPid, connected, _, [?ADDR]})). + start_connect(sctp, T, Svc, Opts) -> - diameter_sctp:start(T, Svc, [{sctp_initmsg, ?SCTP_INIT} | Opts]); + {ok, TPid, [?ADDR]} + = diameter_sctp:start(T, Svc, [{sctp_initmsg, ?SCTP_INIT} | Opts]), + {ok, TPid}; start_connect(tcp, T, Svc, Opts) -> diameter_tcp:start(T, Svc, Opts). %% start_accept/2 start_accept(Prot, Ref) -> - {Mod, Opts} = tmod(Prot), - {ok, TPid, [?ADDR]} = Mod:start({accept, Ref}, - ?SVC([?ADDR]), - [{port, 0} | Opts]), + {ok, TPid, [?ADDR]} + = start_accept(Prot, {accept, Ref}, ?SVC([?ADDR]), [{port, 0}]), ?RECV(?TMSG({TPid, connected})), TPid. -tmod(sctp) -> - {diameter_sctp, [{sctp_initmsg, ?SCTP_INIT}]}; -tmod(tcp) -> - {diameter_tcp, []}. +start_accept(sctp, T, Svc, Opts) -> + diameter_sctp:start(T, Svc, [{sctp_initmsg, ?SCTP_INIT} | Opts]); +start_accept(tcp, T, Svc, Opts) -> + diameter_tcp:start(T, Svc, Opts). %% =========================================================================== -- cgit v1.2.3 From 0ad00d91398c093af6139bd4b8e452d7f6133636 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sat, 19 Aug 2017 18:28:22 +0200 Subject: Use loopback/any config in examples suite --- lib/diameter/test/diameter_examples_SUITE.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/diameter/test/diameter_examples_SUITE.erl b/lib/diameter/test/diameter_examples_SUITE.erl index eb99f10fe6..ee44ed8dc9 100644 --- a/lib/diameter/test/diameter_examples_SUITE.erl +++ b/lib/diameter/test/diameter_examples_SUITE.erl @@ -344,7 +344,7 @@ top(Dir, LibDir) -> start({server, Prot}) -> ok = diameter:start(), ok = server:start(), - {ok, Ref} = server:listen(Prot), + {ok, Ref} = server:listen({Prot, any, 3868}), [_] = ?util:lport(Prot, Ref), ok; @@ -352,7 +352,7 @@ start({client = Svc, Prot}) -> ok = diameter:start(), true = diameter:subscribe(Svc), ok = client:start(), - {ok, Ref} = client:connect(Prot), + {ok, Ref} = client:connect({Prot, loopback, loopback, 3868}), receive #diameter_event{info = {up, Ref, _, _, _}} -> ok end; start(Config) -> -- cgit v1.2.3 From 0bb9e7f639042f8803c3937581ddc9210fd60418 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 22 Aug 2017 13:14:57 +0200 Subject: Work around more common_test woe Testcase init functions are getting Config from groups and suites they're not in. Presumably related to the problems worked around in the parent commit. --- lib/diameter/test/diameter_traffic_SUITE.erl | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'lib') diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl index a2c0f7fae6..d6d69eafa1 100644 --- a/lib/diameter/test/diameter_traffic_SUITE.erl +++ b/lib/diameter/test/diameter_traffic_SUITE.erl @@ -375,6 +375,16 @@ select() -> %% -------------------- +%% Work around common_test accumulating Config improperly, causing +%% testcases to get Config from groups and suites they're not in. +init_per_testcase(N, Config) + when N == rfc4005; + N == start; + N == result_codes; + N == empty; + N == stop -> + Config; + %% Skip testcases that can reasonably fail under SCTP. init_per_testcase(Name, Config) -> TCs = proplists:get_value(runlist, Config, []), @@ -397,6 +407,9 @@ end_per_testcase(_, _) -> ok. %% replace/2 +%% +%% Work around common_test running init functions inappropriately, and +%% this accumulating more config than expected. replace(Pairs, Config) when is_list(Pairs) -> -- cgit v1.2.3 From dfa762240900a512a38d57feb71ac77cff7f625c Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Wed, 23 Aug 2017 12:29:28 +0200 Subject: Fix compatibility of remote send By changing the definition of the request record, commit f489c0d5 broke sending an outgoing request over a peer connection terminated on a remote node running an older version of diameter. The modified fields aren't even used on the remote node, so simply reintroduce one of the fields so that the size of the tuple is unchanged. --- lib/diameter/src/base/diameter_traffic.erl | 1 + 1 file changed, 1 insertion(+) (limited to 'lib') diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index 85378babea..33e8e03f72 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -89,6 +89,7 @@ caller :: pid() | undefined, %% calling process handler :: pid(), %% request process peer :: undefined | {pid(), #diameter_caps{}}, + caps :: undefined, %% no longer used packet :: #diameter_packet{} | undefined}). %% of request %% --------------------------------------------------------------------------- -- cgit v1.2.3 From 501b27d75cdf9623756848fc09cf16e59fe523ce Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 22 Aug 2017 14:45:39 +0200 Subject: Rename transport_opt() capx_strictness to strict_capx To follow the naming of options like strict_mbit and more. Still accept capx_strictness since this is known to be used. Introduced in commit e4f28f3b. --- lib/diameter/src/base/diameter.erl | 2 +- lib/diameter/src/base/diameter_config.erl | 2 ++ lib/diameter/src/base/diameter_peer_fsm.erl | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index 2e18a1d903..888a108f8b 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -394,7 +394,7 @@ call(SvcName, App, Message) -> | {capabilities, [capability()]} | {capabilities_cb, evaluable()} | {capx_timeout, 'Unsigned32'()} - | {capx_strictness, boolean()} + | {strict_capx, boolean()} | {disconnect_cb, evaluable()} | {dpr_timeout, 'Unsigned32'()} | {dpa_timeout, 'Unsigned32'()} diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index f1b6e56782..d54dbd9976 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -591,6 +591,8 @@ opt({K, Tmo}) ?IS_UINT32(Tmo); opt({capx_strictness, B}) -> + is_boolean(B) andalso {value, {strict_capx, B}}; +opt({strict_capx, B}) -> is_boolean(B); opt({length_errors, T}) -> diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index 4870b86be5..77ee3d6057 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -239,7 +239,7 @@ i({Ack, WPid, {M, Ref} = T, Opts, {SvcOpts, Nodes, Dict0, Svc}}) -> proplists:get_value(dpa_timeout, Opts, ?DPA_TIMEOUT)}), Tmo = proplists:get_value(capx_timeout, Opts, ?CAPX_TIMEOUT), - Strictness = proplists:get_value(capx_strictness, Opts, true), + Strict = proplists:get_value(strict_capx, Opts, true), LengthErr = proplists:get_value(length_errors, Opts, exit), {TPid, Addrs} = start_transport(T, Rest, Svc), @@ -253,7 +253,7 @@ i({Ack, WPid, {M, Ref} = T, Opts, {SvcOpts, Nodes, Dict0, Svc}}) -> mode = M, service = svc(Svc, Addrs), length_errors = LengthErr, - strict = Strictness, + strict = Strict, incoming_maxlen = Maxlen, codec = maps:with([decode_format, string_decode, -- cgit v1.2.3 From 3c7e35ca4512088f0b4074f809f3110dfa285b8e Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 22 Aug 2017 15:37:04 +0200 Subject: Document transport_opt() strict_capx --- lib/diameter/doc/src/diameter.xml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'lib') diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml index 3ad24257a5..264dbf9152 100644 --- a/lib/diameter/doc/src/diameter.xml +++ b/lib/diameter/doc/src/diameter.xml @@ -1409,6 +1409,20 @@ Options monitor and link are ignored.

Defaults to the list configured on the service if not specified.

+ +{strict_capx, boolean()]} + +

+Whether or not to enforce the RFC 6733 requirement that any message +before capabilities exchange should close the peer connection. +If false then unexpected messages are discarded.

+ +

+Defaults to true. +Changing this results in non-standard behaviour, but can be useful in +case peers are known to be behave badly.

+ + {transport_config, term()} {transport_config, term(), &dict_Unsigned32; | infinity} -- cgit v1.2.3 From 2d540b95755a6f628b0cfc5a9bab4ff41046fe4b Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 24 Aug 2017 11:51:44 +0200 Subject: Rename type evaluable -> eval Export the old type as a synonym for backwards compatability. The name evaluable is a bit too awkward. --- lib/diameter/doc/src/diameter.xml | 30 ++++++++++++++-------------- lib/diameter/doc/src/diameter_app.xml | 9 +++++---- lib/diameter/doc/src/seealso.ent | 4 ++-- lib/diameter/src/base/diameter.erl | 22 +++++++++++--------- lib/diameter/src/base/diameter_callback.erl | 6 +++--- lib/diameter/src/transport/diameter_sctp.erl | 6 +++--- lib/diameter/src/transport/diameter_tcp.erl | 4 ++-- 7 files changed, 43 insertions(+), 38 deletions(-) (limited to 'lib') diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml index 264dbf9152..6a4a7b9f7c 100644 --- a/lib/diameter/doc/src/diameter.xml +++ b/lib/diameter/doc/src/diameter.xml @@ -397,10 +397,10 @@ from the peer offers it.

Note that each tuple communicates one or more AVP values. It is an error to specify duplicate tuples.

- + -evaluable() = {M,F,A} | fun() | [evaluable() | A] +eval() = {M,F,A} | fun() | [eval() | A]

An expression that can be evaluated as a function in the following @@ -418,7 +418,7 @@ eval(F) ->

-Applying an &evaluable; +Applying an &eval; E to an argument list A is meant in the sense of eval([E|A]).

@@ -484,11 +484,11 @@ Matches only those peers whose Origin-Realm has the specified value, or all peers if the atom any.

 -{eval, &evaluable;} +{eval, &eval;}

Matches only those peers for which the specified -&evaluable; returns +&eval; returns true when applied to the connection's diameter_caps record. Any other return value or exception is equivalent to false.

@@ -650,7 +650,7 @@ Result = ResultCode | {capabilities_cb, CB, ResultCode|discard} Caps = #diameter_caps{} Pkt = #diameter_packet{} ResultCode = integer() -CB = &evaluable; +CB = &eval;

@@ -843,7 +843,7 @@ Length field in a Diameter Header.

| node | nodes | [node()] - | evaluable()} + | eval()}

The degree to which the service allows multiple transport @@ -854,7 +854,7 @@ at capabilities exchange.

If [node()] then a connection is rejected if another already exists on any of the specified nodes. Types false, node, nodes and -&evaluable; are equivalent to +&eval; are equivalent to [], [node()], [node()|nodes()] and the evaluated value respectively, evaluation of each expression taking place whenever a new connection is to be established. @@ -869,7 +869,7 @@ by their own peer and watchdog state machines.

Defaults to nodes.

 -{sequence, {H,N} | &evaluable;} +{sequence, {H,N} | &eval;}

A constant value H for the topmost 32-N bits of @@ -904,7 +904,7 @@ outgoing requests.

 -{share_peers, boolean() | [node()] | evaluable()} +{share_peers, boolean() | [node()] | eval()}

Nodes to which peer connections established on the local @@ -917,7 +917,7 @@ configured to use them: see use_shared_peers below.

If false then peers are not shared. If [node()] then peers are shared with the specified list of nodes. -If evaluable() then peers are shared with the nodes returned +If eval() then peers are shared with the nodes returned by the specified function, evaluated whenever a peer connection becomes available or a remote service requests information about local connections. @@ -1073,7 +1073,7 @@ omitted counters are not returned by &service_info;.

 -{use_shared_peers, boolean() | [node()] | evaluable()} +{use_shared_peers, boolean() | [node()] | eval()}

Nodes from which communicated peers are made available in @@ -1083,7 +1083,7 @@ the remote candidates list of &app_pick_peer; callbacks.

If false then remote peers are not used. If [node()] then only peers from the specified list of nodes are used. -If evaluable() then only peers returned by the specified +If eval() then only peers returned by the specified function are used, evaluated whenever a remote service communicates information about an available peer connection. The value true is equivalent to fun &nodes;. @@ -1155,7 +1155,7 @@ TLS is desired over TCP as implemented by &man_tcp;.

 -{capabilities_cb, &evaluable;} +{capabilities_cb, &eval;}

Callback invoked upon reception of CER/CEA during capabilities @@ -1249,7 +1249,7 @@ transport.

 -{disconnect_cb, &evaluable;} +{disconnect_cb, &eval;}

Callback invoked prior to terminating the transport process of a diff --git a/lib/diameter/doc/src/diameter_app.xml b/lib/diameter/doc/src/diameter_app.xml index dfcd00975b..aa334beb21 100644 --- a/lib/diameter/doc/src/diameter_app.xml +++ b/lib/diameter/doc/src/diameter_app.xml @@ -13,7 +13,8 @@

-20112016 +2011 +2017 Ericsson AB. All Rights Reserved. @@ -319,7 +320,7 @@ or &peer_down; callback.

Action = Send | Discard | {eval_packet, Action, PostF} Send = {send, &packet; | &message;} Discard = {discard, Reason} | discard -PostF = &mod_evaluable;} +PostF = &mod_eval;}

@@ -371,7 +372,7 @@ discarded}.

Action = Send | Discard | {eval_packet, Action, PostF} Send = {send, &packet; | &message;} Discard = {discard, Reason} | discard -PostF = &mod_evaluable;} +PostF = &mod_eval;}

@@ -478,7 +479,7 @@ not selected.

| {answer_message, 3000..3999|5000..5999} | {protocol_error, 3000..3999} Opt = &mod_call_opt; -PostF = &mod_evaluable; +PostF = &mod_eval;

diff --git a/lib/diameter/doc/src/seealso.ent b/lib/diameter/doc/src/seealso.ent index e5c284c6e8..ef6af1a3d0 100644 --- a/lib/diameter/doc/src/seealso.ent +++ b/lib/diameter/doc/src/seealso.ent @@ -4,7 +4,7 @@ %CopyrightBegin% -Copyright Ericsson AB 2012-2015. All Rights Reserved. +Copyright Ericsson AB 2012-2017. All Rights Reserved. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -53,7 +53,7 @@ significant. diameter:application_opt()'> diameter:call_opt()'> diameter:capability()'> -diameter:evaluable()'> +diameter:eval()'> diameter:peer_filter()'> diameter:service_event()'> diameter:service_event_info()'> diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index 888a108f8b..ad6aaab440 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -46,7 +46,8 @@ -export([start/0, stop/0]). --export_type([evaluable/0, +-export_type([eval/0, + evaluable/0, %% deprecated decode_format/0, strict_arities/0, restriction/0, @@ -301,7 +302,7 @@ call(SvcName, App, Message) -> | realm | {host, any|'DiameterIdentity'()} | {realm, any|'DiameterIdentity'()} - | {eval, evaluable()} + | {eval, eval()} | {neg, peer_filter()} | {all, [peer_filter()]} | {any, [peer_filter()]}. @@ -309,10 +310,13 @@ call(SvcName, App, Message) -> -opaque peer_ref() :: pid(). --type evaluable() +-type eval() :: {module(), atom(), list()} | fun() - | maybe_improper_list(evaluable(), list()). + | maybe_improper_list(eval(), list()). + +-type evaluable() + :: eval(). -type sequence() :: {'Unsigned32'(), 0..32}. @@ -322,12 +326,12 @@ call(SvcName, App, Message) -> | node | nodes | [node()] - | evaluable(). + | eval(). -type remotes() :: boolean() | [node()] - | evaluable(). + | eval(). -type message_length() :: 0..16#FFFFFF. @@ -350,7 +354,7 @@ call(SvcName, App, Message) -> :: capability() | {application, [application_opt()]} | {restrict_connections, restriction()} - | {sequence, sequence() | evaluable()} + | {sequence, sequence() | eval()} | {share_peers, remotes()} | {decode_format, decode_format()} | {traffic_counters, boolean()} @@ -392,10 +396,10 @@ call(SvcName, App, Message) -> | {pool_size, pos_integer()} | {applications, [app_alias()]} | {capabilities, [capability()]} - | {capabilities_cb, evaluable()} + | {capabilities_cb, eval()} | {capx_timeout, 'Unsigned32'()} | {strict_capx, boolean()} - | {disconnect_cb, evaluable()} + | {disconnect_cb, eval()} | {dpr_timeout, 'Unsigned32'()} | {dpa_timeout, 'Unsigned32'()} | {length_errors, exit | handle | discard} diff --git a/lib/diameter/src/base/diameter_callback.erl b/lib/diameter/src/base/diameter_callback.erl index f9cdc66c70..d04a416bef 100644 --- a/lib/diameter/src/base/diameter_callback.erl +++ b/lib/diameter/src/base/diameter_callback.erl @@ -26,16 +26,16 @@ %% as the Diameter application callback in question. The record has %% one field for each callback function as well as 'default' and %% 'extra' fields. A function-specific field can be set to a -%% diameter:evaluable() in order to redirect the callback +%% diameter:eval() in order to redirect the callback %% corresponding to that field, or to 'false' to request the default %% callback implemented in this module. If neither of these fields are %% set then the 'default' field determines the form of the callback: a %% module name results in the usual callback as if the module had been -%% configured directly as the callback module, a diameter_evaluable() +%% configured directly as the callback module, a diameter_eval() %% in a callback applied to the atom-valued callback name and argument %% list. For all callbacks not to this module, the 'extra' field is a %% list of additional arguments, following arguments supplied by -%% diameter but preceding those of the diameter:evaluable() being +%% diameter but preceding those of the diameter:eval() being %% applied. %% %% For example, the following config to diameter:start_service/2, in diff --git a/lib/diameter/src/transport/diameter_sctp.erl b/lib/diameter/src/transport/diameter_sctp.erl index 6a9f1f940b..4f56475529 100644 --- a/lib/diameter/src/transport/diameter_sctp.erl +++ b/lib/diameter/src/transport/diameter_sctp.erl @@ -79,7 +79,7 @@ -type option() :: {sender, boolean()} | sender | {packet, boolean() | raw} - | {message_cb, false | diameter:evaluable()}. + | {message_cb, false | diameter:eval()}. -type uint() :: non_neg_integer(). @@ -104,7 +104,7 @@ os = 0 :: uint(), %% next output stream packet = true :: boolean() %% legacy transport_data? | raw, - message_cb = false :: false | diameter:evaluable(), + message_cb = false :: false | diameter:eval(), send = false :: pid() | boolean()}). %% sending process %% Monitor process state. @@ -120,7 +120,7 @@ socket :: gen_sctp:sctp_socket(), service :: pid(), %% service process pending = {0, queue:new()}, - opts :: [[match()] | boolean() | diameter:evaluable()]}). + opts :: [[match()] | boolean() | diameter:eval()]}). %% Field pending implements two queues: the first of transport-to-be %% processes to which an association has been assigned but for which %% diameter hasn't yet spawned a transport process, a short-lived diff --git a/lib/diameter/src/transport/diameter_tcp.erl b/lib/diameter/src/transport/diameter_tcp.erl index 6252dbddfb..ac55d722fa 100644 --- a/lib/diameter/src/transport/diameter_tcp.erl +++ b/lib/diameter/src/transport/diameter_tcp.erl @@ -110,7 +110,7 @@ -type option() :: {port, non_neg_integer()} | {sender, boolean()} | sender - | {message_cb, false | diameter:evaluable()} + | {message_cb, false | diameter:eval()} | {fragment_timer, 0..16#FFFFFFFF}. %% Accepting/connecting transport process state. @@ -125,7 +125,7 @@ timeout :: infinity | 0..16#FFFFFFFF, %% fragment timeout tref = false :: false | reference(), %% fragment timer reference flush = false :: boolean(), %% flush fragment at timeout? - message_cb :: false | diameter:evaluable(), + message_cb :: false | diameter:eval(), send :: pid() | false}). %% sending process %% The usual transport using gen_tcp can be replaced by anything -- cgit v1.2.3 From 5f3becaddef9b18c81a1ec8bd7bf955384c1a225 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Wed, 23 Aug 2017 12:21:14 +0200 Subject: Let a service configure default transport options Only a default spawn_opt has been possible to configure, but there's no reason why most others should need to be configured per transport. Those options that still only make sense on a transport are transport_module/config (because of the semantics of multiple values), applications/capabilities (since these override service options), and private (since it's only to allow user-specific options in a backwards compatible way). --- lib/diameter/doc/src/diameter.xml | 23 +-- lib/diameter/src/base/diameter.erl | 31 +-- lib/diameter/src/base/diameter_config.erl | 297 +++++++++++++--------------- lib/diameter/src/base/diameter_service.erl | 64 ++++-- lib/diameter/src/base/diameter_watchdog.erl | 35 ++-- 5 files changed, 233 insertions(+), 217 deletions(-) (limited to 'lib') diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml index 6a4a7b9f7c..8d39b9efce 100644 --- a/lib/diameter/doc/src/diameter.xml +++ b/lib/diameter/doc/src/diameter.xml @@ -943,18 +943,6 @@ of a single Diameter node across multiple Erlang nodes.

-{spawn_opt, [term()]} - -

-Options list passed to &spawn_opt; when spawning a process for an -incoming Diameter request, unless the transport in question -specifies another value. -Options monitor and link are ignored.

- -

-Defaults to the empty list.

- - {strict_arities, boolean() | encode @@ -1108,6 +1096,15 @@ each node from which requests are sent.

+&transport_opt; + +

+Any transport option except applications or +capabilities. +Used as defaults for transport configuration, values passed to +&add_transport; overriding values configured on the service.

+ + @@ -1406,7 +1403,7 @@ incoming Diameter request. Options monitor and link are ignored.

-Defaults to the list configured on the service if not specified.

+Defaults to the empty list.

diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index ad6aaab440..0f919f6c25 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -348,6 +348,22 @@ call(SvcName, App, Message) -> | encode | decode. +%% Options common to both start_service/2 and add_transport/2. + +-type common_opt() + :: {pool_size, pos_integer()} + | {capabilities_cb, eval()} + | {capx_timeout, 'Unsigned32'()} + | {strict_capx, boolean()} + | {disconnect_cb, eval()} + | {dpr_timeout, 'Unsigned32'()} + | {dpa_timeout, 'Unsigned32'()} + | {length_errors, exit | handle | discard} + | {connect_timer, 'Unsigned32'()} + | {watchdog_timer, 'Unsigned32'() | {module(), atom(), list()}} + | {watchdog_config, [{okay|suspect, non_neg_integer()}]} + | {spawn_opt, list()}. + %% Options passed to start_service/2 -type service_opt() @@ -363,7 +379,7 @@ call(SvcName, App, Message) -> | {strict_mbit, boolean()} | {incoming_maxlen, message_length()} | {use_shared_peers, remotes()} - | {spawn_opt, list()}. + | common_opt(). -type application_opt() :: {alias, app_alias()} @@ -393,20 +409,9 @@ call(SvcName, App, Message) -> :: {transport_module, atom()} | {transport_config, any()} | {transport_config, any(), 'Unsigned32'() | infinity} - | {pool_size, pos_integer()} | {applications, [app_alias()]} | {capabilities, [capability()]} - | {capabilities_cb, eval()} - | {capx_timeout, 'Unsigned32'()} - | {strict_capx, boolean()} - | {disconnect_cb, eval()} - | {dpr_timeout, 'Unsigned32'()} - | {dpa_timeout, 'Unsigned32'()} - | {length_errors, exit | handle | discard} - | {connect_timer, 'Unsigned32'()} - | {watchdog_timer, 'Unsigned32'() | {module(), atom(), list()}} - | {watchdog_config, [{okay|suspect, non_neg_integer()}]} - | {spawn_opt, list()} + | common_opt() | {private, any()}. %% Predicate passed to remove_transport/2 diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index d54dbd9976..a4496f9725 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -102,9 +102,6 @@ -record(monitor, {mref = make_ref() :: reference(), service}). %% name -%% The default sequence mask. --define(NOMASK, {0,32}). - %% Time to lay low before restarting a dead service. -define(RESTART_SLEEP, 2000). @@ -560,89 +557,184 @@ add(SvcName, Type, Opts0) -> end. transport_opts(Opts) -> - lists:map(fun topt/1, Opts). + [setopt(transport, T) || T <- Opts]. + +%% setopt/2 -topt(T) -> - case opt(T) of +setopt(K, T) -> + case opt(K, T) of {value, X} -> X; true -> T; false -> - ?THROW({invalid, T}) + ?THROW({invalid, T}); + {error, Reason} -> + ?THROW({invalid, T, Reason}) end. -opt({transport_module, M}) -> +%% opt/2 + +opt(service, {incoming_maxlen, N}) + when 0 =< N, N < 1 bsl 24 -> + true; + +opt(service, {K, B}) + when K == strict_mbit; + K == string_decode; + K == traffic_counters -> + is_boolean(B); + +opt(service, {K, false}) + when K == share_peers; + K == use_shared_peers; + K == monitor; + K == restrict_connections; + K == strict_arities; + K == decode_format -> + true; + +opt(service, {K, true}) + when K == share_peers; + K == use_shared_peers; + K == strict_arities -> + true; + +opt(service, {decode_format, T}) + when T == record; + T == list; + T == map; + T == record_from_map -> + true; + +opt(service, {strict_arities, T}) + when T == encode; + T == decode -> + true; + +opt(service, {restrict_connections, T}) + when T == node; + T == nodes -> + true; + +opt(service, {K, T}) + when (K == share_peers + orelse K == use_shared_peers + orelse K == restrict_connections), ([] == T + orelse is_atom(hd(T))) -> + true; + +opt(service, {monitor, P}) -> + is_pid(P); + +opt(service, {K, F}) + when K == restrict_connections; + K == share_peers; + K == use_shared_peers -> + try diameter_lib:eval(F) of %% but no guarantee that it won't fail later + Nodes -> + is_list(Nodes) orelse {error, Nodes} + catch + E:R -> + {error, {E, R, ?STACK}} + end; + +opt(service, {sequence, {H,N}}) -> + 0 =< N andalso N =< 32 + andalso is_integer(H) + andalso 0 =< H + andalso 0 == H bsr (32-N); + +opt(service = S, {sequence = K, F}) -> + try diameter_lib:eval(F) of + {_,_} = T -> + KT = {K,T}, + opt(S, KT) andalso {value, KT}; + V -> + {error, V} + catch + E:R -> + {error, {E, R, ?STACK}} + end; + +opt(transport, {transport_module, M}) -> is_atom(M); -opt({transport_config, _, Tmo}) -> +opt(transport, {transport_config, _, Tmo}) -> ?IS_UINT32(Tmo) orelse Tmo == infinity; -opt({applications, As}) -> +opt(transport, {applications, As}) -> is_list(As); -opt({capabilities, Os}) -> - is_list(Os) andalso ok == encode_CER(Os); +opt(transport, {capabilities, Os}) -> + is_list(Os) andalso case encode_CER(Os) of + ok -> true; + No -> {error, No} + end; -opt({K, Tmo}) +opt(_, {K, Tmo}) when K == capx_timeout; K == dpr_timeout; K == dpa_timeout -> ?IS_UINT32(Tmo); -opt({capx_strictness, B}) -> +opt(_, {capx_strictness, B}) -> is_boolean(B) andalso {value, {strict_capx, B}}; -opt({strict_capx, B}) -> +opt(_, {strict_capx, B}) -> is_boolean(B); -opt({length_errors, T}) -> +opt(_, {length_errors, T}) -> lists:member(T, [exit, handle, discard]); -opt({K, Tmo}) - when K == reconnect_timer; %% deprecated - K == connect_timer -> +opt(transport, {reconnect_timer, Tmo}) -> %% deprecated + ?IS_UINT32(Tmo) andalso {value, {connect_timer, Tmo}}; +opt(_, {connect_timer, Tmo}) -> ?IS_UINT32(Tmo); -opt({watchdog_timer, {M,F,A}}) +opt(_, {watchdog_timer, {M,F,A}}) when is_atom(M), is_atom(F), is_list(A) -> true; -opt({watchdog_timer, Tmo}) -> +opt(_, {watchdog_timer, Tmo}) -> ?IS_UINT32(Tmo); -opt({watchdog_config, L}) -> - is_list(L) andalso lists:all(fun wdopt/1, L); +opt(_, {watchdog_config, L}) -> + is_list(L) andalso lists:all(fun wd/1, L); -opt({spawn_opt, {M,F,A}}) +opt(_, {spawn_opt, {M,F,A}}) when is_atom(M), is_atom(F), is_list(A) -> true; -opt({spawn_opt = K, Opts}) -> +opt(_, {spawn_opt = K, Opts}) -> if is_list(Opts) -> {value, {K, spawn_opts(Opts)}}; true -> false end; -opt({pool_size, N}) -> +opt(_, {pool_size, N}) -> is_integer(N) andalso 0 < N; -%% Options that we can't validate. -opt({K, _}) +%% Options we can't validate. +opt(_, {K, _}) + when K == disconnect_cb; + K == capabilities_cb -> + true; +opt(transport, {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. +%% Anything else, which is ignored in transport config. This makes +%% options sensitive to spelling mistakes, but arbitrary options are +%% passed by some users as a way to identify transports so can't just +%% do away with it. +opt(K, _) -> + K == transport. + +%% wd/1 -wdopt({K,N}) -> +wd({K,N}) -> (K == okay orelse K == suspect) andalso is_integer(N) andalso 0 =< N; -wdopt(_) -> +wd(_) -> false. %% start_transport/2 @@ -707,19 +799,7 @@ make_config(SvcName, Opts) -> ok = encode_CER(CapOpts), - SvcOpts = make_opts((Opts -- AppOpts) -- CapOpts, - [{false, share_peers}, - {false, use_shared_peers}, - {false, monitor}, - {?NOMASK, sequence}, - {nodes, restrict_connections}, - {16#FFFFFF, incoming_maxlen}, - {true, strict_arities}, - {true, strict_mbit}, - {record, decode_format}, - {true, traffic_counters}, - {true, string_decode}, - {[], spawn_opt}]), + SvcOpts = service_opts((Opts -- AppOpts) -- CapOpts), D = proplists:get_value(string_decode, SvcOpts, true), @@ -733,115 +813,22 @@ binary_caps(Caps, true) -> binary_caps(Caps, false) -> diameter_capx:binary_caps(Caps). -%% make_opts/2 - -make_opts(Opts, Defs) -> - Known = [{K, get_opt(K, Opts, D)} || {D,K} <- Defs], - Unknown = Opts -- Known, +%% service_opts/1 - [] == Unknown orelse ?THROW({invalid, hd(Unknown)}), - - [{K, opt(K,V)} || {K,V} <- Known]. - -opt(incoming_maxlen, N) - when 0 =< N, N < 1 bsl 24 -> - N; - -opt(spawn_opt, {M,F,A} = T) - when is_atom(M), is_atom(F), is_list(A) -> - T; - -opt(spawn_opt, L) - when is_list(L) -> - spawn_opts(L); - -opt(K, false = B) - when K == share_peers; - K == use_shared_peers; - K == monitor; - K == restrict_connections; - K == strict_arities; - K == strict_mbit; - K == decode_format; - K == traffic_counters; - K == string_decode -> - B; - -opt(K, true = B) - when K == share_peers; - K == use_shared_peers; - K == strict_arities; - K == strict_mbit; - K == traffic_counters; - K == string_decode -> - B; - -opt(decode_format, T) - when T == record; - T == list; - T == map; - T == record_from_map -> - T; - -opt(strict_arities, T) - when T == encode; - T == decode -> - T; - -opt(restrict_connections, T) - when T == node; - T == nodes -> - T; - -opt(K, T) - when (K == share_peers - orelse K == use_shared_peers - orelse K == restrict_connections), ([] == T - orelse is_atom(hd(T))) -> - T; - -opt(monitor, P) - when is_pid(P) -> - P; - -opt(K, F) - when K == restrict_connections; - K == share_peers; - K == use_shared_peers -> - try diameter_lib:eval(F) of %% but no guarantee that it won't fail later - Nodes when is_list(Nodes) -> - F; - V -> - ?THROW({value, {K,V}}) - catch - E:R -> - ?THROW({value, {K, E, R, ?STACK}}) - end; - -opt(sequence, {_,_} = T) -> - sequence(T); - -opt(sequence = K, F) -> - try diameter_lib:eval(F) of - T -> sequence(T) - catch - E:R -> - ?THROW({value, {K, E, R, ?STACK}}) - end; - -opt(K, _) -> - ?THROW({value, K}). +service_opts(Opts) -> + Res = [setopt(service, T) || T <- Opts], + Keys = sets:to_list(sets:from_list([K || {K,_} <- Res])), %% unique + Dups = lists:foldl(fun(K,A) -> lists:keydelete(K, 1, A) end, Res, Keys), + [] == Dups orelse ?THROW({duplicate, Dups}), + Res. +%% Reject duplicates on a service, but not on a transport. There's no +%% particular reason for the inconsistency, but the historic behaviour +%% ignores all but the first of a transport_opt(), and there's no real +%% reason to change it. spawn_opts(L) -> [T || T <- L, T /= link, T /= monitor]. -sequence({H,N} = T) - when 0 =< N, N =< 32, 0 =< H, 0 == H bsr (32-N) -> - T; - -sequence(_) -> - ?THROW({value, sequence}). - make_caps(Caps, Opts) -> case diameter_capx:make_caps(Caps, Opts) of {ok, T} -> diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl index 07f1fd3a4a..c7b0e706a5 100644 --- a/lib/diameter/src/base/diameter_service.erl +++ b/lib/diameter/src/base/diameter_service.erl @@ -117,6 +117,18 @@ decode_format := diameter:decode_format(), traffic_counters := boolean(), string_decode := boolean(), + capabilities_cb => diameter:evaluable(), + pool_size => pos_integer(), + capx_timeout => diameter:'Unsigned32'(), + strict_capx => boolean(), + disconnect_cb => diameter:evaluable(), + dpr_timeout => diameter:'Unsigned32'(), + dpa_timeout => diameter:'Unsigned32'(), + length_errors => exit | handle | discard, + connect_timer => diameter:'Unsigned32'(), + watchdog_timer => diameter:'Unsigned32'() + | {module(), atom(), list()}, + watchdog_config => [{okay|suspect, non_neg_integer()}], spawn_opt := list() | {module(), atom(), list()}}}). %% Record representing an RFC 3539 watchdog process implemented by @@ -682,12 +694,15 @@ i(SvcName) -> cfg_acc({SvcName, #diameter_service{applications = Apps} = Rec, Opts}, {false, Acc}) -> lists:foreach(fun init_mod/1, Apps), + #{monitor := M} + = SvcOpts + = service_opts(Opts), S = #state{service_name = SvcName, service = Rec#diameter_service{pid = self()}, local = init_peers(), remote = init_peers(), - monitor = mref(get_value(monitor, Opts)), - options = service_options(lists:keydelete(monitor, 1, Opts))}, + monitor = mref(M), + options = maps:remove(monitor, SvcOpts)}, {S, Acc}; cfg_acc({_Ref, Type, _Opts} = T, {S, Acc}) @@ -702,8 +717,23 @@ init_peers() -> %% Alias, %% TPid} -service_options(Opts) -> - maps:from_list(lists:delete({strict_arities, true}, Opts)). +service_opts(Opts) -> + T = {strict_arities, true}, + maps:merge(maps:from_list([{monitor, false} | def_opts() -- [T]]), + maps:from_list(Opts -- [T])). + +def_opts() -> %% defaults on the service map + [{share_peers, false}, + {use_shared_peers, false}, + {sequence, {0,32}}, + {restrict_connections, nodes}, + {incoming_maxlen, 16#FFFFFF}, + {strict_arities, true}, + {strict_mbit, true}, + {decode_format, record}, + {traffic_counters, true}, + {string_decode, true}, + {spawn_opt, []}]. mref(false = No) -> No; @@ -721,10 +751,6 @@ init_mod(#diameter_app{alias = Alias, start_fsm({Ref, Type, Opts}, S) -> start(Ref, {Type, Opts}, S). -get_value(Key, Vs) -> - {_, V} = lists:keyfind(Key, 1, Vs), - V. - notify(Share, SvcName, T) -> Nodes = remotes(Share), [] /= Nodes andalso diameter_peer:notify(Nodes, SvcName, T). @@ -809,7 +835,7 @@ start(Ref, Type, Opts, State) -> start(Ref, Type, Opts, N, #state{watchdogT = WatchdogT, local = {PeerT, _, _}, options = #{string_decode := SD} - = SvcOpts0, + = SvcOpts, service_name = SvcName, service = Svc0}) when Type == connect; @@ -818,12 +844,12 @@ start(Ref, Type, Opts, N, #state{watchdogT = WatchdogT, = Svc1 = merge_service(Opts, Svc0), Svc = binary_caps(Svc1, SD), - SvcOpts = merge_options(Opts, SvcOpts0), - RecvData = diameter_traffic:make_recvdata([SvcName, PeerT, Apps, SvcOpts]), - T = {Opts, SvcOpts, RecvData, Svc}, + {SOpts, TOpts} = merge_opts(SvcOpts, Opts), + RecvData = diameter_traffic:make_recvdata([SvcName, PeerT, Apps, SOpts]), + T = {TOpts, SOpts, RecvData, Svc}, Rec = #watchdog{type = Type, ref = Ref, - options = Opts}, + options = TOpts}, diameter_lib:fold_n(fun(_,A) -> [wd(Type, Ref, T, WatchdogT, Rec) | A] @@ -831,10 +857,14 @@ start(Ref, Type, Opts, N, #state{watchdogT = WatchdogT, [], N). -merge_options(Opts, SvcOpts) -> - Keys = maps:keys(SvcOpts), - Map = maps:from_list([KV || {K,_} = KV <- Opts, lists:member(K, Keys)]), - maps:merge(SvcOpts, Map). +merge_opts(SvcOpts, Opts) -> + Keys = [K || {K,_} <- def_opts()], + SO = [T || {K,_} = T <- Opts, lists:member(K, Keys)], + TO = Opts -- SO, + {maps:merge(maps:with(Keys, SvcOpts), maps:from_list(SO)), + TO ++ [T || {K,_} = T <- maps:to_list(SvcOpts), + not lists:member(K, Keys), + not lists:keymember(K, 1, Opts)]}. binary_caps(Svc, true) -> Svc; diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl index b2172356ee..bb671e9860 100644 --- a/lib/diameter/src/base/diameter_watchdog.erl +++ b/lib/diameter/src/base/diameter_watchdog.erl @@ -76,10 +76,8 @@ string_decode := false, strict_arities => diameter:strict_arities(), strict_mbit := boolean(), - failed_avp := false, rfc := 3588 | 6733, - ordered_encode := false, - incoming_maxlen := diameter:message_length()}, + ordered_encode := false}, shutdown = false :: boolean()}). %% --------------------------------------------------------------------------- @@ -137,15 +135,6 @@ i({Ack, T, Pid, {Opts, putr(restart, {T, Opts, Svc, SvcOpts}), %% save seeing it in trace putr(dwr, dwr(Caps)), %% Nodes = restrict_nodes(Restrict), - CodecKeys = [decode_format, - string_decode, - strict_arities, - strict_mbit, - incoming_maxlen, - spawn_opt, - rfc, - ordered_encode], - #watchdog{parent = Pid, transport = start(T, Opts, SvcOpts, Nodes, Dict0, Svc), tw = proplists:get_value(watchdog_timer, @@ -153,13 +142,21 @@ i({Ack, T, Pid, {Opts, ?DEFAULT_TW_INIT), receive_data = RecvData, dictionary = Dict0, - config = - maps:without([traffic_counters | CodecKeys], - config(SvcOpts#{restrict => restrict(Nodes), - suspect => 1, - okay => 3}, - Opts)), - codec = maps:with(CodecKeys -- [strict_arities], + config = maps:with([sequence, + restrict_connections, + restrict, + suspect, + okay], + config(SvcOpts#{restrict => restrict(Nodes), + suspect => 1, + okay => 3}, + Opts)), + codec = maps:with([decode_format, + strict_arities, + strict_mbit, + string_decode, + rfc, + ordered_encode], SvcOpts#{decode_format := false, string_decode := false, ordered_encode => false})}. -- cgit v1.2.3 From b0582c6963f6dc203f05ed810c9446cf3fa0f0ae Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 24 Aug 2017 13:21:28 +0200 Subject: Let strict_mbit and incoming_maxlen be configured per transport Since these can make sense per peer. The remaining service-only options either belong there or make little sense being configured per transport. --- lib/diameter/doc/src/diameter.xml | 112 +++++++++++++++--------------- lib/diameter/src/base/diameter.erl | 4 +- lib/diameter/src/base/diameter_config.erl | 12 ++-- 3 files changed, 64 insertions(+), 64 deletions(-) (limited to 'lib') diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml index 8d39b9efce..30a26ed845 100644 --- a/lib/diameter/doc/src/diameter.xml +++ b/lib/diameter/doc/src/diameter.xml @@ -826,19 +826,6 @@ field of diameter_packet records independently of - -{incoming_maxlen, 0..16777215} - -

-Bound on the expected size of incoming Diameter messages. -Messages larger than the specified number of bytes are discarded.

- -

-Defaults to 16777215, the maximum value of the 24-bit Message -Length field in a Diameter Header.

- - - {restrict_connections, false | node | nodes @@ -974,49 +961,6 @@ of arity 1 as bare values, not wrapped in a list.

- -{strict_mbit, boolean()} - -

-Whether or not to regard an AVP setting the M-bit as erroneous when -the command grammar in question does not explicitly allow the AVP. -If true then such AVPs are regarded as 5001 errors, -DIAMETER_AVP_UNSUPPORTED. -If false then the M-bit is ignored and policing -it becomes the receiver's responsibility.

- -

-Defaults to true.

- - -

-RFC 6733 is unclear about the semantics of the M-bit. -One the one hand, the CCF specification in section 3.2 documents AVP -in a command grammar as meaning any arbitrary AVP; on the -other hand, 1.3.4 states that AVPs setting the M-bit cannot be added -to an existing command: the modified command must instead be -placed in a new Diameter application.

-

-The reason for the latter is presumably interoperability: -allowing arbitrary AVPs setting the M-bit in a command makes its -interpretation implementation-dependent, since there's no -guarantee that all implementations will understand the same set of -arbitrary AVPs in the context of a given command. -However, interpreting AVP in a command grammar as any -AVP, regardless of M-bit, renders 1.3.4 meaningless, since the receiver -can simply ignore any AVP it thinks isn't relevant, regardless of the -sender's intent.

-

-Beware of confusing mandatory in the sense of the M-bit with mandatory -in the sense of the command grammar. -The former is a semantic requirement: that the receiver understand the -semantics of the AVP in the context in question. -The latter is a syntactic requirement: whether or not the AVP must -occur in the message in question.

- - - - {string_decode, boolean()} @@ -1345,6 +1289,19 @@ connection.

Defaults to 5000.

 + +{incoming_maxlen, 0..16777215} + +

+Bound on the expected size of incoming Diameter messages. +Messages larger than the specified number of bytes are discarded.

+ +

+Defaults to 16777215, the maximum value of the 24-bit Message +Length field in a Diameter Header.

+ + + {length_errors, exit|handle|discard} @@ -1420,6 +1377,49 @@ Changing this results in non-standard behaviour, but can be useful in case peers are known to be behave badly.

 + +{strict_mbit, boolean()} + +

+Whether or not to regard an AVP setting the M-bit as erroneous when +the command grammar in question does not explicitly allow the AVP. +If true then such AVPs are regarded as 5001 errors, +DIAMETER_AVP_UNSUPPORTED. +If false then the M-bit is ignored and policing +it becomes the receiver's responsibility.

+ +

+Defaults to true.

+ + +

+RFC 6733 is unclear about the semantics of the M-bit. +One the one hand, the CCF specification in section 3.2 documents AVP +in a command grammar as meaning any arbitrary AVP; on the +other hand, 1.3.4 states that AVPs setting the M-bit cannot be added +to an existing command: the modified command must instead be +placed in a new Diameter application.

+

+The reason for the latter is presumably interoperability: +allowing arbitrary AVPs setting the M-bit in a command makes its +interpretation implementation-dependent, since there's no +guarantee that all implementations will understand the same set of +arbitrary AVPs in the context of a given command. +However, interpreting AVP in a command grammar as any +AVP, regardless of M-bit, renders 1.3.4 meaningless, since the receiver +can simply ignore any AVP it thinks isn't relevant, regardless of the +sender's intent.

+

+Beware of confusing mandatory in the sense of the M-bit with mandatory +in the sense of the command grammar. +The former is a semantic requirement: that the receiver understand the +semantics of the AVP in the context in question. +The latter is a syntactic requirement: whether or not the AVP must +occur in the message in question.

+ + + + {transport_config, term()} {transport_config, term(), &dict_Unsigned32; | infinity} diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index 0f919f6c25..3b41feac0d 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -355,9 +355,11 @@ call(SvcName, App, Message) -> | {capabilities_cb, eval()} | {capx_timeout, 'Unsigned32'()} | {strict_capx, boolean()} + | {strict_mbit, boolean()} | {disconnect_cb, eval()} | {dpr_timeout, 'Unsigned32'()} | {dpa_timeout, 'Unsigned32'()} + | {incoming_maxlen, message_length()} | {length_errors, exit | handle | discard} | {connect_timer, 'Unsigned32'()} | {watchdog_timer, 'Unsigned32'() | {module(), atom(), list()}} @@ -376,8 +378,6 @@ call(SvcName, App, Message) -> | {traffic_counters, boolean()} | {string_decode, boolean()} | {strict_arities, true | strict_arities()} - | {strict_mbit, boolean()} - | {incoming_maxlen, message_length()} | {use_shared_peers, remotes()} | common_opt(). diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index a4496f9725..4721be1ca0 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -575,13 +575,11 @@ setopt(K, T) -> %% opt/2 -opt(service, {incoming_maxlen, N}) - when 0 =< N, N < 1 bsl 24 -> - true; +opt(_, {incoming_maxlen, N}) -> + is_integer(N) andalso 0 =< N andalso N < 1 bsl 24; opt(service, {K, B}) - when K == strict_mbit; - K == string_decode; + when K == string_decode; K == traffic_counters -> is_boolean(B); @@ -680,7 +678,9 @@ opt(_, {K, Tmo}) opt(_, {capx_strictness, B}) -> is_boolean(B) andalso {value, {strict_capx, B}}; -opt(_, {strict_capx, B}) -> +opt(_, {K, B}) + when K == strict_capx; + K == strict_mbit -> is_boolean(B); opt(_, {length_errors, T}) -> -- cgit v1.2.3 From ce08fb57a66d20fdb3074ca62b7aac0228825e0a Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sun, 27 Aug 2017 14:53:41 +0200 Subject: Fix influence of decode_format on service events Decoded CER/CEA messages are passed in events messages that can be subscribed to using diameter:subscribe/1. A configured decode_format was not reflected in these, messages always being passed as records. Clarify that strict_arities only applies to message callbacks. --- lib/diameter/doc/src/diameter.xml | 3 +- lib/diameter/src/base/diameter_peer_fsm.erl | 62 ++++++++++++++++------------- lib/diameter/test/diameter_event_SUITE.erl | 11 +++-- 3 files changed, 43 insertions(+), 33 deletions(-) (limited to 'lib') diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml index 3ad24257a5..ad82cafd2f 100644 --- a/lib/diameter/doc/src/diameter.xml +++ b/lib/diameter/doc/src/diameter.xml @@ -962,7 +962,8 @@ Defaults to the empty list.

Whether or not to require that the number of AVPs in a message or -grouped AVP agree with those specified in the dictionary in question. +grouped AVP agree with those specified in the dictionary in question +when passing messages to &man_app; callbacks. If true then mismatches in an outgoing messages cause message encoding to fail, while mismatches in an incoming message are reported as 5005/5009 errors in the errors field of the diameter_packet record diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index 4870b86be5..6c47d8804e 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -128,9 +128,8 @@ %% outgoing DPR; boolean says whether or not %% the request was sent explicitly with %% diameter:call/4. - codec :: #{decode_format := record, + codec :: #{decode_format := diameter:decode_format(), string_decode := boolean(), - strict_arities => diameter:strict_arities(), strict_mbit := boolean(), rfc := 3588 | 6733, ordered_encode := false}, @@ -260,8 +259,7 @@ i({Ack, WPid, {M, Ref} = T, Opts, {SvcOpts, Nodes, Dict0, Svc}}) -> strict_mbit, rfc, ordered_encode], - SvcOpts#{ordered_encode => false, - decode_format => record})}. + SvcOpts#{ordered_encode => false})}. %% The transport returns its local ip addresses so that different %% transports on the same service can use different local addresses. %% The local addresses are put into Host-IP-Address avps here when @@ -812,7 +810,8 @@ handle('DPA' = N, %% service: explicit DPR is counted in the same way %% as other explicitly sent requests. incr(recv, H, Dict0), - incr_rc(recv, diameter_codec:decode(Dict0, Opts, Pkt), Dict0) + {_, RecPkt} = decode(Dict0, Opts, Pkt), + incr_rc(recv, RecPkt, Dict0) end, diameter_peer:close(TPid), {stop, N}; @@ -916,21 +915,30 @@ handle_request(Name, = S) -> ?LOG(recv, Name), incr(recv, H, Dict0), - send_answer(Name, diameter_codec:decode(Dict0, Opts, Pkt), S). + send_answer(Name, decode(Dict0, Opts, Pkt), S). + +%% decode/3 +%% +%% Decode the message as record for diameter_capx, and in the +%% configured format for events. + +decode(Dict0, Opts, Pkt) -> + {diameter_codec:decode(Dict0, Opts, Pkt), + diameter_codec:decode(Dict0, Opts#{decode_format := record}, Pkt)}. %% send_answer/3 -send_answer(Type, ReqPkt, #state{transport = TPid, - dictionary = Dict, - codec = Opts} - = S) -> - incr_error(recv, ReqPkt, Dict), +send_answer(Type, {DecPkt, RecPkt}, #state{transport = TPid, + dictionary = Dict, + codec = Opts} + = S) -> + incr_error(recv, RecPkt, Dict), #diameter_packet{header = H, transport_data = TD} - = ReqPkt, + = RecPkt, - {Msg, PostF} = build_answer(Type, ReqPkt, S), + {Msg, PostF} = build_answer(Type, DecPkt, RecPkt, S), %% An answer message clears the R and T flags and retains the P %% flag. The E flag is set at encode. @@ -958,15 +966,15 @@ eval([F|A], S) -> eval(T, _) -> close(T). -%% build_answer/3 +%% build_answer/4 build_answer('CER', + DecPkt, #diameter_packet{msg = CER, header = #diameter_header{version = ?DIAMETER_VERSION, is_error = false}, - errors = []} - = Pkt, + errors = []}, #state{dictionary = Dict0} = S) -> {SupportedApps, RCaps, CEA} = recv_CER(CER, S), @@ -984,25 +992,25 @@ build_answer('CER', orelse ?THROW(4003), %% DIAMETER_ELECTION_LOST caps_cb(Caps) of - N -> {cea(CEA, N, Dict0), [fun open/5, Pkt, + N -> {cea(CEA, N, Dict0), [fun open/5, DecPkt, SupportedApps, Caps, {accept, inband_security(IS)}]} catch ?FAILURE(Reason) -> - rejected(Reason, {'CER', Reason, Caps, Pkt}, S) + rejected(Reason, {'CER', Reason, Caps, DecPkt}, S) end; %% The error checks below are similar to those in diameter_traffic for %% other messages. Should factor out the commonality. build_answer(Type, + DecPkt, #diameter_packet{header = H, - errors = Es} - = Pkt, + errors = Es}, S) -> {RC, FailedAVP} = result_code(Type, H, Es), - {answer(Type, RC, FailedAVP, S), post(Type, RC, Pkt, S)}. + {answer(Type, RC, FailedAVP, S), post(Type, RC, DecPkt, S)}. inband_security([]) -> ?NO_INBAND_SECURITY; @@ -1174,12 +1182,10 @@ handle_CEA(#diameter_packet{header = H} = S) -> incr(recv, H, Dict0), - #diameter_packet{} - = DPkt - = diameter_codec:decode(Dict0, Opts, Pkt), + {DecPkt, RecPkt} = decode(Dict0, Opts, Pkt), - RC = result_code(incr_rc(recv, DPkt, Dict0)), - {SApps, IS, RCaps} = recv_CEA(DPkt, S), + RC = result_code(incr_rc(recv, RecPkt, Dict0)), + {SApps, IS, RCaps} = recv_CEA(RecPkt, S), #diameter_caps{origin_host = {OH, DH}} = Caps @@ -1202,9 +1208,9 @@ handle_CEA(#diameter_packet{header = H} orelse ?THROW(election_lost), caps_cb(Caps) of - _ -> open(DPkt, SApps, Caps, {connect, hd([_] = IS)}, S) + _ -> open(DecPkt, SApps, Caps, {connect, hd([_] = IS)}, S) catch - ?FAILURE(Reason) -> close({'CEA', Reason, Caps, DPkt}) + ?FAILURE(Reason) -> close({'CEA', Reason, Caps, DecPkt}) end. %% Check more than the result code since the peer could send success %% regardless. If not 2001 then a peer_up callback could do anything diff --git a/lib/diameter/test/diameter_event_SUITE.erl b/lib/diameter/test/diameter_event_SUITE.erl index 57d3427037..a291dde6be 100644 --- a/lib/diameter/test/diameter_event_SUITE.erl +++ b/lib/diameter/test/diameter_event_SUITE.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2013-2016. All Rights Reserved. +%% Copyright Ericsson AB 2013-2017. All Rights Reserved. %% %% Licensed under the Apache License, Version 2.0 (the "License"); %% you may not use this file except in compliance with the License. @@ -63,7 +63,8 @@ {'Host-IP-Address', [?ADDR]}, {'Vendor-Id', 12345}, {'Product-Name', "OTP/diameter"}, - {'Acct-Application-Id', [D:id() || D <- Dicts]} + {'Acct-Application-Id', [D:id() || D <- Dicts]}, + {decode_format, map} | [{application, [{dictionary, D}, {module, #diameter_callback{}}]} || D <- Dicts]]). @@ -111,7 +112,8 @@ up(Config) -> {Svc, Ref} = connect(Config, [{connect_timer, 5000}, {watchdog_timer, 15000}]), start = event(Svc), - {up, Ref, {TPid, Caps}, Cfg, #diameter_packet{}} = event(Svc), + {up, Ref, {TPid, Caps}, Cfg, #diameter_packet{msg = M}} = event(Svc), + ['CEA' | #{}] = M, %% assert {watchdog, Ref, _, {initial, okay}, _} = event(Svc), %% Kill the transport process and see that the connection is %% reestablished after a watchdog timeout, not after connect_timer @@ -131,8 +133,9 @@ down(Config) -> {connect_timer, 5000}, {watchdog_timer, 20000}]), start = event(Svc), - {closed, Ref, {'CEA', ?NO_COMMON_APP, _, #diameter_packet{}}, _} + {closed, Ref, {'CEA', ?NO_COMMON_APP, _, #diameter_packet{msg = M}}, _} = event(Svc), + ['CEA' | #{}] = M, %% assert {reconnect, Ref, _} = event(Svc, 4000, 10000). %% Connect with matching capabilities but have the server delay its -- cgit v1.2.3