From b9f2d5a867806563c11f8b13e7977f8d03b64a54 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 3 Jul 2017 11:49:23 +0200 Subject: Fix obsolete diameter_gen.hrl comments Most of the contents were moved to module diameter_gen in commit 205521d3. --- lib/diameter/src/base/diameter_codec.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index 82fa796e69..93c1b28f4c 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -29,7 +29,7 @@ msg_name/2, msg_id/1]). -%% Towards generated encoders (from diameter_gen.hrl). +%% towards diameter_gen -export([pack_data/2, pack_avp/2]). -- cgit v1.2.3 From e30c38a44bbe2872e5b9b0ad46774c19b6af5292 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 3 Jul 2017 16:40:51 +0200 Subject: Count AVP arities during decode Instead of after, during the check that AVPs have sufficient arity. This makes the arity checks independent of the record decode, which will allow the latter to be made optional. --- lib/diameter/src/base/diameter_gen.erl | 246 ++++++++++++++++++--------------- 1 file changed, 132 insertions(+), 114 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl index e832832876..6f11583868 100644 --- a/lib/diameter/src/base/diameter_gen.erl +++ b/lib/diameter/src/base/diameter_gen.erl @@ -172,14 +172,17 @@ enc_AVP(_Name, {_Dict, _AvpName, _Data} = T, Opts, _) -> when Failed :: [{5000..5999, #diameter_avp{}}]. decode_avps(Name, Recs, #{module := Mod} = Opts) -> - {Avps, {Rec, Failed}} + {Avps, {Rec, AM, Failed}} = mapfoldl(fun(T,A) -> decode(Name, Opts, Mod, T, A) end, - {newrec(Mod, Name), []}, + {newrec(Mod, Name), #{}, []}, Recs), - {Rec, Avps, Failed ++ missing(Rec, Name, Failed, Opts, Mod)}. + %% AM counts the number of top-level AVPs, which missing/4 then + %% uses when adding 5005 errors. + {Rec, Avps, Failed ++ missing(Name, Opts, Mod, AM)}. + %% Append 5005 errors so that errors are reported in the order %% encountered. Failed-AVP should typically contain the first -%% encountered error accordg to the RFC. +%% error encountered. %% mapfoldl/3 %% @@ -194,6 +197,8 @@ mapfoldl(F, Acc0, [T|Rest], List) -> mapfoldl(_, Acc, [], List) -> {List, Acc}. +%% missing/4 + %% 3588: %% %% DIAMETER_MISSING_AVP 5005 @@ -204,57 +209,41 @@ mapfoldl(_, Acc, [], List) -> %% Vendor-Id if applicable. The value field of the missing AVP %% should be of correct minimum length and contain zeros. -missing(Rec, Name, Failed, Opts, Mod) -> - Avps = lists:foldl(fun({_, #diameter_avp{code = C, vendor_id = V}}, A) -> - maps:put({C,V}, true, A) - end, - maps:new(), - Failed), - missing(Mod:avp_arity(Name), tl(tuple_to_list(Rec)), Avps, Opts, Mod, []). - -missing([{Name, Arity} | As], [Value | Vs], Avps, Opts, Mod, Acc) -> - missing(As, - Vs, - Avps, - Opts, - Mod, - case - [H || missing_arity(Arity, Value), - {C,_,V} = H <- [Mod:avp_header(Name)], - not maps:is_key({C,V}, Avps)] - of - [H] -> - [{5005, empty_avp(Name, H, Opts, Mod)} | Acc]; - [] -> - Acc - end); - -missing([], [], _, _, _, Acc) -> - Acc. - -%% Maximum arities have already been checked in building the record. - -missing_arity(1, V) -> - V == undefined; -missing_arity({0, _}, _) -> - false; -missing_arity({1, _}, L) -> - [] == L; -missing_arity({Min, _}, L) -> - not has_prefix(Min, L). - -%% Compare a non-negative integer and the length of a list without -%% computing the length. -has_prefix(0, _) -> - true; -has_prefix(_, []) -> +missing(Name, Opts, Mod, AM) -> + lists:foldl(fun(T,A) -> missing(T, AM, Opts, Mod, A) end, + [], + Mod:avp_arity(Name)). + +%% missing/5 + +missing({Name, Arity}, AM, Opts, Mod, Acc) -> + case missing(Name, Arity, AM) of + true -> [{5005, empty_avp(Name, Opts, Mod)} | Acc]; + false -> Acc + end. + +%% missing/3 + +missing(Name, Arity, AM) -> + 'AVP' /= Name andalso too_few(Name, Arity, AM). + +%% too_few/3 +%% +%% Maximum arities have already been checked during the decode. + +too_few(_, {0, _}, _) -> false; -has_prefix(N, [_|L]) -> - has_prefix(N-1, L). -%% empty_avp/4 +too_few(FieldName, 1, AM) -> + not maps:is_key(FieldName, AM); + +too_few(FieldName, {M, _}, AM) -> + maps:get(FieldName, AM, 0) < M. -empty_avp(Name, {Code, Flags, VId}, Opts, Mod) -> +%% empty_avp/3 + +empty_avp(Name, Opts, Mod) -> + {Code, Flags, VId} = Mod:avp_header(Name), {Name, Type} = Mod:avp_name(Code, VId), #diameter_avp{name = Name, code = Code, @@ -280,8 +269,27 @@ decode(Name, Mod, #diameter_avp{code = Code, vendor_id = Vid} = Avp, - Acc) -> - decode(Name, Opts, Mod, Mod:avp_name(Code, Vid), Avp, Acc). + {Rec, AM, Failed}) -> + T = Mod:avp_name(Code, Vid), + decode(Name, Opts, Mod, T, Avp, {Rec, incr(field(T), AM), Failed}). + +%% field/1 + +field({AvpName, _Type}) -> + AvpName; + +field('AVP' = A) -> + A. + +%% incr/2 + +incr(Key, Map) -> + maps:update_with(Key, fun incr/1, 1, Map). + +%% incr/1 + +incr(N) -> + N + 1. %% decode/6 @@ -352,15 +360,13 @@ decode(Name, Opts0, Mod, {AvpName, Type}, Avp, Acc) -> %% decode is packed into 'AVP'. %% Reset the dictionary for best-effort decode of Failed-AVP. - DecMod = if Failed -> - AppMod; - true -> - Mod + DecMod = if Failed -> AppMod; + true -> Mod end, - %% On decode, a Grouped AVP is represented as a #diameter_avp{} - %% list with AVP as head and component AVPs as tail. On encode, - %% data can be a list of component AVPs. + %% A Grouped AVP is represented as a #diameter_avp{} list with AVP + %% as head and component AVPs as tail. On encode, data can be a + %% list of component AVPs. try avp_decode(Data, AvpName, Opts, DecMod, Mod) of {Rec, As} when Type == 'Grouped' -> @@ -425,7 +431,7 @@ trim(Avp) -> %% decode_error/7 -decode_error(Name, [_ | Rec], _, #{failed_avp := true} = Opts, Mod, Avp, Acc) -> +decode_error(Name, [_|Rec], _, #{failed_avp := true} = Opts, Mod, Avp, Acc) -> decode_AVP(Name, Avp#diameter_avp{value = Rec}, Opts, Mod, Acc); decode_error(Name, _, _, #{failed_avp := true} = Opts, Mod, Avp, Acc) -> @@ -437,24 +443,25 @@ decode_error(_, [Error | _], ComponentAvps, _, _, Avp, Acc) -> decode_error(_, Error, ComponentAvps, _, _, Avp, Acc) -> decode_error(Error, Avp, Acc, ComponentAvps). -%% decode_error/5 +%% decode_error/6 decode_error(Name, _Reason, #{failed_avp := true} = Opts, Mod, Avp, Acc) -> decode_AVP(Name, Avp, Opts, Mod, Acc); -decode_error(Name, Reason, Opts, Mod, Avp, {Rec, Failed}) -> +decode_error(Name, Reason, Opts, Mod, Avp, {Rec, AM, Failed}) -> Stack = diameter_lib:get_stacktrace(), + AvpName = Avp#diameter_avp.name, diameter_lib:log(decode_error, ?MODULE, ?LINE, - {Reason, Name, Avp#diameter_avp.name, Mod, Stack}), - {Avp, {Rec, [rc(Reason, Avp, Opts, Mod) | Failed]}}. + {Reason, Name, AvpName, Mod, Stack}), + {Avp, {Rec, AM, [rc(Reason, Avp, Opts, Mod) | Failed]}}. %% decode_error/4 -decode_error({RC, ErrorData}, Avp, {Rec, Failed}, ComponentAvps) -> +decode_error({RC, ErrorData}, Avp, {Rec, AM, Failed}, ComponentAvps) -> E = Avp#diameter_avp{data = [ErrorData]}, - {[Avp | trim(ComponentAvps)], {Rec, [{RC, E} | Failed]}}. + {[Avp | trim(ComponentAvps)], {Rec, AM, [{RC, E} | Failed]}}. %% set_strict/3 @@ -490,8 +497,8 @@ decode_AVP(Name, Avp, Opts, Mod, Acc) -> %% DIAMETER_INVALID_AVP_LENGTH (5014). A module specified to a %% @custom_types tag in a dictionary file can also raise an error of %% this form. -rc({'DIAMETER', 5014 = RC, _}, #diameter_avp{name = AvpName} = Avp, Opts, Mod) -> - {RC, Avp#diameter_avp{data = Mod:empty_value(AvpName, Opts)}}; +rc({'DIAMETER', 5014 = RC, _}, #diameter_avp{name = AvpName} = A, Opts, Mod) -> + {RC, A#diameter_avp{data = Mod:empty_value(AvpName, Opts)}}; %% 3588: %% @@ -531,20 +538,33 @@ pack_avp(_, Arity, #diameter_avp{name = AvpName} = Avp, _Opts, Mod, Acc) -> %% type. pack_AVP(_, #diameter_avp{data = {5014 = RC, Data}} = Avp, _, _, Acc) -> - {Rec, Failed} = Acc, - {Rec, [{RC, Avp#diameter_avp{data = Data}} | Failed]}; + {Rec, AM, Failed} = Acc, + {Rec, AM, [{RC, Avp#diameter_avp{data = Data}} | Failed]}; pack_AVP(Name, Avp, Opts, Mod, Acc) -> - pack_arity(Name, pack_arity(Name, Opts, Mod, Avp), Avp, Mod, Acc). - -%% pack_arity/5 + Arity = pack_arity(Name, Opts, Mod, Avp), + if 0 == Arity -> + M = Avp#diameter_avp.is_mandatory, + {Rec, AM, Failed} = Acc, + {Rec, AM, [{if M -> 5001; true -> 5008 end, Avp} | Failed]}; + true -> + pack(Arity, 'AVP', Avp, Mod, Acc) + end. -pack_arity(_, 0, #diameter_avp{is_mandatory = M} = Avp, _, Acc) -> - {Rec, Failed} = Acc, - {Rec, [{if M -> 5001; true -> 5008 end, Avp} | Failed]}; +%% 3588: +%% +%% DIAMETER_AVP_UNSUPPORTED 5001 +%% The peer received a message that contained an AVP that is not +%% recognized or supported and was marked with the Mandatory bit. A +%% Diameter message with this error MUST contain one or more Failed- +%% AVP AVP containing the AVPs that caused the failure. +%% +%% DIAMETER_AVP_NOT_ALLOWED 5008 +%% A message was received with an AVP that MUST NOT be present. The +%% Failed-AVP AVP MUST be included and contain a copy of the +%% offending AVP. -pack_arity(_, Arity, Avp, Mod, Acc) -> - pack(Arity, 'AVP', Avp, Mod, Acc). +%% pack_arity/4 %% Give Failed-AVP special treatment since (1) it'll contain any %% unrecognized mandatory AVP's and (2) the RFC 3588 grammar failed to @@ -573,31 +593,13 @@ pack_arity(Name, 0 end. -%% 3588: -%% -%% DIAMETER_AVP_UNSUPPORTED 5001 -%% The peer received a message that contained an AVP that is not -%% recognized or supported and was marked with the Mandatory bit. A -%% Diameter message with this error MUST contain one or more Failed- -%% AVP AVP containing the AVPs that caused the failure. -%% -%% DIAMETER_AVP_NOT_ALLOWED 5008 -%% A message was received with an AVP that MUST NOT be present. The -%% Failed-AVP AVP MUST be included and contain a copy of the -%% offending AVP. - %% pack/5 -pack(Arity, FieldName, Avp, Mod, {Rec, _} = Acc) -> - pack(Mod:'#get-'(FieldName, Rec), Arity, FieldName, Avp, Mod, Acc). - -%% pack/6 - -pack(undefined, 1, 'AVP' = F, Avp, Mod, {Rec, Failed}) -> %% unlikely - {Mod:'#set-'({F, Avp}, Rec), Failed}; - -pack(undefined, 1, F, #diameter_avp{value = V}, Mod, {Rec, Failed}) -> - {Mod:'#set-'({F, V}, Rec), Failed}; +pack(Arity, F, Avp, Mod, {Rec, AM, Failed}) -> + case too_many(F, Arity, AM) of + true -> {Rec, AM, [{5009, Avp} | Failed]}; + false -> {set(Arity, F, value(F, Avp), Mod, Rec), AM, Failed} + end. %% 3588: %% @@ -608,18 +610,34 @@ pack(undefined, 1, F, #diameter_avp{value = V}, Mod, {Rec, Failed}) -> %% the offending AVP that exceeded the maximum number of occurrences %% -pack(_, 1, _, Avp, _, {Rec, Failed}) -> - {Rec, [{5009, Avp} | Failed]}; +%% too_many/3 -pack(L, {_, Max}, F, Avp, Mod, {Rec, Failed}) -> - case '*' /= Max andalso has_prefix(Max+1, L) of - true -> - {Rec, [{5009, Avp} | Failed]}; - false when F == 'AVP' -> - {Mod:'#set-'({F, [Avp | L]}, Rec), Failed}; - false -> - {Mod:'#set-'({F, [Avp#diameter_avp.value | L]}, Rec), Failed} - end. +too_many(_, {_, '*'}, _) -> + false; + +too_many(FieldName, {_, M}, Map) -> + too_many(FieldName, M, Map); + +too_many(FieldName, M, Map) -> + #{FieldName := N} = Map, + M < N. + +%% set/5 + +set(1, F, Value, Mod, Rec) -> + Mod:'#set-'({F, Value}, Rec); + +set(_, F, V, Mod, Rec) -> + Vs = Mod:'#get-'(F, Rec), + Mod:'#set-'({F, [V|Vs]}, Rec). + +%% value/2 + +value('AVP', Avp) -> + Avp; + +value(_, #diameter_avp{value = V}) -> + V. %% --------------------------------------------------------------------------- %% # grouped_avp/3 -- cgit v1.2.3 From 722fa41564381dff0b7aa2b465193db30bb2f02f Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 6 Jul 2017 09:58:07 +0200 Subject: Add service_opt() record_decode To control whether or not messages and grouped AVPs are decoded to records, in #diameter_packet.msg and #diameter_avp.value respectively. The decode became unnecessary for diameter's needs in parent commit, which decoupled it from the checking of AVP arities. --- lib/diameter/src/base/diameter.erl | 1 + lib/diameter/src/base/diameter_codec.erl | 3 ++- lib/diameter/src/base/diameter_config.erl | 3 +++ lib/diameter/src/base/diameter_gen.erl | 15 ++++++++++++++- lib/diameter/src/base/diameter_peer_fsm.erl | 9 ++++++--- lib/diameter/src/base/diameter_service.erl | 1 + lib/diameter/src/base/diameter_traffic.erl | 9 ++++++--- lib/diameter/src/base/diameter_watchdog.erl | 9 ++++++--- 8 files changed, 39 insertions(+), 11 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index bd92e16fba..f411656e90 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -338,6 +338,7 @@ call(SvcName, App, Message) -> | {restrict_connections, restriction()} | {sequence, sequence() | evaluable()} | {share_peers, remotes()} + | {record_decode, boolean()} | {string_decode, boolean()} | {strict_mbit, boolean()} | {incoming_maxlen, message_length()} diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index 93c1b28f4c..5e4c6e6d8f 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -287,7 +287,8 @@ rec2msg(Mod, Rec) -> %% longer *the* decode. decode(Mod, Pkt) -> - Opts = #{string_decode => true, + Opts = #{record_decode => true, + string_decode => true, strict_mbit => true, rfc => 6733}, decode(Mod, Opts, Pkt). diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index 34018ae6d3..46a3e362ac 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -713,6 +713,7 @@ make_config(SvcName, Opts) -> {nodes, restrict_connections}, {16#FFFFFF, incoming_maxlen}, {true, strict_mbit}, + {true, record_decode}, {true, string_decode}, {[], spawn_opt}]), @@ -756,6 +757,7 @@ opt(K, false = B) K == monitor; K == restrict_connections; K == strict_mbit; + K == record_decode; K == string_decode -> B; @@ -763,6 +765,7 @@ opt(K, true = B) when K == share_peers; K == use_shared_peers; K == strict_mbit; + K == record_decode; K == string_decode -> B; diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl index 6f11583868..4879ad8f6c 100644 --- a/lib/diameter/src/base/diameter_gen.erl +++ b/lib/diameter/src/base/diameter_gen.erl @@ -174,7 +174,7 @@ enc_AVP(_Name, {_Dict, _AvpName, _Data} = T, Opts, _) -> decode_avps(Name, Recs, #{module := Mod} = Opts) -> {Avps, {Rec, AM, Failed}} = mapfoldl(fun(T,A) -> decode(Name, Opts, Mod, T, A) end, - {newrec(Mod, Name), #{}, []}, + {newrec(Mod, Name, Opts), #{}, []}, Recs), %% AM counts the number of top-level AVPs, which missing/4 then %% uses when adding 5005 errors. @@ -624,6 +624,9 @@ too_many(FieldName, M, Map) -> %% set/5 +set(_, _, _, _, undefined = No) -> + No; + set(1, F, Value, Mod, Rec) -> Mod:'#set-'({F, Value}, Rec); @@ -723,5 +726,15 @@ empty(Name, #{module := Mod} = Opts) -> %% ------------------------------------------------------------------------------ +%% newrec/3 + +newrec(_, _, #{record_decode := false}) -> + undefined; + +newrec(Mod, Name, _) -> + newrec(Mod, Name). + +%% newrec/2 + newrec(Mod, Name) -> Mod:'#new-'(Mod:name2rec(Name)). diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index 1b0dc417e5..f2fbb70270 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -128,7 +128,8 @@ %% outgoing DPR; boolean says whether or not %% the request was sent explicitly with %% diameter:call/4. - codec :: #{string_decode := boolean(), + codec :: #{record_decode := true, + string_decode := boolean(), strict_mbit := boolean(), rfc := 3588 | 6733, ordered_encode := false}, @@ -253,11 +254,13 @@ i({Ack, WPid, {M, Ref} = T, Opts, {SvcOpts, Nodes, Dict0, Svc}}) -> length_errors = LengthErr, strict = Strictness, incoming_maxlen = Maxlen, - codec = maps:with([string_decode, + codec = maps:with([record_decode, + string_decode, strict_mbit, rfc, ordered_encode], - SvcOpts#{ordered_encode => false})}. + SvcOpts#{ordered_encode => false, + record_decode => true})}. %% 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 diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl index a976a8b998..6dc4889c82 100644 --- a/lib/diameter/src/base/diameter_service.erl +++ b/lib/diameter/src/base/diameter_service.erl @@ -113,6 +113,7 @@ restrict_connections := diameter:restriction(), incoming_maxlen := diameter:message_length(), strict_mbit := boolean(), + record_decode := boolean(), string_decode := boolean(), spawn_opt := list() | {module(), atom(), list()}}}). diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index 85378babea..228d9802ad 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -76,7 +76,8 @@ service_name :: diameter:service_name(), apps :: [#diameter_app{}], sequence :: diameter:sequence(), - codec :: #{string_decode := boolean(), + codec :: #{record_decode := boolean(), + string_decode := boolean(), strict_mbit := boolean(), incoming_maxlen := diameter:message_length()}}). %% Note that incoming_maxlen is currently handled in diameter_peer_fsm, @@ -102,7 +103,8 @@ make_recvdata([SvcName, PeerT, Apps, SvcOpts | _]) -> peerT = PeerT, apps = Apps, sequence = Mask, - codec = maps:with([string_decode, + codec = maps:with([record_decode, + string_decode, strict_mbit, ordered_encode, incoming_maxlen], @@ -1933,7 +1935,8 @@ choose(false, _, X) -> X. %% Decode options sufficient for AVP extraction. decode_opts(Dict) -> - #{string_decode => false, + #{record_decode => true, + string_decode => false, strict_mbit => false, failed_avp => false, dictionary => Dict}. diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl index a63425d92a..c3dc8c3bf0 100644 --- a/lib/diameter/src/base/diameter_watchdog.erl +++ b/lib/diameter/src/base/diameter_watchdog.erl @@ -72,7 +72,8 @@ restrict := boolean(), suspect := non_neg_integer(), %% OKAY -> SUSPECT okay := non_neg_integer()}, %% REOPEN -> OKAY - codec :: #{string_decode := false, + codec :: #{record_decode := false, + string_decode := false, strict_mbit := boolean(), failed_avp := false, rfc := 3588 | 6733, @@ -135,7 +136,8 @@ 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 = [string_decode, + CodecKeys = [record_decode, + string_decode, strict_mbit, incoming_maxlen, spawn_opt, @@ -155,7 +157,8 @@ i({Ack, T, Pid, {Opts, suspect => 1, okay => 3}, Opts)), - codec = maps:with(CodecKeys, SvcOpts#{string_decode := false, + codec = maps:with(CodecKeys, SvcOpts#{record_decode := false, + string_decode := false, ordered_encode => false})}. wait(Ref, Pid) -> -- cgit v1.2.3 From 1b3b64af3d9a5441b6da37cf4e97b59cb043f33b Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 6 Jul 2017 11:02:31 +0200 Subject: Let messages and grouped AVPs be encoded/decoded from/to maps With {record_decode, map}. The option name is arguably a bit misleading now, but not too objectionable given that the encode/decode in question has historically only been of records. One advantage of the map decode is that the map only contains values for those AVPs existing in the message or grouped AVP in question. The name of the message or grouped AVP is stored in with key ':name', the leading colon ensuring that the key isn't a diameter-name. Decoding to maps makes the hrl files generated from dictionary files largely irrelevant. There are value defines generated into these, but they're typically so long as to be unusable. --- lib/diameter/src/base/diameter.erl | 2 +- lib/diameter/src/base/diameter_codec.erl | 3 ++ lib/diameter/src/base/diameter_config.erl | 3 ++ lib/diameter/src/base/diameter_gen.erl | 28 +++++++++- lib/diameter/src/base/diameter_service.erl | 2 +- lib/diameter/src/base/diameter_traffic.erl | 87 ++++++++++++++++++++++-------- 6 files changed, 99 insertions(+), 26 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index f411656e90..4269c84cd2 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -338,7 +338,7 @@ call(SvcName, App, Message) -> | {restrict_connections, restriction()} | {sequence, sequence() | evaluable()} | {share_peers, remotes()} - | {record_decode, boolean()} + | {record_decode, boolean() | map} | {string_decode, boolean()} | {strict_mbit, boolean()} | {incoming_maxlen, message_length()} diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index 5e4c6e6d8f..0c43d52093 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -275,6 +275,9 @@ rec2msg(_, [Name|_]) when is_atom(Name) -> Name; +rec2msg(_, #{':name' := Name}) -> + Name; + rec2msg(Mod, Rec) -> Mod:rec2msg(element(1, Rec)). diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index 46a3e362ac..a790b946c1 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -769,6 +769,9 @@ opt(K, true = B) K == string_decode -> B; +opt(record_decode, map = T) -> + T; + opt(restrict_connections, T) when T == node; T == nodes -> diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl index 4879ad8f6c..be2e221b7e 100644 --- a/lib/diameter/src/base/diameter_gen.erl +++ b/lib/diameter/src/base/diameter_gen.erl @@ -50,7 +50,9 @@ %% # encode_avps/3 %% --------------------------------------------------------------------------- --spec encode_avps(parent_name(), parent_record() | avp_values(), map()) +-spec encode_avps(parent_name(), + parent_record() | avp_values() | map(), + map()) -> iolist() | no_return(). @@ -83,6 +85,11 @@ encode(Name, Vals, Opts, Mod) when is_list(Vals) -> encode(Name, Mod:'#set-'(Vals, newrec(Mod, Name)), Opts, Mod); +encode(Name, Map, Opts, Mod) + when is_map(Map) -> + [enc(Name, F, A, V, Opts, Mod) || {F,A} <- Mod:avp_arity(Name), + V <- [maps:get(F, Map, def(A))]]; + encode(Name, Rec, Opts, Mod) -> [encode(Name, F, V, Opts, Mod) || {F,V} <- Mod:'#get-'(Rec)]. @@ -627,6 +634,14 @@ too_many(FieldName, M, Map) -> set(_, _, _, _, undefined = No) -> No; +set(1, F, Value, _, Map) + when is_map(Map) -> + maps:put(F, Value, Map); + +set(_, F, V, _, Map) + when is_map(Map) -> + maps:update_with(F, fun(Vs) -> [V|Vs] end, [V], Map); + set(1, F, Value, Mod, Rec) -> Mod:'#set-'({F, Value}, Rec); @@ -731,6 +746,9 @@ empty(Name, #{module := Mod} = Opts) -> newrec(_, _, #{record_decode := false}) -> undefined; +newrec(_, Name, #{record_decode := map}) -> + #{':name' => Name}; + newrec(Mod, Name, _) -> newrec(Mod, Name). @@ -738,3 +756,11 @@ newrec(Mod, Name, _) -> newrec(Mod, Name) -> Mod:'#new-'(Mod:name2rec(Name)). + +%% def/1 + +def(1) -> + undefined; + +def(_) -> + []. diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl index 6dc4889c82..3e555f6263 100644 --- a/lib/diameter/src/base/diameter_service.erl +++ b/lib/diameter/src/base/diameter_service.erl @@ -113,7 +113,7 @@ restrict_connections := diameter:restriction(), incoming_maxlen := diameter:message_length(), strict_mbit := boolean(), - record_decode := boolean(), + record_decode := boolean() | map, string_decode := boolean(), spawn_opt := list() | {module(), atom(), list()}}}). diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index 228d9802ad..f7dec2441f 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -76,7 +76,7 @@ service_name :: diameter:service_name(), apps :: [#diameter_app{}], sequence :: diameter:sequence(), - codec :: #{record_decode := boolean(), + codec :: #{record_decode := boolean() | map, string_decode := boolean(), strict_mbit := boolean(), incoming_maxlen := diameter:message_length()}}). @@ -625,6 +625,12 @@ is_answer_message([#diameter_header{is_request = R, is_error = E} | _], _) -> is_answer_message([Name | _], _) -> Name == 'answer-message'; +%% Message sent as a map. +is_answer_message(Map, _) + when is_map(Map) -> + #{':name' := Name} = Map, + Name == 'answer-message'; + %% Message sent as a record. is_answer_message(Rec, Dict) -> try @@ -873,6 +879,11 @@ reset(Msg, [RC | Avps], Dict) -> set([_|_] = Ans, Avps, _) -> Ans ++ Avps; %% Values nearer tail take precedence. +%% ... a map ... +set(Ans, Avps, _) + when is_map(Ans) -> + maps:merge(Ans, maps:from_list(Avps)); + %% ... or record. set(Rec, Avps, Dict) -> Dict:'#set-'(Avps, Rec). @@ -891,6 +902,9 @@ rc([MsgName | _], RC, Dict) -> _ -> [] end; +rc(#{':name' := Name}, RC, Dict) -> + rc([Name], RC, Dict); + rc(Rec, RC, Dict) -> rc([Dict:rec2msg(element(1, Rec))], RC, Dict). @@ -902,34 +916,50 @@ failed_avp(_, [] = No, _) -> failed_avp(Msg, [_|_] = Avps, Dict) -> [failed(Msg, [{'AVP', Avps}], Dict)]. -%% Reply as name and tuple list ... -failed([MsgName | Values], FailedAvp, Dict) -> - RecName = Dict:msg2rec(MsgName), - try - Dict:'#info-'(RecName, {index, 'Failed-AVP'}), - {'Failed-AVP', [FailedAvp]} - catch - error: _ -> - Avps = proplists:get_value('AVP', Values, []), - A = #diameter_avp{name = 'Failed-AVP', - value = FailedAvp}, - {'AVP', [A|Avps]} - end; +%% failed/3 -%% ... or record. -failed(Rec, FailedAvp, Dict) -> +failed(Msg, FailedAvp, Dict) -> + RecName = msg2rec(Msg, Dict), try - RecName = element(1, Rec), - Dict:'#info-'(RecName, {index, 'Failed-AVP'}), + Dict:'#info-'(RecName, {index, 'Failed-AVP'}), %% assert existence {'Failed-AVP', [FailedAvp]} catch error: _ -> - Avps = Dict:'#get-'('AVP', Rec), + Avps = values(Msg, 'AVP', Dict), A = #diameter_avp{name = 'Failed-AVP', value = FailedAvp}, {'AVP', [A|Avps]} end. +%% msg2rec/2 + +%% Message as name/values list ... +msg2rec([MsgName | _], Dict) -> + Dict:msg2rec(MsgName); + +%% ... map ... +msg2rec(#{':name' := MsgName}, Dict) -> + Dict:msg2rec(MsgName); + +%% ... or record. +msg2rec(Rec, _) -> + element(1, Rec). + +%% values/2 + +%% Message as name/values list ... +values([_ | Avps], F, _) -> + proplists:get_value(F, Avps, []); + +%% ... map ... +values(Msg, F, _) + when is_map(Msg) -> + maps:get(F, Msg, []); + +%% ... or record. +values(Rec, F, Dict) -> + Dict:'#get-'(F, Rec). + %% 3. Diameter Header %% %% E(rror) - If set, the message contains a protocol error, @@ -1861,7 +1891,7 @@ str(T) -> get_avp(?RELAY, Name, Msg) -> get_avp(?BASE, Name, Msg); -%% Message as a header/avps list. +%% Message is a header/avps list. get_avp(Dict, Name, [#diameter_header{} | Avps]) -> try {Code, _, VId} = Dict:avp_header(Name), @@ -1874,7 +1904,7 @@ get_avp(Dict, Name, [#diameter_header{} | Avps]) -> undefined end; -%% Outgoing message as a name/values list. +%% Message as name/values list ... get_avp(_, Name, [_MsgName | Avps]) -> case lists:keyfind(Name, 1, Avps) of {_, V} -> @@ -1883,7 +1913,17 @@ get_avp(_, Name, [_MsgName | Avps]) -> undefined end; -%% Message is typically a record but not necessarily. +%% ... map ... +get_avp(_, Name, Map) + when is_map(Map) -> + case maps:find(Name, Map) of + {ok, V} -> + #diameter_avp{name = Name, value = V}; + error -> + undefined + end; + +%% ... or record (but not necessarily). get_avp(Dict, Name, Rec) -> try #diameter_avp{name = Name, value = Dict:'#get-'(Name, Rec)} @@ -1913,7 +1953,8 @@ ungroup(Avp) -> avp_decode(Dict, Name, #diameter_avp{value = undefined, data = Bin} - = Avp) -> + = Avp) + when is_binary(Bin) -> try Dict:avp(decode, Bin, Name, decode_opts(Dict)) of V -> Avp#diameter_avp{value = V} -- cgit v1.2.3 From d52611e9bd0628affa7b4f56a6126e4a99b69a7a Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 6 Jul 2017 12:07:36 +0200 Subject: Let messages and grouped AVPs be decoded to lists That is, decode to the same format that encode already accepts. Only a message has its name at the head of the list since AVPs are already name/value pairs. --- lib/diameter/src/base/diameter.erl | 2 +- lib/diameter/src/base/diameter_codec.erl | 8 ++++++- lib/diameter/src/base/diameter_config.erl | 4 +++- lib/diameter/src/base/diameter_gen.erl | 35 ++++++++++++++++++++---------- lib/diameter/src/base/diameter_service.erl | 2 +- lib/diameter/src/base/diameter_traffic.erl | 2 +- 6 files changed, 36 insertions(+), 17 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index 4269c84cd2..b033632c47 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -338,7 +338,7 @@ call(SvcName, App, Message) -> | {restrict_connections, restriction()} | {sequence, sequence() | evaluable()} | {share_peers, remotes()} - | {record_decode, boolean() | map} + | {record_decode, boolean() | list | map} | {string_decode, boolean()} | {strict_mbit, boolean()} | {incoming_maxlen, message_length()} diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index 0c43d52093..9043af145e 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -379,10 +379,16 @@ decode_avps(MsgName, Mod, AppMod, Opts, Pkt, Avps) -> %% ... or not Opts#{dictionary => AppMod, failed_avp => false}), ?LOGC([] /= Errors, decode_errors, Pkt#diameter_packet.header), - Pkt#diameter_packet{msg = Rec, + Pkt#diameter_packet{msg = reformat(MsgName, Rec, Opts), errors = Errors, avps = As}. +reformat(MsgName, Avps, #{decode_format := list}) -> + [MsgName | Avps]; + +reformat(_, Msg, _) -> + Msg. + %%% --------------------------------------------------------------------------- %%% # decode_header/1 %%% --------------------------------------------------------------------------- diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index a790b946c1..8f958a67b4 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -769,7 +769,9 @@ opt(K, true = B) K == string_decode -> B; -opt(record_decode, map = T) -> +opt(record_decode, T) + when T == list; + T == map -> T; opt(restrict_connections, T) diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl index be2e221b7e..2381b73d07 100644 --- a/lib/diameter/src/base/diameter_gen.erl +++ b/lib/diameter/src/base/diameter_gen.erl @@ -178,14 +178,17 @@ enc_AVP(_Name, {_Dict, _AvpName, _Data} = T, Opts, _) -> -> {parent_record(), [avp()], Failed} when Failed :: [{5000..5999, #diameter_avp{}}]. -decode_avps(Name, Recs, #{module := Mod} = Opts) -> +decode_avps(Name, Recs, #{module := Mod, record_decode := Fmt} = Opts) -> {Avps, {Rec, AM, Failed}} = mapfoldl(fun(T,A) -> decode(Name, Opts, Mod, T, A) end, - {newrec(Mod, Name, Opts), #{}, []}, + {newrec(Mod, Name, Fmt), #{}, []}, Recs), %% AM counts the number of top-level AVPs, which missing/4 then %% uses when adding 5005 errors. - {Rec, Avps, Failed ++ missing(Name, Opts, Mod, AM)}. + Arities = Mod:avp_arity(Name), + {reformat(Rec, Arities, Fmt), + Avps, + Failed ++ missing(Arities, Opts, Mod, AM)}. %% Append 5005 errors so that errors are reported in the order %% encountered. Failed-AVP should typically contain the first @@ -216,10 +219,10 @@ mapfoldl(_, Acc, [], List) -> %% Vendor-Id if applicable. The value field of the missing AVP %% should be of correct minimum length and contain zeros. -missing(Name, Opts, Mod, AM) -> +missing(Arities, Opts, Mod, AM) -> lists:foldl(fun(T,A) -> missing(T, AM, Opts, Mod, A) end, [], - Mod:avp_arity(Name)). + Arities). %% missing/5 @@ -631,7 +634,7 @@ too_many(FieldName, M, Map) -> %% set/5 -set(_, _, _, _, undefined = No) -> +set(_, _, _, _, false = No) -> No; set(1, F, Value, _, Map) @@ -743,20 +746,28 @@ empty(Name, #{module := Mod} = Opts) -> %% newrec/3 -newrec(_, _, #{record_decode := false}) -> - undefined; +newrec(_, _, false = No) -> + No; -newrec(_, Name, #{record_decode := map}) -> - #{':name' => Name}; +newrec(Mod, Name, true) -> + newrec(Mod, Name); -newrec(Mod, Name, _) -> - newrec(Mod, Name). +newrec(_, Name, _) -> + #{':name' => Name}. %% newrec/2 newrec(Mod, Name) -> Mod:'#new-'(Mod:name2rec(Name)). +%% reformat/3 + +reformat(Map, Arities, list) -> + [{F,V} || {F,_} <- Arities, #{F := V} <- [Map]]; + +reformat(Rec, _, _) -> + Rec. + %% def/1 def(1) -> diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl index 3e555f6263..7f7e3e3a3f 100644 --- a/lib/diameter/src/base/diameter_service.erl +++ b/lib/diameter/src/base/diameter_service.erl @@ -113,7 +113,7 @@ restrict_connections := diameter:restriction(), incoming_maxlen := diameter:message_length(), strict_mbit := boolean(), - record_decode := boolean() | map, + record_decode := boolean() | map | list, string_decode := boolean(), spawn_opt := list() | {module(), atom(), list()}}}). diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index f7dec2441f..6594994cfa 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -76,7 +76,7 @@ service_name :: diameter:service_name(), apps :: [#diameter_app{}], sequence :: diameter:sequence(), - codec :: #{record_decode := boolean() | map, + codec :: #{record_decode := boolean() | map | list, string_decode := boolean(), strict_mbit := boolean(), incoming_maxlen := diameter:message_length()}}). -- cgit v1.2.3 From 55e65b262cdf0b794ab443928676720a323cf6b0 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sat, 8 Jul 2017 00:48:07 +0200 Subject: Rename record_decode -> decode_format {record_decode, map} is a bit too quirky. --- lib/diameter/src/base/diameter.erl | 6 +++++- lib/diameter/src/base/diameter_codec.erl | 2 +- lib/diameter/src/base/diameter_config.erl | 10 +++++----- lib/diameter/src/base/diameter_gen.erl | 4 ++-- lib/diameter/src/base/diameter_peer_fsm.erl | 6 +++--- lib/diameter/src/base/diameter_service.erl | 2 +- lib/diameter/src/base/diameter_traffic.erl | 6 +++--- lib/diameter/src/base/diameter_watchdog.erl | 6 +++--- 8 files changed, 23 insertions(+), 19 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index b033632c47..75deaad511 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -47,6 +47,7 @@ stop/0]). -export_type([evaluable/0, + decode_format/0, restriction/0, message_length/0, remotes/0, @@ -330,6 +331,9 @@ call(SvcName, App, Message) -> -type message_length() :: 0..16#FFFFFF. +-type decode_format() + :: record | list | map | false. + %% Options passed to start_service/2 -type service_opt() @@ -338,7 +342,7 @@ call(SvcName, App, Message) -> | {restrict_connections, restriction()} | {sequence, sequence() | evaluable()} | {share_peers, remotes()} - | {record_decode, boolean() | list | map} + | {decode_format, decode_format()} | {string_decode, boolean()} | {strict_mbit, boolean()} | {incoming_maxlen, message_length()} diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index 9043af145e..275e80b9bb 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -290,7 +290,7 @@ rec2msg(Mod, Rec) -> %% longer *the* decode. decode(Mod, Pkt) -> - Opts = #{record_decode => true, + Opts = #{decode_format => record, string_decode => true, strict_mbit => true, rfc => 6733}, diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index 8f958a67b4..09018308d5 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -713,7 +713,7 @@ make_config(SvcName, Opts) -> {nodes, restrict_connections}, {16#FFFFFF, incoming_maxlen}, {true, strict_mbit}, - {true, record_decode}, + {record, decode_format}, {true, string_decode}, {[], spawn_opt}]), @@ -757,7 +757,7 @@ opt(K, false = B) K == monitor; K == restrict_connections; K == strict_mbit; - K == record_decode; + K == decode_format; K == string_decode -> B; @@ -765,12 +765,12 @@ opt(K, true = B) when K == share_peers; K == use_shared_peers; K == strict_mbit; - K == record_decode; K == string_decode -> B; -opt(record_decode, T) - when T == list; +opt(decode_format, T) + when T == record; + T == list; T == map -> T; diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl index 2381b73d07..239d4a535f 100644 --- a/lib/diameter/src/base/diameter_gen.erl +++ b/lib/diameter/src/base/diameter_gen.erl @@ -178,7 +178,7 @@ enc_AVP(_Name, {_Dict, _AvpName, _Data} = T, Opts, _) -> -> {parent_record(), [avp()], Failed} when Failed :: [{5000..5999, #diameter_avp{}}]. -decode_avps(Name, Recs, #{module := Mod, record_decode := Fmt} = Opts) -> +decode_avps(Name, Recs, #{module := Mod, decode_format := Fmt} = Opts) -> {Avps, {Rec, AM, Failed}} = mapfoldl(fun(T,A) -> decode(Name, Opts, Mod, T, A) end, {newrec(Mod, Name, Fmt), #{}, []}, @@ -749,7 +749,7 @@ empty(Name, #{module := Mod} = Opts) -> newrec(_, _, false = No) -> No; -newrec(Mod, Name, true) -> +newrec(Mod, Name, record) -> newrec(Mod, Name); newrec(_, Name, _) -> diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index f2fbb70270..abf948593f 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -128,7 +128,7 @@ %% outgoing DPR; boolean says whether or not %% the request was sent explicitly with %% diameter:call/4. - codec :: #{record_decode := true, + codec :: #{decode_format := record, string_decode := boolean(), strict_mbit := boolean(), rfc := 3588 | 6733, @@ -254,13 +254,13 @@ i({Ack, WPid, {M, Ref} = T, Opts, {SvcOpts, Nodes, Dict0, Svc}}) -> length_errors = LengthErr, strict = Strictness, incoming_maxlen = Maxlen, - codec = maps:with([record_decode, + codec = maps:with([decode_format, string_decode, strict_mbit, rfc, ordered_encode], SvcOpts#{ordered_encode => false, - record_decode => true})}. + decode_format => record})}. %% 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 diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl index 7f7e3e3a3f..43be4d889a 100644 --- a/lib/diameter/src/base/diameter_service.erl +++ b/lib/diameter/src/base/diameter_service.erl @@ -113,7 +113,7 @@ restrict_connections := diameter:restriction(), incoming_maxlen := diameter:message_length(), strict_mbit := boolean(), - record_decode := boolean() | map | list, + decode_format := diameter:decode_format(), string_decode := boolean(), spawn_opt := list() | {module(), atom(), list()}}}). diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index 6594994cfa..f684f60cb7 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -76,7 +76,7 @@ service_name :: diameter:service_name(), apps :: [#diameter_app{}], sequence :: diameter:sequence(), - codec :: #{record_decode := boolean() | map | list, + codec :: #{decode_format := diameter:decode_format(), string_decode := boolean(), strict_mbit := boolean(), incoming_maxlen := diameter:message_length()}}). @@ -103,7 +103,7 @@ make_recvdata([SvcName, PeerT, Apps, SvcOpts | _]) -> peerT = PeerT, apps = Apps, sequence = Mask, - codec = maps:with([record_decode, + codec = maps:with([decode_format, string_decode, strict_mbit, ordered_encode, @@ -1976,7 +1976,7 @@ choose(false, _, X) -> X. %% Decode options sufficient for AVP extraction. decode_opts(Dict) -> - #{record_decode => true, + #{decode_format => record, string_decode => false, strict_mbit => false, failed_avp => false, diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl index c3dc8c3bf0..60baf1e8a4 100644 --- a/lib/diameter/src/base/diameter_watchdog.erl +++ b/lib/diameter/src/base/diameter_watchdog.erl @@ -72,7 +72,7 @@ restrict := boolean(), suspect := non_neg_integer(), %% OKAY -> SUSPECT okay := non_neg_integer()}, %% REOPEN -> OKAY - codec :: #{record_decode := false, + codec :: #{decode_format := false, string_decode := false, strict_mbit := boolean(), failed_avp := false, @@ -136,7 +136,7 @@ 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 = [record_decode, + CodecKeys = [decode_format, string_decode, strict_mbit, incoming_maxlen, @@ -157,7 +157,7 @@ i({Ack, T, Pid, {Opts, suspect => 1, okay => 3}, Opts)), - codec = maps:with(CodecKeys, SvcOpts#{record_decode := false, + codec = maps:with(CodecKeys, SvcOpts#{decode_format := false, string_decode := false, ordered_encode => false})}. -- cgit v1.2.3 From fa2f0572aa0604bf03d4d3eaa358719ffd877545 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sat, 8 Jul 2017 02:06:11 +0200 Subject: Add decode_format record_from_map Undocumented, for transforming a map decode to record. The record decode becomes more expensive the larger the number of AVPs in the message definition in question, since the record is recreated each time an AVP value is set in it. The map decode can potentially do better. --- lib/diameter/src/base/diameter.erl | 6 +++++- lib/diameter/src/base/diameter_config.erl | 3 ++- lib/diameter/src/base/diameter_gen.erl | 13 +++++++++---- 3 files changed, 16 insertions(+), 6 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index 75deaad511..85a54c8e61 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -332,7 +332,11 @@ call(SvcName, App, Message) -> :: 0..16#FFFFFF. -type decode_format() - :: record | list | map | false. + :: record + | list + | map + | false + | record_from_map. %% Options passed to start_service/2 diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index 09018308d5..d591fa903e 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -771,7 +771,8 @@ opt(K, true = B) opt(decode_format, T) when T == record; T == list; - T == map -> + T == map; + T == record_from_map -> T; opt(restrict_connections, T) diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl index 239d4a535f..78d8bd2fa3 100644 --- a/lib/diameter/src/base/diameter_gen.erl +++ b/lib/diameter/src/base/diameter_gen.erl @@ -186,7 +186,7 @@ decode_avps(Name, Recs, #{module := Mod, decode_format := Fmt} = Opts) -> %% AM counts the number of top-level AVPs, which missing/4 then %% uses when adding 5005 errors. Arities = Mod:avp_arity(Name), - {reformat(Rec, Arities, Fmt), + {reformat(Rec, Arities, Mod, Fmt), Avps, Failed ++ missing(Arities, Opts, Mod, AM)}. @@ -760,12 +760,17 @@ newrec(_, Name, _) -> newrec(Mod, Name) -> Mod:'#new-'(Mod:name2rec(Name)). -%% reformat/3 +%% reformat/4 -reformat(Map, Arities, list) -> +reformat(Map, Arities, _Mod, list) -> [{F,V} || {F,_} <- Arities, #{F := V} <- [Map]]; -reformat(Rec, _, _) -> +reformat(Map, Arities, Mod, record_from_map) -> + #{':name' := Name} = Map, + RecName = Mod:name2rec(Name), + list_to_tuple([RecName | [maps:get(F, Map, def(A)) || {F,A} <- Arities]]); + +reformat(Rec, _, _, _) -> Rec. %% def/1 -- cgit v1.2.3 From e0603ba18a67c1ef33f60122fe6f00393c0c0203 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 13 Jul 2017 01:28:06 +0200 Subject: Tweak map-valued decode Use the same [MsgName | Avps] representation as for the list decode, but with Avps a map instead of a AVP name/values list. As a result, don't set the message/AVP name on an additional key in the map, which felt a bit odd. Messages are [MsgName :: atom() | map()], Grouped AVPs are just map(). Fix at least one problem in the traffic suite along the way: with decode_format false, the own decode in to_map/2 didn't know whether or not to decode strings, resulting on some failures. --- lib/diameter/src/base/diameter_codec.erl | 7 ++-- lib/diameter/src/base/diameter_gen.erl | 13 ++++--- lib/diameter/src/base/diameter_traffic.erl | 58 +++++++++++------------------- 3 files changed, 30 insertions(+), 48 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index 275e80b9bb..9b21bf4141 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -275,9 +275,6 @@ rec2msg(_, [Name|_]) when is_atom(Name) -> Name; -rec2msg(_, #{':name' := Name}) -> - Name; - rec2msg(Mod, Rec) -> Mod:rec2msg(element(1, Rec)). @@ -383,7 +380,9 @@ decode_avps(MsgName, Mod, AppMod, Opts, Pkt, Avps) -> %% ... or not errors = Errors, avps = As}. -reformat(MsgName, Avps, #{decode_format := list}) -> +reformat(MsgName, Avps, #{decode_format := T}) + when T == map; + T == list -> [MsgName | Avps]; reformat(_, Msg, _) -> diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl index 78d8bd2fa3..597ec143a8 100644 --- a/lib/diameter/src/base/diameter_gen.erl +++ b/lib/diameter/src/base/diameter_gen.erl @@ -186,7 +186,7 @@ decode_avps(Name, Recs, #{module := Mod, decode_format := Fmt} = Opts) -> %% AM counts the number of top-level AVPs, which missing/4 then %% uses when adding 5005 errors. Arities = Mod:avp_arity(Name), - {reformat(Rec, Arities, Mod, Fmt), + {reformat(Name, Rec, Arities, Mod, Fmt), Avps, Failed ++ missing(Arities, Opts, Mod, AM)}. @@ -752,8 +752,8 @@ newrec(_, _, false = No) -> newrec(Mod, Name, record) -> newrec(Mod, Name); -newrec(_, Name, _) -> - #{':name' => Name}. +newrec(_, _, _) -> + #{}. %% newrec/2 @@ -762,15 +762,14 @@ newrec(Mod, Name) -> %% reformat/4 -reformat(Map, Arities, _Mod, list) -> +reformat(_, Map, Arities, _Mod, list) -> [{F,V} || {F,_} <- Arities, #{F := V} <- [Map]]; -reformat(Map, Arities, Mod, record_from_map) -> - #{':name' := Name} = Map, +reformat(Name, Map, Arities, Mod, record_from_map) -> RecName = Mod:name2rec(Name), list_to_tuple([RecName | [maps:get(F, Map, def(A)) || {F,A} <- Arities]]); -reformat(Rec, _, _, _) -> +reformat(_, Rec, _, _, _) -> Rec. %% def/1 diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index f684f60cb7..e9c422d6ab 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -621,16 +621,10 @@ is_answer_message(#diameter_packet{msg = Msg}, Dict0) -> is_answer_message([#diameter_header{is_request = R, is_error = E} | _], _) -> E andalso not R; -%% Message sent as a tagged avp/value list. +%% Message sent as a map or tagged avp/value list. is_answer_message([Name | _], _) -> Name == 'answer-message'; -%% Message sent as a map. -is_answer_message(Map, _) - when is_map(Map) -> - #{':name' := Name} = Map, - Name == 'answer-message'; - %% Message sent as a record. is_answer_message(Rec, Dict) -> try @@ -875,15 +869,13 @@ reset(Msg, [RC | Avps], Dict) -> %% set/3 -%% Reply as name and tuple list ... +%% Reply as name/values list ... +set([Name|As], Avps, _) + when is_map(As) -> + [Name | maps:merge(As, maps:from_list(Avps))]; set([_|_] = Ans, Avps, _) -> Ans ++ Avps; %% Values nearer tail take precedence. -%% ... a map ... -set(Ans, Avps, _) - when is_map(Ans) -> - maps:merge(Ans, maps:from_list(Avps)); - %% ... or record. set(Rec, Avps, Dict) -> Dict:'#set-'(Avps, Rec). @@ -902,9 +894,6 @@ rc([MsgName | _], RC, Dict) -> _ -> [] end; -rc(#{':name' := Name}, RC, Dict) -> - rc([Name], RC, Dict); - rc(Rec, RC, Dict) -> rc([Dict:rec2msg(element(1, Rec))], RC, Dict). @@ -937,10 +926,6 @@ failed(Msg, FailedAvp, Dict) -> msg2rec([MsgName | _], Dict) -> Dict:msg2rec(MsgName); -%% ... map ... -msg2rec(#{':name' := MsgName}, Dict) -> - Dict:msg2rec(MsgName); - %% ... or record. msg2rec(Rec, _) -> element(1, Rec). @@ -949,12 +934,11 @@ msg2rec(Rec, _) -> %% Message as name/values list ... values([_ | Avps], F, _) -> - proplists:get_value(F, Avps, []); - -%% ... map ... -values(Msg, F, _) - when is_map(Msg) -> - maps:get(F, Msg, []); + if is_map(Avps) -> + maps:get(F, Avps, []); + is_list(Avps) -> + proplists:get_value(F, Avps, []) + end; %% ... or record. values(Rec, F, Dict) -> @@ -1906,23 +1890,13 @@ get_avp(Dict, Name, [#diameter_header{} | Avps]) -> %% Message as name/values list ... get_avp(_, Name, [_MsgName | Avps]) -> - case lists:keyfind(Name, 1, Avps) of + case find(Name, Avps) of {_, V} -> #diameter_avp{name = Name, value = V}; _ -> undefined end; -%% ... map ... -get_avp(_, Name, Map) - when is_map(Map) -> - case maps:find(Name, Map) of - {ok, V} -> - #diameter_avp{name = Name, value = V}; - error -> - undefined - end; - %% ... or record (but not necessarily). get_avp(Dict, Name, Rec) -> try @@ -1932,6 +1906,16 @@ get_avp(Dict, Name, Rec) -> undefined end. +%% find/2 + +find(Key, Map) + when is_map(Map) -> + maps:find(Key, Map); + +find(Key, List) + when is_list(List) -> + lists:keyfind(Key, 1, List). + %% get_avp_value/3 get_avp_value(Dict, Name, Msg) -> -- cgit v1.2.3 From e54f92c328c8fb117de88b1171f64358f1e7d3b1 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sun, 9 Jul 2017 00:38:56 +0200 Subject: Don't take length of AVP lists unnecessarily at encode Count as AVPs are encoded instead. --- lib/diameter/src/base/diameter_gen.erl | 64 +++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 21 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl index 597ec143a8..313be5f215 100644 --- a/lib/diameter/src/base/diameter_gen.erl +++ b/lib/diameter/src/base/diameter_gen.erl @@ -104,7 +104,8 @@ enc(_, AvpName, 1, undefined, _, _) -> ?THROW([mandatory_avp_missing, AvpName]); enc(Name, AvpName, 1, Value, Opts, Mod) -> - enc(Name, AvpName, [Value], Opts, Mod); + H = avp_header(AvpName, Mod), + enc1(Name, AvpName, H, Value, Opts, Mod); enc(_, _, {0,_}, [], _, _) -> []; @@ -113,31 +114,47 @@ enc(_, AvpName, _, T, _, _) when not is_list(T) -> ?THROW([repeated_avp_as_non_list, AvpName, T]); -enc(_, AvpName, {Min, _}, L, _, _) - when length(L) < Min -> - ?THROW([repeated_avp_insufficient_arity, AvpName, Min, L]); +enc(Name, AvpName, {Min, Max}, Values, Opts, Mod) -> + H = avp_header(AvpName, Mod), + enc(Name, AvpName, H, Min, 0, Max, Values, Opts, Mod). -enc(_, AvpName, {_, Max}, L, _, _) - when Max < length(L) -> - ?THROW([repeated_avp_excessive_arity, AvpName, Max, L]); +%% enc/9 -enc(Name, AvpName, _, Values, Opts, Mod) -> - enc(Name, AvpName, Values, Opts, Mod). +enc(_, AvpName, _, Min, N, _, [], _, _) + when N < Min -> + ?THROW([repeated_avp_insufficient_arity, AvpName, Min, N]); -%% enc/5 +enc(_, _, _, _, _, _, [], _, _) -> + []; -enc(Name, 'AVP', Values, Opts, Mod) -> - [enc_AVP(Name, A, Opts, Mod) || A <- Values]; +enc(_, AvpName, _, _, N, Max, _, _, _) + when Max =< N -> + ?THROW([repeated_avp_excessive_arity, AvpName, Max]); -enc(_, AvpName, Values, Opts, Mod) -> - enc(AvpName, Values, Opts, Mod). +enc(Name, AvpName, H, Min, N, Max, [V|Vs], Opts, Mod) -> + [enc1(Name, AvpName, H, V, Opts, Mod) + | enc(Name, AvpName, H, Min, N+1, Max, Vs, Opts, Mod)]. -%% enc/4 +%% avp_header/2 + +avp_header('AVP', _) -> + false; + +avp_header(AvpName, Mod) -> + {_,_,_} = Mod:avp_header(AvpName). + +%% enc1/6 -enc(AvpName, Values, Opts, Mod) -> - H = Mod:avp_header(AvpName), - [diameter_codec:pack_data(H, Mod:avp(encode, V, AvpName, Opts)) - || V <- Values]. +enc1(Name, 'AVP', false, Value, Opts, Mod) -> + enc_AVP(Name, Value, Opts, Mod); + +enc1(_, AvpName, Hdr, Value, Opts, Mod) -> + enc1(AvpName, Hdr, Value, Opts, Mod). + +%% enc1/5 + +enc1(AvpName, {_,_,_} = Hdr, Value, Opts, Mod) -> + diameter_codec:pack_data(Hdr, Mod:avp(encode, Value, AvpName, Opts)). %% enc_AVP/4 @@ -160,16 +177,21 @@ enc_AVP(_, #diameter_avp{name = N, value = V}, _, _) enc_AVP(Name, #diameter_avp{name = AvpName, value = Data}, Opts, Mod) -> 0 == Mod:avp_arity(Name, AvpName) orelse ?THROW([known_avp_as_AVP, Name, AvpName, Data]), - enc(AvpName, [Data], Opts, Mod); + enc(AvpName, Data, Opts, Mod); %% The backdoor ... enc_AVP(_, {AvpName, Value}, Opts, Mod) -> - enc(AvpName, [Value], Opts, Mod); + enc(AvpName, Value, Opts, Mod); %% ... and the side door. enc_AVP(_Name, {_Dict, _AvpName, _Data} = T, Opts, _) -> diameter_codec:pack_avp(#diameter_avp{data = T}, Opts). +%% enc/4 + +enc(AvpName, Value, Opts, Mod) -> + enc1(AvpName, Mod:avp_header(AvpName), Value, Opts, Mod). + %% --------------------------------------------------------------------------- %% # decode_avps/3 %% --------------------------------------------------------------------------- -- cgit v1.2.3 From 246a5d8611e258119fc6bdc6c52772539c8b09ca Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 10 Jul 2017 20:32:02 +0200 Subject: Don't count AVPs unnecessarily at encode Stop counting when there can be no arity errors. --- lib/diameter/src/base/diameter_gen.erl | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl index 313be5f215..a7dc3aaec3 100644 --- a/lib/diameter/src/base/diameter_gen.erl +++ b/lib/diameter/src/base/diameter_gen.erl @@ -127,6 +127,10 @@ enc(_, AvpName, _, Min, N, _, [], _, _) enc(_, _, _, _, _, _, [], _, _) -> []; +enc(Name, AvpName, H, Min, N, '*', Vs, Opts, Mod) + when Min =< N -> + [enc1(Name, AvpName, H, V, Opts, Mod) || V <- Vs]; + enc(_, AvpName, _, _, N, Max, _, _, _) when Max =< N -> ?THROW([repeated_avp_excessive_arity, AvpName, Max]); -- cgit v1.2.3 From df5814ace0461c37389d96e87ef8aae297802b2e Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Wed, 12 Jul 2017 17:42:03 +0200 Subject: Fix detection of 5009 errors As noted in the parent commit, the wrong AVPs were reported, being counted from the back of the message instead of the front. Both 5005 and 5009 errors are now detected after the message is decoded. --- lib/diameter/src/base/diameter_gen.erl | 201 ++++++++++++++++++--------------- 1 file changed, 110 insertions(+), 91 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl index a7dc3aaec3..eee0580080 100644 --- a/lib/diameter/src/base/diameter_gen.erl +++ b/lib/diameter/src/base/diameter_gen.erl @@ -209,14 +209,14 @@ decode_avps(Name, Recs, #{module := Mod, decode_format := Fmt} = Opts) -> = mapfoldl(fun(T,A) -> decode(Name, Opts, Mod, T, A) end, {newrec(Mod, Name, Fmt), #{}, []}, Recs), - %% AM counts the number of top-level AVPs, which missing/4 then - %% uses when adding 5005 errors. + %% AM counts the number of top-level AVPs, which arities/5 then + %% uses when adding 500[59] errors. Arities = Mod:avp_arity(Name), {reformat(Name, Rec, Arities, Mod, Fmt), Avps, - Failed ++ missing(Arities, Opts, Mod, AM)}. + Failed ++ arities(Arities, Opts, Mod, AM, Avps)}. -%% Append 5005 errors so that errors are reported in the order +%% Append arity errors so that errors are reported in the order %% encountered. Failed-AVP should typically contain the first %% error encountered. @@ -233,9 +233,7 @@ mapfoldl(F, Acc0, [T|Rest], List) -> mapfoldl(_, Acc, [], List) -> {List, Acc}. -%% missing/4 - -%% 3588: +%% 3588/6733: %% %% DIAMETER_MISSING_AVP 5005 %% The request did not contain an AVP that is required by the Command @@ -244,40 +242,73 @@ mapfoldl(_, Acc, [], List) -> %% AVP MUST contain an example of the missing AVP complete with the %% Vendor-Id if applicable. The value field of the missing AVP %% should be of correct minimum length and contain zeros. +%% +%% DIAMETER_AVP_OCCURS_TOO_MANY_TIMES 5009 +%% A message was received that included an AVP that appeared more +%% often than permitted in the message definition. The Failed-AVP +%% AVP MUST be included and contain a copy of the first instance of +%% the offending AVP that exceeded the maximum number of occurrences -missing(Arities, Opts, Mod, AM) -> - lists:foldl(fun(T,A) -> missing(T, AM, Opts, Mod, A) end, - [], - Arities). +arities(Arities, Opts, Mod, AM, Avps) -> + [Count, Map | More] + = lists:foldl(fun({N,T}, A) -> more(N, T, Opts, Mod, AM, A) end, + [0, #{}], + Arities), + less(Count, Map, Avps) ++ lists:reverse(More). -%% missing/5 +%% more/6 -missing({Name, Arity}, AM, Opts, Mod, Acc) -> - case missing(Name, Arity, AM) of - true -> [{5005, empty_avp(Name, Opts, Mod)} | Acc]; - false -> Acc - end. +more(_Name, {0, '*'}, _Opts, _Mod, _AM, Acc) -> + Acc; -%% missing/3 +more(Name, {Mn, Mx}, Opts, Mod, AM, Acc) -> + more(Name, Mn, Mx, Opts, Mod, AM, Acc); -missing(Name, Arity, AM) -> - 'AVP' /= Name andalso too_few(Name, Arity, AM). +more(Name, 1, Opts, Mod, AM, Acc) -> + more(Name, 1, 1, Opts, Mod, AM, Acc). -%% too_few/3 -%% -%% Maximum arities have already been checked during the decode. +%% more/7 -too_few(_, {0, _}, _) -> - false; +more(Name, Mn, Mx, Opts, Mod, AM, Acc) -> + macc(Name, Mn, maps:get(Name, AM, 0), Mx, Opts, Mod, Acc). + +%% macc/7 -too_few(FieldName, 1, AM) -> - not maps:is_key(FieldName, AM); +macc(Name, Mn, N, _, Opts, Mod, [M, Map | T]) + when N < Mn -> + [M, Map, {5005, empty_avp(Name, Opts, Mod)} | T]; -too_few(FieldName, {M, _}, AM) -> - maps:get(FieldName, AM, 0) < M. +macc(Name, _, N, Mx, _Opts, _Mod, [M, Map | T]) + when Mx < N -> + K = N - Mx, + [M + K, maps:put(Name, K, Map) | T]; + +macc(_Name, _, _, _, _Opts, _Mod, Acc) -> + Acc. + +%% less/3 + +less(0, _, _) -> + []; + +less(N, Map, [#diameter_avp{name = undefined} | Avps]) -> + less(N, Map, Avps); + +less(N, Map, [#diameter_avp{name = Name} = Avp | Avps]) -> + case Map of + #{Name := 0} -> + [{5009, Avp} | less(N-1, Map, Avps)]; + #{Name := M} -> + less(N, maps:put(Name, M-1, Map), Avps); + _ -> + less(N, Map, Avps) + end. %% empty_avp/3 +empty_avp('AVP', _, _) -> + #diameter_avp{data = <<0:64>>}; + empty_avp(Name, Opts, Mod) -> {Code, Flags, VId} = Mod:avp_header(Name), {Name, Type} = Mod:avp_name(Code, VId), @@ -307,15 +338,25 @@ decode(Name, = Avp, {Rec, AM, Failed}) -> T = Mod:avp_name(Code, Vid), - decode(Name, Opts, Mod, T, Avp, {Rec, incr(field(T), AM), Failed}). + F = field(T), + A = Mod:avp_arity(Name, F), + decode(Name, Opts, Mod, T, A, Avp, {Rec, incr(field(F, A), AM), Failed}). %% field/1 -field({AvpName, _Type}) -> +field({AvpName, _}) -> AvpName; -field('AVP' = A) -> - A. +field(_) -> + 'AVP'. + +%% field/2 + +field(_, 0) -> + 'AVP'; + +field(F, _) -> + F. %% incr/2 @@ -327,11 +368,11 @@ incr(Key, Map) -> incr(N) -> N + 1. -%% decode/6 +%% decode/7 %% AVP not in dictionary. -decode(Name, Opts, Mod, 'AVP', Avp, Acc) -> - decode_AVP(Name, Avp, Opts, Mod, Acc); +decode(Name, Opts, Mod, 'AVP', Arity, Avp, Acc) -> + decode_AVP(Name, Arity, Avp, Opts, Mod, Acc); %% 6733, 4.4: %% @@ -380,7 +421,7 @@ decode(Name, Opts, Mod, 'AVP', Avp, Acc) -> %% defined the RFC's "unrecognized", which is slightly stronger than %% "not defined".) -decode(Name, Opts0, Mod, {AvpName, Type}, Avp, Acc) -> +decode(Name, Opts0, Mod, {AvpName, Type}, Arity, Avp, Acc) -> #diameter_avp{data = Data, is_mandatory = M} = Avp, @@ -409,13 +450,13 @@ decode(Name, Opts0, Mod, {AvpName, Type}, Avp, Acc) -> A = Avp#diameter_avp{name = AvpName, value = Rec, type = Type}, - {[A|As], pack_avp(Name, A, Opts, Mod, Acc)}; + {[A|As], pack_avp(Name, Arity, A, Opts, Mod, Acc)}; V when Type /= 'Grouped' -> A = Avp#diameter_avp{name = AvpName, value = V, type = Type}, - {A, pack_avp(Name, A, Opts, Mod, Acc)} + {A, pack_avp(Name, Arity, A, Opts, Mod, Acc)} catch throw: {?MODULE, {grouped, Error, ComponentAvps}} -> decode_error(Name, @@ -525,7 +566,12 @@ set_failed(_, Opts) -> %% undecoded. Note that the type field is 'undefined' in this case. decode_AVP(Name, Avp, Opts, Mod, Acc) -> - {trim(Avp), pack_AVP(Name, Avp, Opts, Mod, Acc)}. + decode_AVP(Name, Mod:avp_arity(Name, 'AVP'), Avp, Opts, Mod, Acc). + +%% decode_AVP/6 + +decode_AVP(Name, Arity, Avp, Opts, Mod, Acc) -> + {trim(Avp), pack_AVP(Name, Arity, Avp, Opts, Mod, Acc)}. %% rc/2 @@ -545,11 +591,6 @@ rc({'DIAMETER', 5014 = RC, _}, #diameter_avp{name = AvpName} = A, Opts, Mod) -> rc(_, Avp, _, _) -> {5004, Avp}. -%% pack_avp/5 - -pack_avp(Name, #diameter_avp{name = AvpName} = Avp, Opts, Mod, Acc) -> - pack_avp(Name, Mod:avp_arity(Name, AvpName), Avp, Opts, Mod, Acc). - %% pack_avp/6 pack_avp(Name, 0, Avp, Opts, Mod, Acc) -> @@ -560,6 +601,11 @@ pack_avp(_, Arity, #diameter_avp{name = AvpName} = Avp, _Opts, Mod, Acc) -> %% pack_AVP/5 +pack_AVP(Name, Avp, Opts, Mod, Acc) -> + pack_AVP(Name, Mod:avp_arity(Name, 'AVP'), Avp, Opts, Mod, Acc). + +%% pack_AVP/6 + %% Length failure was induced because of a header/payload length %% mismatch. The AVP Length is reset to match the received data if %% this AVP is encoded in an answer message, since the length is @@ -573,17 +619,17 @@ pack_avp(_, Arity, #diameter_avp{name = AvpName} = Avp, _Opts, Mod, Acc) -> %% payload for the AVP's type, but in this case we don't know the %% type. -pack_AVP(_, #diameter_avp{data = {5014 = RC, Data}} = Avp, _, _, Acc) -> +pack_AVP(_, _, #diameter_avp{data = {5014 = RC, Data}} = Avp, _, _, Acc) -> {Rec, AM, Failed} = Acc, {Rec, AM, [{RC, Avp#diameter_avp{data = Data}} | Failed]}; -pack_AVP(Name, Avp, Opts, Mod, Acc) -> - Arity = pack_arity(Name, Opts, Mod, Avp), - if 0 == Arity -> +pack_AVP(Name, Arity, Avp, Opts, Mod, Acc) -> + case pack_AVP(Name, Opts, Arity, Avp) of + false -> M = Avp#diameter_avp.is_mandatory, {Rec, AM, Failed} = Acc, {Rec, AM, [{if M -> 5001; true -> 5008 end, Avp} | Failed]}; - true -> + true -> pack(Arity, 'AVP', Avp, Mod, Acc) end. @@ -600,18 +646,18 @@ pack_AVP(Name, Avp, Opts, Mod, Acc) -> %% Failed-AVP AVP MUST be included and contain a copy of the %% offending AVP. -%% pack_arity/4 +%% pack_AVP/4 %% Give Failed-AVP special treatment since (1) it'll contain any %% unrecognized mandatory AVP's and (2) the RFC 3588 grammar failed to %% allow for Failed-AVP in an answer-message. -pack_arity(Name, - #{strict_mbit := Strict, - failed_avp := Failed}, - Mod, - #diameter_avp{is_mandatory = M, - name = AvpName}) -> +pack_AVP(Name, + #{strict_mbit := Strict, + failed_avp := Failed}, + Arity, + #diameter_avp{is_mandatory = M, + name = AvpName}) -> %% Not testing just Name /= 'Failed-AVP' means we're changing the %% packing of AVPs nested within Failed-AVP, but the point of @@ -619,44 +665,17 @@ pack_arity(Name, %% possible, and failing because a mandatory AVP couldn't be %% packed into a dedicated field defeats that point. - if Failed == true; - Name == 'Failed-AVP'; - Name == 'answer-message', AvpName == 'Failed-AVP'; - not M; - not Strict -> - Mod:avp_arity(Name, 'AVP'); - true -> - 0 - end. + 0 < Arity andalso (Failed == true + orelse Name == 'Failed-AVP' + orelse (Name == 'answer-message' + andalso AvpName == 'Failed-AVP') + orelse not M + orelse not Strict). %% pack/5 pack(Arity, F, Avp, Mod, {Rec, AM, Failed}) -> - case too_many(F, Arity, AM) of - true -> {Rec, AM, [{5009, Avp} | Failed]}; - false -> {set(Arity, F, value(F, Avp), Mod, Rec), AM, Failed} - end. - -%% 3588: -%% -%% DIAMETER_AVP_OCCURS_TOO_MANY_TIMES 5009 -%% A message was received that included an AVP that appeared more -%% often than permitted in the message definition. The Failed-AVP -%% AVP MUST be included and contain a copy of the first instance of -%% the offending AVP that exceeded the maximum number of occurrences -%% - -%% too_many/3 - -too_many(_, {_, '*'}, _) -> - false; - -too_many(FieldName, {_, M}, Map) -> - too_many(FieldName, M, Map); - -too_many(FieldName, M, Map) -> - #{FieldName := N} = Map, - M < N. + {set(Arity, F, value(F, Avp), Mod, Rec), AM, Failed}. %% set/5 -- cgit v1.2.3 From 66bb5251e89487c5fb8c1f10b8ceb2c6c8f31eed Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 13 Jul 2017 01:09:57 +0200 Subject: Add service_opt() strict_arities To be able to disable the relatively expensive check that the number of AVPs received in a message or grouped AVP agrees with the dictionary in question. The may well be easier for the user in handle_request/answer callbacks, when digesting the received message, and in some cases may not be important. The check at encode can also be disabled, allowing messages that don't agree with the dictionary in question to be sent, which can be useful in test (at least). --- lib/diameter/src/base/diameter.erl | 7 ++ lib/diameter/src/base/diameter_config.erl | 8 ++ lib/diameter/src/base/diameter_gen.erl | 125 ++++++++++++++++++++-------- lib/diameter/src/base/diameter_peer_fsm.erl | 1 + lib/diameter/src/base/diameter_service.erl | 3 +- lib/diameter/src/base/diameter_traffic.erl | 2 + lib/diameter/src/base/diameter_watchdog.erl | 9 +- 7 files changed, 114 insertions(+), 41 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index 85a54c8e61..c919ff4c93 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -48,6 +48,7 @@ -export_type([evaluable/0, decode_format/0, + strict_arities/0, restriction/0, message_length/0, remotes/0, @@ -338,6 +339,11 @@ call(SvcName, App, Message) -> | false | record_from_map. +-type strict_arities() + :: false + | encode + | decode. + %% Options passed to start_service/2 -type service_opt() @@ -348,6 +354,7 @@ call(SvcName, App, Message) -> | {share_peers, remotes()} | {decode_format, decode_format()} | {string_decode, boolean()} + | {strict_arities, true | strict_arities()} | {strict_mbit, boolean()} | {incoming_maxlen, message_length()} | {use_shared_peers, remotes()} diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index d591fa903e..2b069f40cc 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -712,6 +712,7 @@ make_config(SvcName, Opts) -> {?NOMASK, sequence}, {nodes, restrict_connections}, {16#FFFFFF, incoming_maxlen}, + {true, strict_arities}, {true, strict_mbit}, {record, decode_format}, {true, string_decode}, @@ -756,6 +757,7 @@ opt(K, false = B) K == use_shared_peers; K == monitor; K == restrict_connections; + K == strict_arities; K == strict_mbit; K == decode_format; K == string_decode -> @@ -764,6 +766,7 @@ opt(K, false = B) opt(K, true = B) when K == share_peers; K == use_shared_peers; + K == strict_arities; K == strict_mbit; K == string_decode -> B; @@ -775,6 +778,11 @@ opt(decode_format, T) T == record_from_map -> T; +opt(strict_arities, T) + when T == encode; + T == decode -> + T; + opt(restrict_connections, T) when T == node; T == nodes -> diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl index eee0580080..243bc7965a 100644 --- a/lib/diameter/src/base/diameter_gen.erl +++ b/lib/diameter/src/base/diameter_gen.erl @@ -46,6 +46,9 @@ -type grouped_avp() :: nonempty_improper_list(#diameter_avp{}, [avp()]). -type avp() :: non_grouped_avp() | grouped_avp(). +%% The arbitrary arity returned from dictionary avp_arity functions. +-define(ANY, {0, '*'}). + %% --------------------------------------------------------------------------- %% # encode_avps/3 %% --------------------------------------------------------------------------- @@ -88,18 +91,26 @@ encode(Name, Vals, Opts, Mod) encode(Name, Map, Opts, Mod) when is_map(Map) -> [enc(Name, F, A, V, Opts, Mod) || {F,A} <- Mod:avp_arity(Name), - V <- [maps:get(F, Map, def(A))]]; + V <- [maps:get(F, Map, undefined)]]; encode(Name, Rec, Opts, Mod) -> [encode(Name, F, V, Opts, Mod) || {F,V} <- Mod:'#get-'(Rec)]. %% encode/5 +encode(Name, AvpName, Values, #{strict_arities := T} = Opts, Mod) + when T /= encode -> + enc(Name, AvpName, ?ANY, Values, Opts, Mod); + encode(Name, AvpName, Values, Opts, Mod) -> enc(Name, AvpName, Mod:avp_arity(Name, AvpName), Values, Opts, Mod). %% enc/6 +enc(Name, AvpName, Arity, Values, #{strict_arities := T} = Opts, Mod) + when T /= encode, Arity /= ?ANY -> + enc(Name, AvpName, ?ANY, Values, Opts, Mod); + enc(_, AvpName, 1, undefined, _, _) -> ?THROW([mandatory_avp_missing, AvpName]); @@ -110,6 +121,9 @@ enc(Name, AvpName, 1, Value, Opts, Mod) -> enc(_, _, {0,_}, [], _, _) -> []; +enc(_, _, _, undefined, _, _) -> + []; + enc(_, AvpName, _, T, _, _) when not is_list(T) -> ?THROW([repeated_avp_as_non_list, AvpName, T]); @@ -120,6 +134,14 @@ enc(Name, AvpName, {Min, Max}, Values, Opts, Mod) -> %% enc/9 +enc(Name, AvpName, H, Min, N, '*', Vs, Opts, Mod) + when Min =< N -> + [enc1(Name, AvpName, H, V, Opts, Mod) || V <- Vs]; + +enc(Name, AvpName, H, _, _, _, Vs, #{strict_arities := T} = Opts, Mod) + when T /= encode -> + [enc1(Name, AvpName, H, V, Opts, Mod) || V <- Vs]; + enc(_, AvpName, _, Min, N, _, [], _, _) when N < Min -> ?THROW([repeated_avp_insufficient_arity, AvpName, Min, N]); @@ -127,10 +149,6 @@ enc(_, AvpName, _, Min, N, _, [], _, _) enc(_, _, _, _, _, _, [], _, _) -> []; -enc(Name, AvpName, H, Min, N, '*', Vs, Opts, Mod) - when Min =< N -> - [enc1(Name, AvpName, H, V, Opts, Mod) || V <- Vs]; - enc(_, AvpName, _, _, N, Max, _, _, _) when Max =< N -> ?THROW([repeated_avp_excessive_arity, AvpName, Max]); @@ -207,12 +225,12 @@ enc(AvpName, Value, Opts, Mod) -> decode_avps(Name, Recs, #{module := Mod, decode_format := Fmt} = Opts) -> {Avps, {Rec, AM, Failed}} = mapfoldl(fun(T,A) -> decode(Name, Opts, Mod, T, A) end, - {newrec(Mod, Name, Fmt), #{}, []}, + {newrec(Fmt, Mod, Name, Opts), #{}, []}, Recs), %% AM counts the number of top-level AVPs, which arities/5 then %% uses when adding 500[59] errors. Arities = Mod:avp_arity(Name), - {reformat(Name, Rec, Arities, Mod, Fmt), + {reformat(Name, Rec, Arities, Mod, Opts, Fmt), Avps, Failed ++ arities(Arities, Opts, Mod, AM, Avps)}. @@ -249,6 +267,12 @@ mapfoldl(_, Acc, [], List) -> %% AVP MUST be included and contain a copy of the first instance of %% the offending AVP that exceeded the maximum number of occurrences +%% arities/5 + +arities(_, #{strict_arities := T}, _, _, _) + when T /= decode -> + []; + arities(Arities, Opts, Mod, AM, Avps) -> [Count, Map | More] = lists:foldl(fun({N,T}, A) -> more(N, T, Opts, Mod, AM, A) end, @@ -258,7 +282,7 @@ arities(Arities, Opts, Mod, AM, Avps) -> %% more/6 -more(_Name, {0, '*'}, _Opts, _Mod, _AM, Acc) -> +more(_Name, ?ANY, _Opts, _Mod, _AM, Acc) -> Acc; more(Name, {Mn, Mx}, Opts, Mod, AM, Acc) -> @@ -331,16 +355,21 @@ empty_avp(Name, Opts, Mod) -> %% decode/5 -decode(Name, - Opts, - Mod, - #diameter_avp{code = Code, vendor_id = Vid} - = Avp, - {Rec, AM, Failed}) -> - T = Mod:avp_name(Code, Vid), - F = field(T), - A = Mod:avp_arity(Name, F), - decode(Name, Opts, Mod, T, A, Avp, {Rec, incr(field(F, A), AM), Failed}). +decode(Name, Opts, Mod, Avp, Acc) -> + #diameter_avp{code = Code, vendor_id = Vid} + = Avp, + N = Mod:avp_name(Code, Vid), + case Opts of + #{strict_arities := T} when T /= decode -> + decode(Name, Opts, Mod, N, ?ANY, Avp, Acc); + _ -> + {Rec, AM, Failed} = Acc, + F = field(N), + A = Mod:avp_arity(Name, F), + decode(Name, Opts, Mod, N, A, Avp, {Rec, + incr(field(F, A), AM), + Failed}) + end. %% field/1 @@ -565,6 +594,10 @@ set_failed(_, Opts) -> %% Don't know this AVP: see if it can be packed in an 'AVP' field %% undecoded. Note that the type field is 'undefined' in this case. +decode_AVP(Name, Avp, #{strict_arities := T} = Opts, Mod, Acc) + when T /= decode -> + decode_AVP(Name, ?ANY, Avp, Opts, Mod, Acc); + decode_AVP(Name, Avp, Opts, Mod, Acc) -> decode_AVP(Name, Mod:avp_arity(Name, 'AVP'), Avp, Opts, Mod, Acc). @@ -601,6 +634,10 @@ pack_avp(_, Arity, #diameter_avp{name = AvpName} = Avp, _Opts, Mod, Acc) -> %% pack_AVP/5 +pack_AVP(Name, Avp, #{strict_arities := T} = Opts, Mod, Acc) + when T /= decode -> + pack_AVP(Name, ?ANY, Avp, Opts, Mod, Acc); + pack_AVP(Name, Avp, Opts, Mod, Acc) -> pack_AVP(Name, Mod:avp_arity(Name, 'AVP'), Avp, Opts, Mod, Acc). @@ -652,10 +689,17 @@ pack_AVP(Name, Arity, Avp, Opts, Mod, Acc) -> %% unrecognized mandatory AVP's and (2) the RFC 3588 grammar failed to %% allow for Failed-AVP in an answer-message. +pack_AVP(_, #{strict_arities := T}, _, _) + when T /= decode -> + true; + +pack_AVP(_, _, 0, _) -> + false; + pack_AVP(Name, #{strict_mbit := Strict, failed_avp := Failed}, - Arity, + _, #diameter_avp{is_mandatory = M, name = AvpName}) -> @@ -665,12 +709,11 @@ pack_AVP(Name, %% possible, and failing because a mandatory AVP couldn't be %% packed into a dedicated field defeats that point. - 0 < Arity andalso (Failed == true - orelse Name == 'Failed-AVP' - orelse (Name == 'answer-message' - andalso AvpName == 'Failed-AVP') - orelse not M - orelse not Strict). + Failed == true + orelse Name == 'Failed-AVP' + orelse (Name == 'answer-message' andalso AvpName == 'Failed-AVP') + orelse not M + orelse not Strict. %% pack/5 @@ -789,15 +832,21 @@ empty(Name, #{module := Mod} = Opts) -> %% ------------------------------------------------------------------------------ -%% newrec/3 +%% newrec/4 -newrec(_, _, false = No) -> +newrec(false = No, _, _, _) -> No; -newrec(Mod, Name, record) -> +newrec(record, Mod, Name, #{strict_arities := T}) + when T /= decode -> + RecName = Mod:name2rec(Name), + Sz = Mod:'#info-'(RecName, size), + erlang:make_tuple(Sz, [], [{1, RecName}]); + +newrec(record, Mod, Name, _) -> newrec(Mod, Name); -newrec(_, _, _) -> +newrec(_, _, _, _) -> #{}. %% newrec/2 @@ -805,22 +854,24 @@ newrec(_, _, _) -> newrec(Mod, Name) -> Mod:'#new-'(Mod:name2rec(Name)). -%% reformat/4 +%% reformat/5 -reformat(_, Map, Arities, _Mod, list) -> +reformat(_, Map, Arities, _Mod, _Opts, list) -> [{F,V} || {F,_} <- Arities, #{F := V} <- [Map]]; -reformat(Name, Map, Arities, Mod, record_from_map) -> +reformat(Name, Map, Arities, Mod, Opts, record_from_map) -> + SA = maps:get(strict_arities, Opts, decode), RecName = Mod:name2rec(Name), - list_to_tuple([RecName | [maps:get(F, Map, def(A)) || {F,A} <- Arities]]); + list_to_tuple([RecName | [maps:get(F, Map, def(A, SA)) + || {F,A} <- Arities]]); -reformat(_, Rec, _, _, _) -> +reformat(_, Rec, _, _, _, _) -> Rec. -%% def/1 +%% def/2 -def(1) -> +def(1, decode) -> undefined; -def(_) -> +def(_, _) -> []. diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index abf948593f..36014df94e 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -130,6 +130,7 @@ %% diameter:call/4. codec :: #{decode_format := record, string_decode := boolean(), + strict_arities => diameter:strict_arities(), strict_mbit := boolean(), rfc := 3588 | 6733, ordered_encode := false}, diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl index 43be4d889a..c3c3c4b0e7 100644 --- a/lib/diameter/src/base/diameter_service.erl +++ b/lib/diameter/src/base/diameter_service.erl @@ -112,6 +112,7 @@ use_shared_peers := diameter:remotes(),%% use from restrict_connections := diameter:restriction(), incoming_maxlen := diameter:message_length(), + strict_arities => diameter:strict_arities(), strict_mbit := boolean(), decode_format := diameter:decode_format(), string_decode := boolean(), @@ -701,7 +702,7 @@ init_peers() -> %% TPid} service_options(Opts) -> - maps:from_list(Opts). + maps:from_list(lists:delete({strict_arities, true}, Opts)). mref(false = No) -> No; diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index e9c422d6ab..3463a93568 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -78,6 +78,7 @@ sequence :: diameter:sequence(), codec :: #{decode_format := diameter:decode_format(), string_decode := boolean(), + strict_arities => diameter:strict_arities(), strict_mbit := boolean(), incoming_maxlen := diameter:message_length()}}). %% Note that incoming_maxlen is currently handled in diameter_peer_fsm, @@ -105,6 +106,7 @@ make_recvdata([SvcName, PeerT, Apps, SvcOpts | _]) -> sequence = Mask, codec = maps:with([decode_format, string_decode, + strict_arities, strict_mbit, ordered_encode, incoming_maxlen], diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl index 60baf1e8a4..294325751e 100644 --- a/lib/diameter/src/base/diameter_watchdog.erl +++ b/lib/diameter/src/base/diameter_watchdog.erl @@ -74,6 +74,7 @@ okay := non_neg_integer()}, %% REOPEN -> OKAY codec :: #{decode_format := false, string_decode := false, + strict_arities => diameter:strict_arities(), strict_mbit := boolean(), failed_avp := false, rfc := 3588 | 6733, @@ -138,6 +139,7 @@ i({Ack, T, Pid, {Opts, Nodes = restrict_nodes(Restrict), CodecKeys = [decode_format, string_decode, + strict_arities, strict_mbit, incoming_maxlen, spawn_opt, @@ -157,9 +159,10 @@ i({Ack, T, Pid, {Opts, suspect => 1, okay => 3}, Opts)), - codec = maps:with(CodecKeys, SvcOpts#{decode_format := false, - string_decode := false, - ordered_encode => false})}. + codec = maps:with(CodecKeys -- [strict_arities], + SvcOpts#{decode_format := false, + string_decode := false, + ordered_encode => false})}. wait(Ref, Pid) -> receive -- cgit v1.2.3 From 9adab9b89d49e7c8706e34b87be7d5123ce181cd Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 13 Jul 2017 13:18:19 +0200 Subject: Be forgiving of non-list values at encode --- lib/diameter/src/base/diameter_gen.erl | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl index 243bc7965a..3d0612c294 100644 --- a/lib/diameter/src/base/diameter_gen.erl +++ b/lib/diameter/src/base/diameter_gen.erl @@ -124,9 +124,12 @@ enc(_, _, {0,_}, [], _, _) -> enc(_, _, _, undefined, _, _) -> []; -enc(_, AvpName, _, T, _, _) - when not is_list(T) -> - ?THROW([repeated_avp_as_non_list, AvpName, T]); +%% Be forgiving when a list of values is expected. If the value itself +%% is a list then the user has to wrap it to avoid each member from +%% being interpreted as an individual AVP value. +enc(Name, AvpName, Arity, V, Opts, Mod) + when not is_list(V) -> + enc(Name, AvpName, Arity, [V], Opts, Mod); enc(Name, AvpName, {Min, Max}, Values, Opts, Mod) -> H = avp_header(AvpName, Mod), -- cgit v1.2.3 From b3d9e0c09ff9d8f963b6084a84ab120cd423a9ae Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 25 Jul 2017 01:27:58 +0200 Subject: Redo message decode as a single pass Decode has previously been two passes: first chunk the message into a reversed list of toplevel diameter_avp records, then fold through the reversed list to build the full result. Various workarounds have made it a bit more convoluted than it should be however. Rework it completely, turning the previous 2-pass tail-recursive implementation into a 1-pass body recursive one. The relay decode still exists in diameter_codec, as a stripped down version of the full decode in diameter_gen. --- lib/diameter/src/base/diameter_codec.erl | 185 +++----- lib/diameter/src/base/diameter_gen.erl | 648 +++++++++++++---------------- lib/diameter/src/base/diameter_traffic.erl | 12 +- 3 files changed, 358 insertions(+), 487 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index 9b21bf4141..c179c4b362 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -110,7 +110,7 @@ encode(Mod, Opts, Msg) -> enc(_, Opts, #diameter_packet{msg = [#diameter_header{} = Hdr | As]} = Pkt) -> - try encode_avps(reorder(As), Opts) of + try encode_avps(As, Opts) of Avps -> Bin = list_to_binary(Avps), Len = 20 + size(Bin), @@ -206,51 +206,12 @@ values(Avps) -> %% Message as a list of #diameter_avp{} ... encode_avps(_, _, [#diameter_avp{} | _] = Avps, Opts) -> - encode_avps(reorder(Avps), Opts); + encode_avps(Avps, Opts); %% ... or as a tuple list or record. encode_avps(Mod, MsgName, Values, Opts) -> Mod:encode_avps(MsgName, Values, Opts). -%% reorder/1 -%% -%% Reorder AVPs for the relay case using the index field of -%% diameter_avp records. Decode populates this field in collect_avps -%% and presents AVPs in reverse order. A relay then sends the reversed -%% list with a Route-Record AVP prepended. The goal here is just to do -%% lists:reverse/1 in Grouped AVPs and the outer list, but only in the -%% case there are indexed AVPs at all, so as not to reverse lists that -%% have been explicilty sent (unindexed, in the desired order) as a -%% diameter_avp list. The effect is the same as lists:keysort/2, but -%% only on the cases we expect, not a general sort. - -reorder(Avps) -> - case reorder(Avps, []) of - false -> - Avps; - Sorted -> - Sorted - end. - -%% reorder/3 - -%% In case someone has reversed the list already. (Not likely.) -reorder([#diameter_avp{index = 0} | _] = Avps, Acc) -> - Avps ++ Acc; - -%% Assume indexed AVPs are in reverse order. -reorder([#diameter_avp{index = N} = A | Avps], Acc) - when is_integer(N) -> - lists:reverse(Avps, [A | Acc]); - -%% An unindexed AVP. -reorder([H | T], Acc) -> - reorder(T, [H | Acc]); - -%% No indexed members. -reorder([], _) -> - false. - %% encode_avps/2 encode_avps(Avps, Opts) -> @@ -327,13 +288,7 @@ decode(Mod, AppMod, Opts, Pkt) -> %% Relay application: just extract the avp's without any decoding of %% their data since we don't know the application in question. decode(?APP_ID_RELAY, _, _, _, #diameter_packet{} = Pkt) -> - case collect_avps(Pkt) of - {E, As} -> - Pkt#diameter_packet{avps = As, - errors = [E]}; - As -> - Pkt#diameter_packet{avps = As} - end; + collect_avps(Pkt); %% Otherwise decode using the dictionary. decode(_, Mod, AppMod, Opts, #diameter_packet{header = Hdr} = Pkt) -> @@ -342,35 +297,31 @@ decode(_, Mod, AppMod, Opts, #diameter_packet{header = Hdr} = Pkt) -> is_error = IsError} = Hdr, - MsgName = if IsError andalso not IsRequest -> + MsgName = if IsError, not IsRequest -> 'answer-message'; true -> Mod:msg_name(CmdCode, IsRequest) end, - decode_avps(MsgName, Mod, AppMod, Opts, Pkt, collect_avps(Pkt)); + decode_avps(MsgName, Mod, AppMod, Opts, Pkt); decode(Id, Mod, AppMod, Opts, Bin) when is_binary(Bin) -> decode(Id, Mod, AppMod, Opts, #diameter_packet{header = decode_header(Bin), bin = Bin}). -%% decode_avps/6 +%% decode_avps/5 -decode_avps(MsgName, Mod, AppMod, Opts, Pkt, {E, Avps}) -> - ?LOG(invalid_avp_length, Pkt#diameter_packet.header), - #diameter_packet{errors = Failed} - = P - = decode_avps(MsgName, Mod, AppMod, Opts, Pkt, Avps), - P#diameter_packet{errors = [E | Failed]}; - -decode_avps('', _, _, _, Pkt, Avps) -> %% unknown message ... - ?LOG(unknown_message, Pkt#diameter_packet.header), - Pkt#diameter_packet{avps = lists:reverse(Avps), - errors = [3001]}; %% DIAMETER_COMMAND_UNSUPPORTED +decode_avps('', _, _, _, #diameter_packet{header = H, %% unknown message + bin = Bin} + = Pkt) -> + ?LOG(unknown_message, H), + Pkt#diameter_packet{avps = collect_avps(Bin), + errors = [3001]}; %% DIAMETER_COMMAND_UNSUPPORTED %% msg = undefined identifies this case. -decode_avps(MsgName, Mod, AppMod, Opts, Pkt, Avps) -> %% ... or not +decode_avps(MsgName, Mod, AppMod, Opts, #diameter_packet{bin = Bin} = Pkt) -> + <<_:20/binary, Avps/binary>> = Bin, {Rec, As, Errors} = Mod:decode_avps(MsgName, Avps, Opts#{dictionary => AppMod, @@ -380,6 +331,8 @@ decode_avps(MsgName, Mod, AppMod, Opts, Pkt, Avps) -> %% ... or not errors = Errors, avps = As}. +%% reformat/3 + reformat(MsgName, Avps, #{decode_format := T}) when T == map; T == list -> @@ -524,24 +477,21 @@ msg_id(<<_:32, Rbit:1, _:7, CmdCode:24, ApplId:32, _/binary>>) -> %%% # collect_avps/1 %%% --------------------------------------------------------------------------- -%% Note that the returned list of AVP's is reversed relative to their -%% order in the binary. Note also that grouped avp's aren't unraveled, -%% only those at the top level. +%% This is only used for the relay decode. Note that grouped avp's +%% aren't unraveled, only those at the top level. --spec collect_avps(#diameter_packet{} | binary()) - -> [Avp] - | {Error, [Avp]} - when Avp :: #diameter_avp{}, - Error :: {5014, #diameter_avp{}}. +-spec collect_avps(#diameter_packet{}) + -> #diameter_packet{}; + (binary()) + -> [#diameter_avp{}]. -collect_avps(#diameter_packet{bin = <<_:20/binary, Avps/binary>>}) -> - collect_avps(Avps, 0, []); +collect_avps(#diameter_packet{bin = Bin} = Pkt) -> + Pkt#diameter_packet{avps = collect_avps(Bin)}; -collect_avps(Bin) - when is_binary(Bin) -> - collect_avps(Bin, 0, []). +collect_avps(<<_:20/binary, Avps/binary>>) -> + collect(Avps, 0). -%% collect_avps/3 +%% collect/2 %% 0 1 2 3 %% 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 @@ -555,65 +505,44 @@ collect_avps(Bin) %% | Data ... %% +-+-+-+-+-+-+-+-+ -collect_avps(<>, - N, - Acc) -> - collect_avps(Code, - if 1 == V -> I; 0 == V -> undefined end, - 1 == M, - 1 == P, - Len - 8 - V*4, %% Might be negative, which ensures - ?PAD(Len), %% failure of the Data match below. - Rest, - N, - Acc); - -collect_avps(<<>>, _, Acc) -> - Acc; +collect(<>, N)-> + Vid = if 1 == V -> I; 0 == V -> undefined end, + DataLen = Len - 8 - V*4, %% Might be negative, which ensures + Pad = ?PAD(Len), %% failure of the match below. + MB = 1 == M, + PB = 1 == P, -%% Header is truncated. pack_avp/1 will pad this at encode if sent in -%% a Failed-AVP. -collect_avps(Bin, _, Acc) -> - {{5014, #diameter_avp{data = Bin}}, Acc}. - -%% collect_avps/9 - -%% Duplicate the diameter_avp creation in each branch below to avoid -%% modifying the record, which profiling has shown to be a relatively -%% costly part of building the list. - -collect_avps(Code, VendorId, M, P, Len, Pad, Rest, N, Acc) -> case Rest of - <> -> + <> -> Avp = #diameter_avp{code = Code, - vendor_id = VendorId, - is_mandatory = M, - need_encryption = P, + vendor_id = Vid, + is_mandatory = MB, + need_encryption = PB, data = Data, index = N}, - collect_avps(T, N+1, [Avp | Acc]); + [Avp | collect(T, N+1)]; _ -> %% Length in header points past the end of the message, or - %% doesn't span the header. As stated in the 6733 text - %% above, it's sufficient to return a zero-filled minimal - %% payload if this is a request. Do this (in cases that we - %% know the type) by inducing a decode failure and letting - %% the dictionary's decode (in diameter_gen) deal with it. - %% - %% Note that the extra bit can only occur in the trailing - %% AVP of a message or Grouped AVP, since a faulty AVP - %% Length is otherwise indistinguishable from a correct - %% one here, as we don't know the types of the AVPs being - %% extracted. - Avp = #diameter_avp{code = Code, - vendor_id = VendorId, - is_mandatory = M, - need_encryption = P, - data = {5014, Rest}, - index = N}, - [Avp | Acc] - end. + %% doesn't span the header. Note that an length error can + %% only occur in the trailing AVP of a message or Grouped + %% AVP, since a faulty AVP Length is otherwise + %% indistinguishable from a correct one here, as we don't + %% know the types of the AVPs being extracted. + [#diameter_avp{code = Code, + vendor_id = Vid, + is_mandatory = MB, + need_encryption = PB, + data = {5014, Rest}, + index = N}] + end; +collect(<<>>, _) -> + []; + +%% Header is truncated. pack_avp/1 will pad this at encode if sent in +%% a Failed-AVP. +collect(Bin, _) -> + [#diameter_avp{data = {5014, Bin}}]. %% 3588: %% diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl index 3d0612c294..3688ff7b8f 100644 --- a/lib/diameter/src/base/diameter_gen.erl +++ b/lib/diameter/src/base/diameter_gen.erl @@ -91,7 +91,7 @@ encode(Name, Vals, Opts, Mod) encode(Name, Map, Opts, Mod) when is_map(Map) -> [enc(Name, F, A, V, Opts, Mod) || {F,A} <- Mod:avp_arity(Name), - V <- [maps:get(F, Map, undefined)]]; + V <- [mget(F, Map, undefined)]]; encode(Name, Rec, Opts, Mod) -> [encode(Name, F, V, Opts, Mod) || {F,V} <- Mod:'#get-'(Rec)]. @@ -221,115 +221,168 @@ enc(AvpName, Value, Opts, Mod) -> %% # decode_avps/3 %% --------------------------------------------------------------------------- --spec decode_avps(parent_name(), [#diameter_avp{}], map()) +-spec decode_avps(parent_name(), binary(), map()) -> {parent_record(), [avp()], Failed} when Failed :: [{5000..5999, #diameter_avp{}}]. -decode_avps(Name, Recs, #{module := Mod, decode_format := Fmt} = Opts) -> - {Avps, {Rec, AM, Failed}} - = mapfoldl(fun(T,A) -> decode(Name, Opts, Mod, T, A) end, - {newrec(Fmt, Mod, Name, Opts), #{}, []}, - Recs), - %% AM counts the number of top-level AVPs, which arities/5 then - %% uses when adding 500[59] errors. - Arities = Mod:avp_arity(Name), - {reformat(Name, Rec, Arities, Mod, Opts, Fmt), +decode_avps(Name, Bin, #{module := Mod, decode_format := Fmt} = Opts) -> + Strict = mget(strict_arities, Opts, decode), + [AM, Avps, Failed | Rec] + = decode(Bin, Name, Mod, Fmt, Strict, Opts, 0, #{}), + %% AM counts the number of top-level AVPs, which missing/5 then + %% uses when appending 5005 errors. + {reformat(Name, Rec, Strict, Mod, Fmt), Avps, - Failed ++ arities(Arities, Opts, Mod, AM, Avps)}. + Failed ++ missing(Name, Strict, Mod, Opts, AM)}. %% Append arity errors so that errors are reported in the order %% encountered. Failed-AVP should typically contain the first %% error encountered. -%% mapfoldl/3 -%% -%% Like lists:mapfoldl/3, but don't reverse the list. +%% decode/8 + +decode(<>, + Name, + Mod, + Fmt, + Strict, + Opts0, + Idx, + AM0) -> + Vid = if 1 == V -> I; true -> undefined end, + MB = 1 == M, + PB = 1 == P, + NameT = Mod:avp_name(Code, Vid), %% {AvpName, Type} | 'AVP' + DataLen = Len - 8 - 4*V, %% possibly negative, causing case match to fail + Pad = (4 - (Len rem 4)) rem 4, + + case Rest of + <> -> + Opts = setopts(NameT, Name, MB, Opts0), + %% Not AvpName or else a failed Failed-AVP + %% decode is packed into 'AVP'. + + Avp = #diameter_avp{code = Code, + vendor_id = Vid, + is_mandatory = MB, + need_encryption = PB, + data = Data, + name = name(NameT), + type = type(NameT), + index = Idx}, + + Dec = decode(Data, Name, NameT, Mod, Opts, Avp), %% decode + + AvpName = field(NameT), + Arity = avp_arity(Name, AvpName, Mod, Opts, Avp), + AM = incr(AvpName, Arity, Strict, AM0), %% count + + Acc = decode(T, Name, Mod, Fmt, Strict, Opts, Idx+1, AM),%% recurse + acc(Acc, Dec, Name, AvpName, Arity, Strict, Mod, Opts, AM0); + _ -> + Avp = #diameter_avp{code = Code, + vendor_id = Vid, + is_mandatory = MB, + need_encryption = PB, + data = Rest, + name = name(NameT), + type = type(NameT), + index = Idx}, + [AM0, [Avp], [{5014, Avp}] | newrec(Fmt, Mod, Name, Strict)] + end; + +decode(<<>>, Name, Mod, Fmt, Strict, _, _, AM) -> + [AM, [], [] | newrec(Fmt, Mod, Name, Strict)]; + +decode(Bin, Name, Mod, Fmt, Strict, _, Idx, AM) -> + Avp = #diameter_avp{data = Bin, + index = Idx}, + [AM, [Avp], [{5014, Avp}] | newrec(Fmt, Mod, Name, Strict)]. -mapfoldl(F, Acc, List) -> - mapfoldl(F, Acc, List, []). +%% Data is a truncated header if command_code = undefined, otherwise +%% payload bytes. The former is padded to the length of a header if +%% the AVP reaches an outgoing encode. +%% +%% RFC 6733 says that an AVP returned with 5014 can contain a minimal +%% payload for the AVP's type, but don't always know the type. -mapfoldl(F, Acc0, [T|Rest], List) -> - {B, Acc} = F(T, Acc0), - mapfoldl(F, Acc, Rest, [B|List]); -mapfoldl(_, Acc, [], List) -> - {List, Acc}. +setopts('AVP', _, _, Opts) -> + Opts; -%% 3588/6733: -%% -%% DIAMETER_MISSING_AVP 5005 -%% The request did not contain an AVP that is required by the Command -%% Code definition. If this value is sent in the Result-Code AVP, a -%% Failed-AVP AVP SHOULD be included in the message. The Failed-AVP -%% AVP MUST contain an example of the missing AVP complete with the -%% Vendor-Id if applicable. The value field of the missing AVP -%% should be of correct minimum length and contain zeros. -%% -%% DIAMETER_AVP_OCCURS_TOO_MANY_TIMES 5009 -%% A message was received that included an AVP that appeared more -%% often than permitted in the message definition. The Failed-AVP -%% AVP MUST be included and contain a copy of the first instance of -%% the offending AVP that exceeded the maximum number of occurrences +setopts({_, Type}, Name, M, Opts) -> + set_failed(Name, set_strict(Type, M, Opts)). -%% arities/5 +%% incr/4 -arities(_, #{strict_arities := T}, _, _, _) - when T /= decode -> - []; +incr(F, A, SA, AM) + when F == 'AVP'; + A == ?ANY; + A == 0; + SA /= decode -> + AM; -arities(Arities, Opts, Mod, AM, Avps) -> - [Count, Map | More] - = lists:foldl(fun({N,T}, A) -> more(N, T, Opts, Mod, AM, A) end, - [0, #{}], - Arities), - less(Count, Map, Avps) ++ lists:reverse(More). +incr(AvpName, _, _, AM) -> + maps:update_with(AvpName, fun incr/1, 1, AM). -%% more/6 +%% incr/1 -more(_Name, ?ANY, _Opts, _Mod, _AM, Acc) -> - Acc; +incr(N) -> + N + 1. -more(Name, {Mn, Mx}, Opts, Mod, AM, Acc) -> - more(Name, Mn, Mx, Opts, Mod, AM, Acc); +%% mget/3 +%% +%% Measurably faster than maps:get/3. -more(Name, 1, Opts, Mod, AM, Acc) -> - more(Name, 1, 1, Opts, Mod, AM, Acc). +mget(Key, Map, Def) -> + case Map of + #{Key := V} -> + V; + _ -> + Def + end. -%% more/7 +%% name/1 -more(Name, Mn, Mx, Opts, Mod, AM, Acc) -> - macc(Name, Mn, maps:get(Name, AM, 0), Mx, Opts, Mod, Acc). +name({Name, _}) -> + Name; +name(_) -> + undefined. -%% macc/7 +%% type/1 -macc(Name, Mn, N, _, Opts, Mod, [M, Map | T]) - when N < Mn -> - [M, Map, {5005, empty_avp(Name, Opts, Mod)} | T]; +type({_, Type}) -> + Type; +type(_) -> + undefined. -macc(Name, _, N, Mx, _Opts, _Mod, [M, Map | T]) - when Mx < N -> - K = N - Mx, - [M + K, maps:put(Name, K, Map) | T]; +%% missing/5 -macc(_Name, _, _, _, _Opts, _Mod, Acc) -> - Acc. +missing(Name, decode, Mod, Opts, AM) -> + [{5005, empty_avp(N, Opts, Mod)} || {N,A} <- Mod:avp_arity(Name), + N /= 'AVP', + Mn <- [min_arity(A)], + 0 < Mn, + mget(N, AM, 0) < Mn]; -%% less/3 +missing(_, _, _, _, _) -> + []. -less(0, _, _) -> - []; +%% 3588/6733: +%% +%% DIAMETER_MISSING_AVP 5005 +%% The request did not contain an AVP that is required by the Command +%% Code definition. If this value is sent in the Result-Code AVP, a +%% Failed-AVP AVP SHOULD be included in the message. The Failed-AVP +%% AVP MUST contain an example of the missing AVP complete with the +%% Vendor-Id if applicable. The value field of the missing AVP +%% should be of correct minimum length and contain zeros. -less(N, Map, [#diameter_avp{name = undefined} | Avps]) -> - less(N, Map, Avps); +%% min_arity/1 -less(N, Map, [#diameter_avp{name = Name} = Avp | Avps]) -> - case Map of - #{Name := 0} -> - [{5009, Avp} | less(N-1, Map, Avps)]; - #{Name := M} -> - less(N, maps:put(Name, M-1, Map), Avps); - _ -> - less(N, Map, Avps) - end. +min_arity(1) -> + 1; +min_arity({Mn,_}) -> + Mn. %% empty_avp/3 @@ -356,55 +409,18 @@ empty_avp(Name, Opts, Mod) -> %% specific errors that can be described by this AVP are described in %% the following section. -%% decode/5 - -decode(Name, Opts, Mod, Avp, Acc) -> - #diameter_avp{code = Code, vendor_id = Vid} - = Avp, - N = Mod:avp_name(Code, Vid), - case Opts of - #{strict_arities := T} when T /= decode -> - decode(Name, Opts, Mod, N, ?ANY, Avp, Acc); - _ -> - {Rec, AM, Failed} = Acc, - F = field(N), - A = Mod:avp_arity(Name, F), - decode(Name, Opts, Mod, N, A, Avp, {Rec, - incr(field(F, A), AM), - Failed}) - end. - %% field/1 field({AvpName, _}) -> AvpName; - field(_) -> 'AVP'. -%% field/2 - -field(_, 0) -> - 'AVP'; - -field(F, _) -> - F. - -%% incr/2 - -incr(Key, Map) -> - maps:update_with(Key, fun incr/1, 1, Map). - -%% incr/1 - -incr(N) -> - N + 1. - -%% decode/7 +%% decode/6 %% AVP not in dictionary. -decode(Name, Opts, Mod, 'AVP', Arity, Avp, Acc) -> - decode_AVP(Name, Arity, Avp, Opts, Mod, Acc); +decode(_Data, _Name, 'AVP', _Mod, _Opts, Avp) -> + Avp; %% 6733, 4.4: %% @@ -453,20 +469,9 @@ decode(Name, Opts, Mod, 'AVP', Arity, Avp, Acc) -> %% defined the RFC's "unrecognized", which is slightly stronger than %% "not defined".) -decode(Name, Opts0, Mod, {AvpName, Type}, Arity, Avp, Acc) -> - #diameter_avp{data = Data, is_mandatory = M} - = Avp, - - %% Whether or not to ignore an M-bit on an encapsulated AVP, or on - %% all AVPs with the service_opt() strict_mbit. - Opts1 = set_strict(Type, M, Opts0), - - %% Whether or not we're decoding within Failed-AVP and should - %% ignore decode errors. +decode(Data, Name, {AvpName, Type}, Mod, Opts, Avp) -> #{dictionary := AppMod, failed_avp := Failed} - = Opts - = set_failed(Name, Opts1), %% Not AvpName or else a failed Failed-AVP - %% decode is packed into 'AVP'. + = Opts, %% Reset the dictionary for best-effort decode of Failed-AVP. DecMod = if Failed -> AppMod; @@ -479,103 +484,55 @@ decode(Name, Opts0, Mod, {AvpName, Type}, Arity, Avp, Acc) -> try avp_decode(Data, AvpName, Opts, DecMod, Mod) of {Rec, As} when Type == 'Grouped' -> - A = Avp#diameter_avp{name = AvpName, - value = Rec, - type = Type}, - {[A|As], pack_avp(Name, Arity, A, Opts, Mod, Acc)}; - + A = Avp#diameter_avp{value = Rec}, + [A | As]; V when Type /= 'Grouped' -> - A = Avp#diameter_avp{name = AvpName, - value = V, - type = Type}, - {A, pack_avp(Name, Arity, A, Opts, Mod, Acc)} + Avp#diameter_avp{value = V} catch - throw: {?MODULE, {grouped, Error, ComponentAvps}} -> - decode_error(Name, - Error, - ComponentAvps, - Opts, - Mod, - Avp#diameter_avp{name = AvpName, - data = trim(Avp#diameter_avp.data), - type = Type}, - Acc); - + throw: {?MODULE, T} -> + decode_error(Failed, T, Avp); error: Reason -> - decode_error(Name, - Reason, - Opts, - Mod, - Avp#diameter_avp{name = AvpName, - data = trim(Avp#diameter_avp.data), - type = Type}, - Acc) + decode_error(Failed, Reason, Name, Mod, Opts, Avp) end. -%% avp_decode/5 - -avp_decode(Data, AvpName, Opts, Mod, Mod) -> - Mod:avp(decode, Data, AvpName, Opts); - -avp_decode(Data, AvpName, Opts, Mod, _) -> - Mod:avp(decode, Data, AvpName, Opts, Mod). - -%% trim/1 +%% decode_error/3 %% -%% Remove any extra bit that was added in diameter_codec to induce a -%% 5014 error. - -trim(#diameter_avp{data = Data} = Avp) -> - Avp#diameter_avp{data = trim(Data)}; - -trim({5014, Bin}) -> - Bin; - -trim(Avps) - when is_list(Avps) -> - lists:map(fun trim/1, Avps); - -trim(Avp) -> - Avp. - -%% decode_error/7 - -decode_error(Name, [_|Rec], _, #{failed_avp := true} = Opts, Mod, Avp, Acc) -> - decode_AVP(Name, Avp#diameter_avp{value = Rec}, Opts, Mod, Acc); - -decode_error(Name, _, _, #{failed_avp := true} = Opts, Mod, Avp, Acc) -> - decode_AVP(Name, Avp, Opts, Mod, Acc); +%% Error when decoding a grouped AVP. -decode_error(_, [Error | _], ComponentAvps, _, _, Avp, Acc) -> - decode_error(Error, Avp, Acc, ComponentAvps); +decode_error(true, {Rec, _, _}, Avp) -> + Avp#diameter_avp{value = Rec}; -decode_error(_, Error, ComponentAvps, _, _, Avp, Acc) -> - decode_error(Error, Avp, Acc, ComponentAvps). +decode_error(false, {_, ComponentAvps, [{RC,A} | _]}, Avp) -> + {RC, [Avp | ComponentAvps], Avp#diameter_avp{data = [A]}}. %% decode_error/6 +%% +%% Error when decoding a non-grouped AVP. -decode_error(Name, _Reason, #{failed_avp := true} = Opts, Mod, Avp, Acc) -> - decode_AVP(Name, Avp, Opts, Mod, Acc); +decode_error(true, _, _, _, _, Avp) -> + Avp; -decode_error(Name, Reason, Opts, Mod, Avp, {Rec, AM, Failed}) -> +decode_error(false, Reason, Name, Mod, Opts, Avp) -> Stack = diameter_lib:get_stacktrace(), - AvpName = Avp#diameter_avp.name, diameter_lib:log(decode_error, ?MODULE, ?LINE, - {Reason, Name, AvpName, Mod, Stack}), - {Avp, {Rec, AM, [rc(Reason, Avp, Opts, Mod) | Failed]}}. + {Reason, Name, Avp#diameter_avp.name, Mod, Stack}), + rc(Reason, Avp, Opts, Mod). -%% decode_error/4 +%% avp_decode/5 + +avp_decode(Data, AvpName, Opts, Mod, Mod) -> + Mod:avp(decode, Data, AvpName, Opts); -decode_error({RC, ErrorData}, Avp, {Rec, AM, Failed}, ComponentAvps) -> - E = Avp#diameter_avp{data = [ErrorData]}, - {[Avp | trim(ComponentAvps)], {Rec, AM, [{RC, E} | Failed]}}. +avp_decode(Data, AvpName, Opts, Mod, _) -> + Mod:avp(decode, Data, AvpName, Opts, Mod). %% set_strict/3 - +%% %% Set false as soon as we see a Grouped AVP that doesn't set the %% M-bit, to ignore the M-bit on an encapsulated AVP. + set_strict('Grouped', false = M, #{strict_mbit := true} = Opts) -> Opts#{strict_mbit := M}; set_strict(_, _, Opts) -> @@ -592,86 +549,82 @@ set_failed('Failed-AVP', #{failed_avp := false} = Opts) -> set_failed(_, Opts) -> Opts. -%% decode_AVP/5 -%% -%% Don't know this AVP: see if it can be packed in an 'AVP' field -%% undecoded. Note that the type field is 'undefined' in this case. - -decode_AVP(Name, Avp, #{strict_arities := T} = Opts, Mod, Acc) - when T /= decode -> - decode_AVP(Name, ?ANY, Avp, Opts, Mod, Acc); - -decode_AVP(Name, Avp, Opts, Mod, Acc) -> - decode_AVP(Name, Mod:avp_arity(Name, 'AVP'), Avp, Opts, Mod, Acc). - -%% decode_AVP/6 - -decode_AVP(Name, Arity, Avp, Opts, Mod, Acc) -> - {trim(Avp), pack_AVP(Name, Arity, Avp, Opts, Mod, Acc)}. - -%% rc/2 - -%% diameter_types will raise an error of this form to communicate -%% DIAMETER_INVALID_AVP_LENGTH (5014). A module specified to a -%% @custom_types tag in a dictionary file can also raise an error of -%% this form. -rc({'DIAMETER', 5014 = RC, _}, #diameter_avp{name = AvpName} = A, Opts, Mod) -> - {RC, A#diameter_avp{data = Mod:empty_value(AvpName, Opts)}}; +%% acc/9 + +acc([AM | Acc], As, Name, AvpName, Arity, Strict, Mod, Opts, AM0) -> + [AM | acc1(Acc, As, Name, AvpName, Arity, Strict, Mod, Opts, AM0)]. + +%% acc1/9 + +%% Faulty AVP, not grouped. +acc1(Acc, {_RC, Avp} = E, _, _, _, _, _, _, _) -> + [Avps, Failed | Rec] = Acc, + [[Avp | Avps], [E | Failed] | Rec]; + +%% Faulty component in grouped AVP. +acc1(Acc, {RC, As, Avp}, _, _, _, _, _, _, _) -> + [Avps, Failed | Rec] = Acc, + [[As | Avps], [{RC, Avp} | Failed] | Rec]; + +%% Grouped AVP ... +acc1([Avps | Acc], [Avp|_] = As, Name, AvpName, Arity, Strict, Mod, Opts, AM)-> + [[As|Avps] | acc2(Acc, Avp, Name, AvpName, Arity, Strict, Mod, Opts, AM)]; + +%% ... or not. +acc1([Avps | Acc], Avp, Name, AvpName, Arity, Strict, Mod, Opts, AM) -> + [[Avp|Avps] | acc2(Acc, Avp, Name, AvpName, Arity, Strict, Mod, Opts, AM)]. + +%% acc2/9 + +%% No errors, but nowhere to pack. +acc2(Acc, Avp, _, 'AVP', 0, _, _, _, _) -> + [Failed | Rec] = Acc, + [[{rc(Avp), Avp} | Failed] | Rec]; + +%% No AVP of this name: try to pack as 'AVP'. +acc2(Acc, Avp, Name, _, 0, Strict, Mod, Opts, AM) -> + Arity = pack_arity(Name, Opts, Mod, Avp), + acc2(Acc, Avp, Name, 'AVP', Arity, Strict, Mod, Opts, AM); + +%% Relaxed arities. +acc2(Acc, Avp, _, AvpName, Arity, Strict, Mod, _, _) + when Strict /= decode -> + pack(Arity, AvpName, Avp, Mod, Acc); + +%% No maximum arity. +acc2(Acc, Avp, _, AvpName, {_,'*'} = Arity, _, Mod, _, _) -> + pack(Arity, AvpName, Avp, Mod, Acc); + +%% Or check. +acc2(Acc, Avp, _, AvpName, Arity, _, Mod, _, AM) -> + Count = maps:get(AvpName, AM, 0), + Mx = max_arity(Arity), + if Mx =< Count -> + [Failed | Rec] = Acc, + [[{5009, Avp} | Failed] | Rec]; + true -> + pack(Arity, AvpName, Avp, Mod, Acc) + end. -%% 3588: +%% 3588/6733: %% -%% DIAMETER_INVALID_AVP_VALUE 5004 -%% The request contained an AVP with an invalid value in its data -%% portion. A Diameter message indicating this error MUST include -%% the offending AVPs within a Failed-AVP AVP. -rc(_, Avp, _, _) -> - {5004, Avp}. - -%% pack_avp/6 - -pack_avp(Name, 0, Avp, Opts, Mod, Acc) -> - pack_AVP(Name, Avp, Opts, Mod, Acc); - -pack_avp(_, Arity, #diameter_avp{name = AvpName} = Avp, _Opts, Mod, Acc) -> - pack(Arity, AvpName, Avp, Mod, Acc). - -%% pack_AVP/5 +%% DIAMETER_AVP_OCCURS_TOO_MANY_TIMES 5009 +%% A message was received that included an AVP that appeared more +%% often than permitted in the message definition. The Failed-AVP +%% AVP MUST be included and contain a copy of the first instance of +%% the offending AVP that exceeded the maximum number of occurrences -pack_AVP(Name, Avp, #{strict_arities := T} = Opts, Mod, Acc) - when T /= decode -> - pack_AVP(Name, ?ANY, Avp, Opts, Mod, Acc); +%% max_arity/1 -pack_AVP(Name, Avp, Opts, Mod, Acc) -> - pack_AVP(Name, Mod:avp_arity(Name, 'AVP'), Avp, Opts, Mod, Acc). +max_arity(1) -> + 1; +max_arity({_,Mx}) -> + Mx. -%% pack_AVP/6 +%% rc/1 -%% Length failure was induced because of a header/payload length -%% mismatch. The AVP Length is reset to match the received data if -%% this AVP is encoded in an answer message, since the length is -%% computed. -%% -%% Data is a truncated header if command_code = undefined, otherwise -%% payload bytes. The former is padded to the length of a header if -%% the AVP reaches an outgoing encode in diameter_codec. -%% -%% RFC 6733 says that an AVP returned with 5014 can contain a minimal -%% payload for the AVP's type, but in this case we don't know the -%% type. - -pack_AVP(_, _, #diameter_avp{data = {5014 = RC, Data}} = Avp, _, _, Acc) -> - {Rec, AM, Failed} = Acc, - {Rec, AM, [{RC, Avp#diameter_avp{data = Data}} | Failed]}; - -pack_AVP(Name, Arity, Avp, Opts, Mod, Acc) -> - case pack_AVP(Name, Opts, Arity, Avp) of - false -> - M = Avp#diameter_avp.is_mandatory, - {Rec, AM, Failed} = Acc, - {Rec, AM, [{if M -> 5001; true -> 5008 end, Avp} | Failed]}; - true -> - pack(Arity, 'AVP', Avp, Mod, Acc) - end. +rc(#diameter_avp{is_mandatory = M}) -> + if M -> 5001; true -> 5008 end. %% 3588: %% @@ -686,42 +639,59 @@ pack_AVP(Name, Arity, Avp, Opts, Mod, Acc) -> %% Failed-AVP AVP MUST be included and contain a copy of the %% offending AVP. -%% pack_AVP/4 +%% pack_arity/4 %% Give Failed-AVP special treatment since (1) it'll contain any %% unrecognized mandatory AVP's and (2) the RFC 3588 grammar failed to %% allow for Failed-AVP in an answer-message. -pack_AVP(_, #{strict_arities := T}, _, _) - when T /= decode -> - true; +pack_arity(Name, _, Mod, #diameter_avp{is_mandatory = M, name = AvpName}) + when Name == 'Failed-AVP'; + Name == 'answer-message', AvpName == 'Failed-AVP'; + not M -> + Mod:avp_arity(Name, 'AVP'); +%% Not testing just Name /= 'Failed-AVP' means we're changing the +%% packing of AVPs nested within Failed-AVP, but the point of +%% ignoring errors within Failed-AVP is to decode as much as +%% possible, and failing because a mandatory AVP couldn't be +%% packed into a dedicated field defeats that point. -pack_AVP(_, _, 0, _) -> - false; +pack_arity(Name, #{strict_mbit := Strict, failed_avp := Failed}, Mod, _) + when not Strict; + Failed -> + Mod:avp_arity(Name, 'AVP'); + +pack_arity(_, _, _, _) -> + 0. -pack_AVP(Name, - #{strict_mbit := Strict, - failed_avp := Failed}, - _, - #diameter_avp{is_mandatory = M, - name = AvpName}) -> - - %% Not testing just Name /= 'Failed-AVP' means we're changing the - %% packing of AVPs nested within Failed-AVP, but the point of - %% ignoring errors within Failed-AVP is to decode as much as - %% possible, and failing because a mandatory AVP couldn't be - %% packed into a dedicated field defeats that point. - - Failed == true - orelse Name == 'Failed-AVP' - orelse (Name == 'answer-message' andalso AvpName == 'Failed-AVP') - orelse not M - orelse not Strict. +%% avp_arity/5 + +avp_arity(Name, 'AVP', Mod, Opts, Avp) -> + pack_arity(Name, Opts, Mod, Avp); + +avp_arity(Name, AvpName, Mod, _, _) -> + Mod:avp_arity(Name, AvpName). + +%% rc/4 + +%% Length error communicated from diameter_types or a +%% @custom_types/@codecs module. +rc({'DIAMETER', 5014 = RC, _}, #diameter_avp{name = AvpName} = A, Opts, Mod) -> + {RC, A#diameter_avp{data = Mod:empty_value(AvpName, Opts)}}; + +%% 3588: +%% +%% DIAMETER_INVALID_AVP_VALUE 5004 +%% The request contained an AVP with an invalid value in its data +%% portion. A Diameter message indicating this error MUST include +%% the offending AVPs within a Failed-AVP AVP. +rc(_, Avp, _, _) -> + {5004, Avp}. %% pack/5 -pack(Arity, F, Avp, Mod, {Rec, AM, Failed}) -> - {set(Arity, F, value(F, Avp), Mod, Rec), AM, Failed}. +pack(Arity, F, Avp, Mod, [Failed | Rec]) -> + [Failed | set(Arity, F, value(F, Avp), Mod, Rec)]. %% set/5 @@ -730,7 +700,7 @@ set(_, _, _, _, false = No) -> set(1, F, Value, _, Map) when is_map(Map) -> - maps:put(F, Value, Map); + Map#{F => Value}; set(_, F, V, _, Map) when is_map(Map) -> @@ -755,36 +725,28 @@ value(_, #diameter_avp{value = V}) -> %% # grouped_avp/3 %% --------------------------------------------------------------------------- --spec grouped_avp(decode, avp_name(), binary() | {5014, binary()}, term()) +%% Note that Grouped is the only AVP type that doesn't just return a +%% decoded value, also returning the list of component diameter_avp +%% records. + +-spec grouped_avp(decode, avp_name(), binary(), term()) -> {avp_record(), [avp()]}; (encode, avp_name(), avp_record() | avp_values(), term()) -> iolist() | no_return(). -%% Length error induced by diameter_codec:collect_avps/1: the AVP -%% length in the header was too short (insufficient for the extracted -%% header) or too long (past the end of the message). An empty payload -%% is sufficient according to the RFC text for 5014. -grouped_avp(decode, _Name, {5014 = RC, _Bin}, _) -> - ?THROW({grouped, {RC, []}, []}); - -grouped_avp(decode, Name, Data, Opts) -> - grouped_decode(Name, diameter_codec:collect_avps(Data), Opts); +%% An error in decoding a component AVP throws the first faulty +%% component, which a catch wraps in the Grouped AVP in question. A +%% partially decoded record is only used when ignoring errors in +%% Failed-AVP. +grouped_avp(decode, Name, Bin, Opts) -> + {Rec, Avps, Es} = T = decode_avps(Name, Bin, Opts), + [] == Es orelse ?THROW(T), + {Rec, Avps}; grouped_avp(encode, Name, Data, Opts) -> encode_avps(Name, Data, Opts). -%% grouped_decode/2 -%% -%% Note that Grouped is the only AVP type that doesn't just return a -%% decoded value, also returning the list of component diameter_avp -%% records. - -%% Length error in trailing component AVP. -grouped_decode(_Name, {Error, Acc}, _) -> - {5014, Avp} = Error, - ?THROW({grouped, Error, [Avp | Acc]}); - %% 7.5. Failed-AVP AVP %% In the case where the offending AVP is embedded within a Grouped AVP, @@ -795,15 +757,6 @@ grouped_decode(_Name, {Error, Acc}, _) -> %% to the single offending AVP. This enables the recipient to detect %% the location of the offending AVP when embedded in a group. -%% An error in decoding a component AVP throws the first faulty -%% component, which the catch in d/3 wraps in the Grouped AVP in -%% question. A partially decoded record is only used when ignoring -%% errors in Failed-AVP. -grouped_decode(Name, ComponentAvps, Opts) -> - {Rec, Avps, Es} = decode_avps(Name, ComponentAvps, Opts), - [] == Es orelse ?THROW({grouped, [{_,_} = hd(Es) | Rec], Avps}), - {Rec, Avps}. - %% --------------------------------------------------------------------------- %% # empty_group/2 %% --------------------------------------------------------------------------- @@ -840,7 +793,7 @@ empty(Name, #{module := Mod} = Opts) -> newrec(false = No, _, _, _) -> No; -newrec(record, Mod, Name, #{strict_arities := T}) +newrec(record, Mod, Name, T) when T /= decode -> RecName = Mod:name2rec(Name), Sz = Mod:'#info-'(RecName, size), @@ -859,16 +812,15 @@ newrec(Mod, Name) -> %% reformat/5 -reformat(_, Map, Arities, _Mod, _Opts, list) -> - [{F,V} || {F,_} <- Arities, #{F := V} <- [Map]]; +reformat(Name, Map, _Strict, Mod, list) -> + [{F,V} || {F,_} <- Mod:avp_arity(Name), #{F := V} <- [Map]]; -reformat(Name, Map, Arities, Mod, Opts, record_from_map) -> - SA = maps:get(strict_arities, Opts, decode), +reformat(Name, Map, Strict, Mod, record_from_map) -> RecName = Mod:name2rec(Name), - list_to_tuple([RecName | [maps:get(F, Map, def(A, SA)) - || {F,A} <- Arities]]); + list_to_tuple([RecName | [mget(F, Map, def(A, Strict)) + || {F,A} <- Mod:avp_arity(Name)]]); -reformat(_, Rec, _, _, _, _) -> +reformat(_, Rec, _, _, _) -> Rec. %% def/2 diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index 3463a93568..3194cb4bf1 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -327,9 +327,7 @@ recv_request(Ack, %% A request was sent for an application that is not %% supported. RC = 3007, - Es = Pkt#diameter_packet.errors, - DecPkt = Pkt#diameter_packet{avps = collect_avps(Pkt), - errors = [RC | Es]}, + DecPkt = diameter_codec:collect_avps(Pkt), send_answer(answer_message(RC, Dict0, Caps, DecPkt), TPid, Dict0, @@ -345,14 +343,6 @@ recv_request(Ack, decode(Id, Dict, #recvdata{codec = Opts}, Pkt) -> errors(Id, diameter_codec:decode(Id, Dict, Opts, Pkt)). -collect_avps(Pkt) -> - case diameter_codec:collect_avps(Pkt) of - {_Error, Avps} -> - Avps; - Avps -> - Avps - end. - %% send_A/7 send_A([T | Fs], TPid, App, Dict0, RecvData, DecPkt, Caps) -> -- cgit v1.2.3 From 43b84071754a27894a44ffb0f34d4a03157ebeb5 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 3 Aug 2017 14:09:33 +0200 Subject: Don't extract options unnecessarily at encode Extract strict_arities once and pass it through as an argument. --- lib/diameter/src/base/diameter_gen.erl | 88 +++++++++++++++++----------------- 1 file changed, 45 insertions(+), 43 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl index 3688ff7b8f..4bfad345d0 100644 --- a/lib/diameter/src/base/diameter_gen.erl +++ b/lib/diameter/src/base/diameter_gen.erl @@ -60,8 +60,9 @@ | no_return(). encode_avps(Name, Vals, #{module := Mod} = Opts) -> + Strict = mget(strict_arities, Opts, encode), try - encode(Name, Vals, Opts, Mod) + encode(Name, Vals, Opts, Strict, Mod) catch throw: {?MODULE, Reason} -> diameter_lib:log({encode, error}, @@ -78,87 +79,88 @@ encode_avps(Name, Vals, #{module := Mod} = Opts) -> erlang:error({encode_failure, Reason, Name, Stack}) end. -%% encode/4 - -encode(Name, Vals, #{ordered_encode := false} = Opts, Mod) - when is_list(Vals) -> - lists:map(fun({F,V}) -> encode(Name, F, V, Opts, Mod) end, Vals); +%% encode/5 -encode(Name, Vals, Opts, Mod) +encode(Name, Vals, Opts, Strict, Mod) when is_list(Vals) -> - encode(Name, Mod:'#set-'(Vals, newrec(Mod, Name)), Opts, Mod); + case Opts of + #{ordered_encode := false} -> + lists:map(fun({F,V}) -> encode(Name, F, V, Opts, Strict, Mod) end, + Vals); + _ -> + Rec = Mod:'#set-'(Vals, newrec(Mod, Name)), + encode(Name, Rec, Opts, Strict, Mod) + end; -encode(Name, Map, Opts, Mod) +encode(Name, Map, Opts, Strict, Mod) when is_map(Map) -> - [enc(Name, F, A, V, Opts, Mod) || {F,A} <- Mod:avp_arity(Name), - V <- [mget(F, Map, undefined)]]; + [enc(Name, F, A, V, Opts, Strict, Mod) || {F,A} <- Mod:avp_arity(Name), + V <- [mget(F, Map, undefined)]]; -encode(Name, Rec, Opts, Mod) -> - [encode(Name, F, V, Opts, Mod) || {F,V} <- Mod:'#get-'(Rec)]. +encode(Name, Rec, Opts, Strict, Mod) -> + [encode(Name, F, V, Opts, Strict, Mod) || {F,V} <- Mod:'#get-'(Rec)]. -%% encode/5 +%% encode/6 -encode(Name, AvpName, Values, #{strict_arities := T} = Opts, Mod) - when T /= encode -> - enc(Name, AvpName, ?ANY, Values, Opts, Mod); +encode(Name, AvpName, Values, Opts, Strict, Mod) + when Strict /= encode -> + enc(Name, AvpName, ?ANY, Values, Opts, Strict, Mod); -encode(Name, AvpName, Values, Opts, Mod) -> - enc(Name, AvpName, Mod:avp_arity(Name, AvpName), Values, Opts, Mod). +encode(Name, AvpName, Values, Opts, Strict, Mod) -> + Arity = Mod:avp_arity(Name, AvpName), + enc(Name, AvpName, Arity, Values, Opts, Strict, Mod). -%% enc/6 +%% enc/7 -enc(Name, AvpName, Arity, Values, #{strict_arities := T} = Opts, Mod) - when T /= encode, Arity /= ?ANY -> - enc(Name, AvpName, ?ANY, Values, Opts, Mod); +enc(Name, AvpName, Arity, Values, Opts, Strict, Mod) + when Strict /= encode, Arity /= ?ANY -> + enc(Name, AvpName, ?ANY, Values, Opts, Strict, Mod); -enc(_, AvpName, 1, undefined, _, _) -> +enc(_, AvpName, 1, undefined, _, _, _) -> ?THROW([mandatory_avp_missing, AvpName]); -enc(Name, AvpName, 1, Value, Opts, Mod) -> +enc(Name, AvpName, 1, Value, Opts, _, Mod) -> H = avp_header(AvpName, Mod), enc1(Name, AvpName, H, Value, Opts, Mod); -enc(_, _, {0,_}, [], _, _) -> +enc(_, _, {0,_}, [], _, _, _) -> []; -enc(_, _, _, undefined, _, _) -> +enc(_, _, _, undefined, _, _, _) -> []; %% Be forgiving when a list of values is expected. If the value itself %% is a list then the user has to wrap it to avoid each member from %% being interpreted as an individual AVP value. -enc(Name, AvpName, Arity, V, Opts, Mod) +enc(Name, AvpName, Arity, V, Opts, Strict, Mod) when not is_list(V) -> - enc(Name, AvpName, Arity, [V], Opts, Mod); + enc(Name, AvpName, Arity, [V], Opts, Strict, Mod); -enc(Name, AvpName, {Min, Max}, Values, Opts, Mod) -> +enc(Name, AvpName, {Min, Max}, Values, Opts, Strict, Mod) -> H = avp_header(AvpName, Mod), - enc(Name, AvpName, H, Min, 0, Max, Values, Opts, Mod). + enc(Name, AvpName, H, Min, 0, Max, Values, Opts, Strict, Mod). -%% enc/9 - -enc(Name, AvpName, H, Min, N, '*', Vs, Opts, Mod) - when Min =< N -> - [enc1(Name, AvpName, H, V, Opts, Mod) || V <- Vs]; +%% enc/10 -enc(Name, AvpName, H, _, _, _, Vs, #{strict_arities := T} = Opts, Mod) - when T /= encode -> +enc(Name, AvpName, H, Min, N, Max, Vs, Opts, Strict, Mod) + when Strict /= encode; + Max == '*', Min =< N -> [enc1(Name, AvpName, H, V, Opts, Mod) || V <- Vs]; -enc(_, AvpName, _, Min, N, _, [], _, _) +enc(_, AvpName, _, Min, N, _, [], _, _, _) when N < Min -> ?THROW([repeated_avp_insufficient_arity, AvpName, Min, N]); -enc(_, _, _, _, _, _, [], _, _) -> +enc(_, _, _, _, _, _, [], _, _, _) -> []; -enc(_, AvpName, _, _, N, Max, _, _, _) +enc(_, AvpName, _, _, N, Max, _, _, _, _) when Max =< N -> ?THROW([repeated_avp_excessive_arity, AvpName, Max]); -enc(Name, AvpName, H, Min, N, Max, [V|Vs], Opts, Mod) -> +enc(Name, AvpName, H, Min, N, Max, [V|Vs], Opts, Strict, Mod) -> [enc1(Name, AvpName, H, V, Opts, Mod) - | enc(Name, AvpName, H, Min, N+1, Max, Vs, Opts, Mod)]. + | enc(Name, AvpName, H, Min, N+1, Max, Vs, Opts, Strict, Mod)]. %% avp_header/2 -- cgit v1.2.3 From 96cd627acfde682875149effda4611dc778622fd Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sun, 30 Jul 2017 01:27:42 +0200 Subject: Count AVPs in #diameter_avp.index The index field in record diameter_avp was previously used to enumerate AVPs so that the list could be returned in some cases, since diameter_codec:collect_avps/2 (now /1) reversed the order. That's no longer the case as of the grandparent commit, so use the field to enumerate instances of the same AVP instead, and only when arities are being checked, to save having to look them up in the map when checking for 5009 errors, or counting AVPs at all in diameter_codec:collect_avps/1. --- lib/diameter/src/base/diameter_codec.erl | 18 +++--- lib/diameter/src/base/diameter_gen.erl | 104 +++++++++++++++---------------- 2 files changed, 58 insertions(+), 64 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index c179c4b362..81c21fb8f2 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -489,9 +489,9 @@ collect_avps(#diameter_packet{bin = Bin} = Pkt) -> Pkt#diameter_packet{avps = collect_avps(Bin)}; collect_avps(<<_:20/binary, Avps/binary>>) -> - collect(Avps, 0). + collect(Avps). -%% collect/2 +%% collect/1 %% 0 1 2 3 %% 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 @@ -505,7 +505,7 @@ collect_avps(<<_:20/binary, Avps/binary>>) -> %% | Data ... %% +-+-+-+-+-+-+-+-+ -collect(<>, N)-> +collect(<>) -> Vid = if 1 == V -> I; 0 == V -> undefined end, DataLen = Len - 8 - V*4, %% Might be negative, which ensures Pad = ?PAD(Len), %% failure of the match below. @@ -518,9 +518,8 @@ collect(<>, N)-> vendor_id = Vid, is_mandatory = MB, need_encryption = PB, - data = Data, - index = N}, - [Avp | collect(T, N+1)]; + data = Data}, + [Avp | collect(T)]; _ -> %% Length in header points past the end of the message, or %% doesn't span the header. Note that an length error can @@ -532,16 +531,15 @@ collect(<>, N)-> vendor_id = Vid, is_mandatory = MB, need_encryption = PB, - data = {5014, Rest}, - index = N}] + data = {5014, Rest}}] end; -collect(<<>>, _) -> +collect(<<>>) -> []; %% Header is truncated. pack_avp/1 will pad this at encode if sent in %% a Failed-AVP. -collect(Bin, _) -> +collect(Bin) -> [#diameter_avp{data = {5014, Bin}}]. %% 3588: diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl index 4bfad345d0..f0196ccaa9 100644 --- a/lib/diameter/src/base/diameter_gen.erl +++ b/lib/diameter/src/base/diameter_gen.erl @@ -230,7 +230,7 @@ enc(AvpName, Value, Opts, Mod) -> decode_avps(Name, Bin, #{module := Mod, decode_format := Fmt} = Opts) -> Strict = mget(strict_arities, Opts, decode), [AM, Avps, Failed | Rec] - = decode(Bin, Name, Mod, Fmt, Strict, Opts, 0, #{}), + = decode(Bin, Name, Mod, Fmt, Strict, Opts, #{}), %% AM counts the number of top-level AVPs, which missing/5 then %% uses when appending 5005 errors. {reformat(Name, Rec, Strict, Mod, Fmt), @@ -241,7 +241,7 @@ decode_avps(Name, Bin, #{module := Mod, decode_format := Fmt} = Opts) -> %% encountered. Failed-AVP should typically contain the first %% error encountered. -%% decode/8 +%% decode/7 decode(<>, Name, @@ -249,7 +249,6 @@ decode(<>, Fmt, Strict, Opts0, - Idx, AM0) -> Vid = if 1 == V -> I; true -> undefined end, MB = 1 == M, @@ -258,12 +257,16 @@ decode(<>, DataLen = Len - 8 - 4*V, %% possibly negative, causing case match to fail Pad = (4 - (Len rem 4)) rem 4, + Opts = setopts(NameT, Name, MB, Opts0), + %% Not AvpName or else a failed Failed-AVP + %% decode is packed into 'AVP'. + + AvpName = field(NameT), + Arity = avp_arity(Name, AvpName, Mod, Opts, MB), + {Idx, AM} = incr(AvpName, Arity, Strict, AM0), %% count + case Rest of <> -> - Opts = setopts(NameT, Name, MB, Opts0), - %% Not AvpName or else a failed Failed-AVP - %% decode is packed into 'AVP'. - Avp = #diameter_avp{code = Code, vendor_id = Vid, is_mandatory = MB, @@ -272,15 +275,9 @@ decode(<>, name = name(NameT), type = type(NameT), index = Idx}, - - Dec = decode(Data, Name, NameT, Mod, Opts, Avp), %% decode - - AvpName = field(NameT), - Arity = avp_arity(Name, AvpName, Mod, Opts, Avp), - AM = incr(AvpName, Arity, Strict, AM0), %% count - - Acc = decode(T, Name, Mod, Fmt, Strict, Opts, Idx+1, AM),%% recurse - acc(Acc, Dec, Name, AvpName, Arity, Strict, Mod, Opts, AM0); + Dec = decode(Data, Name, NameT, Mod, Opts, Avp), %% decode + Acc = decode(T, Name, Mod, Fmt, Strict, Opts, AM), %% recurse + acc(Acc, Dec, Name, AvpName, Arity, Strict, Mod, Opts); _ -> Avp = #diameter_avp{code = Code, vendor_id = Vid, @@ -290,15 +287,14 @@ decode(<>, name = name(NameT), type = type(NameT), index = Idx}, - [AM0, [Avp], [{5014, Avp}] | newrec(Fmt, Mod, Name, Strict)] + [AM, [Avp], [{5014, Avp}] | newrec(Fmt, Mod, Name, Strict)] end; -decode(<<>>, Name, Mod, Fmt, Strict, _, _, AM) -> +decode(<<>>, Name, Mod, Fmt, Strict, _, AM) -> [AM, [], [] | newrec(Fmt, Mod, Name, Strict)]; -decode(Bin, Name, Mod, Fmt, Strict, _, Idx, AM) -> - Avp = #diameter_avp{data = Bin, - index = Idx}, +decode(Bin, Name, Mod, Fmt, Strict, _, AM) -> + Avp = #diameter_avp{data = Bin}, [AM, [Avp], [{5014, Avp}] | newrec(Fmt, Mod, Name, Strict)]. %% Data is a truncated header if command_code = undefined, otherwise @@ -321,15 +317,15 @@ incr(F, A, SA, AM) A == ?ANY; A == 0; SA /= decode -> - AM; + {undefined, AM}; incr(AvpName, _, _, AM) -> - maps:update_with(AvpName, fun incr/1, 1, AM). - -%% incr/1 - -incr(N) -> - N + 1. + case AM of + #{AvpName := N} -> + {N, AM#{AvpName => N+1}}; + _ -> + {0, AM#{AvpName => 1}} + end. %% mget/3 %% @@ -551,57 +547,57 @@ set_failed('Failed-AVP', #{failed_avp := false} = Opts) -> set_failed(_, Opts) -> Opts. -%% acc/9 +%% acc/8 -acc([AM | Acc], As, Name, AvpName, Arity, Strict, Mod, Opts, AM0) -> - [AM | acc1(Acc, As, Name, AvpName, Arity, Strict, Mod, Opts, AM0)]. +acc([AM | Acc], As, Name, AvpName, Arity, Strict, Mod, Opts) -> + [AM | acc1(Acc, As, Name, AvpName, Arity, Strict, Mod, Opts)]. -%% acc1/9 +%% acc1/8 %% Faulty AVP, not grouped. -acc1(Acc, {_RC, Avp} = E, _, _, _, _, _, _, _) -> +acc1(Acc, {_RC, Avp} = E, _, _, _, _, _, _) -> [Avps, Failed | Rec] = Acc, [[Avp | Avps], [E | Failed] | Rec]; %% Faulty component in grouped AVP. -acc1(Acc, {RC, As, Avp}, _, _, _, _, _, _, _) -> +acc1(Acc, {RC, As, Avp}, _, _, _, _, _, _) -> [Avps, Failed | Rec] = Acc, [[As | Avps], [{RC, Avp} | Failed] | Rec]; %% Grouped AVP ... -acc1([Avps | Acc], [Avp|_] = As, Name, AvpName, Arity, Strict, Mod, Opts, AM)-> - [[As|Avps] | acc2(Acc, Avp, Name, AvpName, Arity, Strict, Mod, Opts, AM)]; +acc1([Avps | Acc], [Avp|_] = As, Name, AvpName, Arity, Strict, Mod, Opts) -> + [[As|Avps] | acc2(Acc, Avp, Name, AvpName, Arity, Strict, Mod, Opts)]; %% ... or not. -acc1([Avps | Acc], Avp, Name, AvpName, Arity, Strict, Mod, Opts, AM) -> - [[Avp|Avps] | acc2(Acc, Avp, Name, AvpName, Arity, Strict, Mod, Opts, AM)]. +acc1([Avps | Acc], Avp, Name, AvpName, Arity, Strict, Mod, Opts) -> + [[Avp|Avps] | acc2(Acc, Avp, Name, AvpName, Arity, Strict, Mod, Opts)]. -%% acc2/9 +%% acc2/8 %% No errors, but nowhere to pack. -acc2(Acc, Avp, _, 'AVP', 0, _, _, _, _) -> +acc2(Acc, Avp, _, 'AVP', 0, _, _, _) -> [Failed | Rec] = Acc, [[{rc(Avp), Avp} | Failed] | Rec]; %% No AVP of this name: try to pack as 'AVP'. -acc2(Acc, Avp, Name, _, 0, Strict, Mod, Opts, AM) -> - Arity = pack_arity(Name, Opts, Mod, Avp), - acc2(Acc, Avp, Name, 'AVP', Arity, Strict, Mod, Opts, AM); +acc2(Acc, Avp, Name, AvpName, 0, Strict, Mod, Opts) -> + M = Avp#diameter_avp.is_mandatory, + Arity = pack_arity(Name, AvpName, Opts, Mod, M), + acc2(Acc, Avp, Name, 'AVP', Arity, Strict, Mod, Opts); %% Relaxed arities. -acc2(Acc, Avp, _, AvpName, Arity, Strict, Mod, _, _) +acc2(Acc, Avp, _, AvpName, Arity, Strict, Mod, _) when Strict /= decode -> pack(Arity, AvpName, Avp, Mod, Acc); %% No maximum arity. -acc2(Acc, Avp, _, AvpName, {_,'*'} = Arity, _, Mod, _, _) -> +acc2(Acc, Avp, _, AvpName, {_,'*'} = Arity, _, Mod, _) -> pack(Arity, AvpName, Avp, Mod, Acc); %% Or check. -acc2(Acc, Avp, _, AvpName, Arity, _, Mod, _, AM) -> - Count = maps:get(AvpName, AM, 0), +acc2(Acc, Avp, _, AvpName, Arity, _, Mod, _) -> Mx = max_arity(Arity), - if Mx =< Count -> + if Mx =< Avp#diameter_avp.index -> [Failed | Rec] = Acc, [[{5009, Avp} | Failed] | Rec]; true -> @@ -641,13 +637,13 @@ rc(#diameter_avp{is_mandatory = M}) -> %% Failed-AVP AVP MUST be included and contain a copy of the %% offending AVP. -%% pack_arity/4 +%% pack_arity/5 %% Give Failed-AVP special treatment since (1) it'll contain any %% unrecognized mandatory AVP's and (2) the RFC 3588 grammar failed to %% allow for Failed-AVP in an answer-message. -pack_arity(Name, _, Mod, #diameter_avp{is_mandatory = M, name = AvpName}) +pack_arity(Name, AvpName, _, Mod, M) when Name == 'Failed-AVP'; Name == 'answer-message', AvpName == 'Failed-AVP'; not M -> @@ -658,18 +654,18 @@ pack_arity(Name, _, Mod, #diameter_avp{is_mandatory = M, name = AvpName}) %% possible, and failing because a mandatory AVP couldn't be %% packed into a dedicated field defeats that point. -pack_arity(Name, #{strict_mbit := Strict, failed_avp := Failed}, Mod, _) +pack_arity(Name, _, #{strict_mbit := Strict, failed_avp := Failed}, Mod, _) when not Strict; Failed -> Mod:avp_arity(Name, 'AVP'); -pack_arity(_, _, _, _) -> +pack_arity(_, _, _, _, _) -> 0. %% avp_arity/5 -avp_arity(Name, 'AVP', Mod, Opts, Avp) -> - pack_arity(Name, Opts, Mod, Avp); +avp_arity(Name, 'AVP' = AvpName, Mod, Opts, M) -> + pack_arity(Name, AvpName, Opts, Mod, M); avp_arity(Name, AvpName, Mod, _, _) -> Mod:avp_arity(Name, AvpName). -- cgit v1.2.3 From 1a2c87f0035849fc5a24bff5dd36796500d1f451 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sat, 29 Jul 2017 16:53:18 +0200 Subject: Optimize sub-binaries With ERL_COMPILER_OPTIONS=bin_opt_info, before: base/diameter_codec.erl:508: Warning: NOT OPTIMIZED: the binary matching instruction that follows in the same function have problems that prevent delayed sub binary optimization (probably indicated by INFO warnings) And after: base/diameter_codec.erl:508: Warning: OPTIMIZED: creation of sub binary delayed This has a surprisingly large impact on the performance of diameter_codec:collect_avps/2: about 15% faster in one testcase on a RAR with 7 AVPs. --- lib/diameter/src/base/diameter_codec.erl | 51 ++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 23 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index 81c21fb8f2..c171ea9dca 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -506,20 +506,33 @@ collect_avps(<<_:20/binary, Avps/binary>>) -> %% +-+-+-+-+-+-+-+-+ collect(<>) -> - Vid = if 1 == V -> I; 0 == V -> undefined end, - DataLen = Len - 8 - V*4, %% Might be negative, which ensures - Pad = ?PAD(Len), %% failure of the match below. - MB = 1 == M, - PB = 1 == P, - - case Rest of - <> -> + collect(Rest, + Code, + if 1 == V -> I; 0 == V -> undefined end, + Len - 8 - V*4, %% Might be negative, which ensures + ?PAD(Len), %% failure of the match below. + 1 == M, + 1 == P); + +collect(<<>>) -> + []; + +%% Header is truncated. pack_avp/1 will pad this at encode if sent in +%% a Failed-AVP. +collect(Bin) -> + [#diameter_avp{data = {5014, Bin}}]. + +%% collect/7 + +collect(Bin, Code, Vid, DataLen, Pad, M, P) -> + case Bin of + <> -> Avp = #diameter_avp{code = Code, vendor_id = Vid, - is_mandatory = MB, - need_encryption = PB, + is_mandatory = M, + need_encryption = P, data = Data}, - [Avp | collect(T)]; + [Avp | collect(Rest)]; _ -> %% Length in header points past the end of the message, or %% doesn't span the header. Note that an length error can @@ -529,18 +542,10 @@ collect(<>) -> %% know the types of the AVPs being extracted. [#diameter_avp{code = Code, vendor_id = Vid, - is_mandatory = MB, - need_encryption = PB, - data = {5014, Rest}}] - end; - -collect(<<>>) -> - []; - -%% Header is truncated. pack_avp/1 will pad this at encode if sent in -%% a Failed-AVP. -collect(Bin) -> - [#diameter_avp{data = {5014, Bin}}]. + is_mandatory = M, + need_encryption = P, + data = {5014, Bin}}] + end. %% 3588: %% -- cgit v1.2.3 From bacca97210d31865c46b759115039b22f9ba9ed7 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sat, 29 Jul 2017 17:24:30 +0200 Subject: Optimize sub-binaries Also inline incr/8 and associated functions that were needed for the compiler to accept the optimization, since this does make for a measuable improvement. --- lib/diameter/src/base/diameter_gen.erl | 82 +++++++++++++++++++++++----------- 1 file changed, 56 insertions(+), 26 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl index f0196ccaa9..7a1a46ec52 100644 --- a/lib/diameter/src/base/diameter_gen.erl +++ b/lib/diameter/src/base/diameter_gen.erl @@ -26,6 +26,14 @@ -module(diameter_gen). +-compile({inline, [incr/8, + incr/4, + field/1, + setopts/4, + avp_arity/5, + set_failed/2, + set_strict/3]}). + -export([encode_avps/3, decode_avps/3, grouped_avp/4, @@ -248,54 +256,76 @@ decode(<>, Mod, Fmt, Strict, - Opts0, - AM0) -> - Vid = if 1 == V -> I; true -> undefined end, - MB = 1 == M, - PB = 1 == P, - NameT = Mod:avp_name(Code, Vid), %% {AvpName, Type} | 'AVP' - DataLen = Len - 8 - 4*V, %% possibly negative, causing case match to fail - Pad = (4 - (Len rem 4)) rem 4, + Opts, + AM) -> + decode(Rest, + Code, + if 1 == V -> I; true -> undefined end, + Len - 8 - 4*V, %% possibly negative, causing case match to fail + (4 - (Len rem 4)) rem 4, + 1 == M, + 1 == P, + Name, + Mod, + Fmt, + Strict, + Opts, + AM); - Opts = setopts(NameT, Name, MB, Opts0), - %% Not AvpName or else a failed Failed-AVP - %% decode is packed into 'AVP'. +decode(<<>>, Name, Mod, Fmt, Strict, _, AM) -> + [AM, [], [] | newrec(Fmt, Mod, Name, Strict)]; - AvpName = field(NameT), - Arity = avp_arity(Name, AvpName, Mod, Opts, MB), - {Idx, AM} = incr(AvpName, Arity, Strict, AM0), %% count +decode(Bin, Name, Mod, Fmt, Strict, _, AM) -> + Avp = #diameter_avp{data = Bin}, + [AM, [Avp], [{5014, Avp}] | newrec(Fmt, Mod, Name, Strict)]. + +%% decode/13 - case Rest of +decode(Bin, Code, Vid, DataLen, Pad, M, P, Name, Mod, Fmt, Strict, Opts0, AM0) -> + case Bin of <> -> + {NameT, AvpName, Arity, {Idx, AM}} + = incr(Name, Code, Vid, M, Mod, Strict, Opts0, AM0), + + Opts = setopts(NameT, Name, M, Opts0), + %% Not AvpName or else a failed Failed-AVP + %% decode is packed into 'AVP'. + Avp = #diameter_avp{code = Code, vendor_id = Vid, - is_mandatory = MB, - need_encryption = PB, + is_mandatory = M, + need_encryption = P, data = Data, name = name(NameT), type = type(NameT), index = Idx}, + Dec = decode(Data, Name, NameT, Mod, Opts, Avp), %% decode Acc = decode(T, Name, Mod, Fmt, Strict, Opts, AM), %% recurse acc(Acc, Dec, Name, AvpName, Arity, Strict, Mod, Opts); _ -> + {NameT, _AvpName, _Arity, {Idx, AM}} + = incr(Name, Code, Vid, M, Mod, Strict, Opts0, AM0), + Avp = #diameter_avp{code = Code, vendor_id = Vid, - is_mandatory = MB, - need_encryption = PB, - data = Rest, + is_mandatory = M, + need_encryption = P, + data = Bin, name = name(NameT), type = type(NameT), index = Idx}, + [AM, [Avp], [{5014, Avp}] | newrec(Fmt, Mod, Name, Strict)] - end; + end. -decode(<<>>, Name, Mod, Fmt, Strict, _, AM) -> - [AM, [], [] | newrec(Fmt, Mod, Name, Strict)]; +%% incr/8 -decode(Bin, Name, Mod, Fmt, Strict, _, AM) -> - Avp = #diameter_avp{data = Bin}, - [AM, [Avp], [{5014, Avp}] | newrec(Fmt, Mod, Name, Strict)]. +incr(Name, Code, Vid, M, Mod, Strict, Opts, AM0) -> + NameT = Mod:avp_name(Code, Vid), %% {AvpName, Type} | 'AVP' + AvpName = field(NameT), + Arity = avp_arity(Name, AvpName, Mod, Opts, M), + {NameT, AvpName, Arity, incr(AvpName, Arity, Strict, AM0)}. %% Data is a truncated header if command_code = undefined, otherwise %% payload bytes. The former is padded to the length of a header if -- cgit v1.2.3 From 5beaaf0cdc856c1a7623a1beee4da74aa9c1825e Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 20 Jul 2017 22:58:53 +0200 Subject: Fix type spec That dialyzer hasn't noticed is broken. --- lib/diameter/src/base/diameter_traffic.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index 3194cb4bf1..cfa857e09a 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -220,13 +220,13 @@ incr_rc(Dir, Pkt, TPid, Dict0) -> -> pid() %% request handler | boolean() %% answer, known request or not | discard %% request discarded by MFA - when Route :: {Handler, RequestRef, Seqs} + when Route :: {Handler, RequestRef, TPid} | Ack, RecvData :: {[SpawnOpt], #recvdata{}}, SpawnOpt :: term(), Handler :: pid(), RequestRef :: reference(), - Seqs :: {0..16#FFFFFFFF, 0..16#FFFFFFFF}, + TPid :: pid(), Ack :: boolean(). receive_message(TPid, Route, Pkt, Dict0, RecvData) -> -- cgit v1.2.3 From b435807b3aec896493cf3f2fc029c6c12b80ed55 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sun, 23 Jul 2017 13:10:06 +0200 Subject: Restructure/simplify message reception in diameter_peer_fsm Create less garbage. --- lib/diameter/src/base/diameter_peer_fsm.erl | 153 ++++++++++++++-------------- 1 file changed, 74 insertions(+), 79 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index 36014df94e..4870b86be5 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -653,10 +653,6 @@ encode(Rec, Opts, Dict) -> %% incoming/2 -incoming({recv = T, Name, Pkt}, #state{parent = Pid, ack = Ack} = S) -> - Pid ! {T, self(), get_route(Ack, Pkt), Name, Pkt}, - rcv(Name, Pkt, S); - incoming(#diameter_header{is_request = R}, #state{transport = TPid, ack = Ack}) -> R andalso Ack andalso send(TPid, false), @@ -674,98 +670,97 @@ incoming(T, _) -> %% recv/2 -recv(#diameter_packet{header = #diameter_header{} = Hdr} - = Pkt, - #state{dictionary = Dict0} - = S) -> - recv1(diameter_codec:msg_name(Dict0, Hdr), Pkt, S); - -recv(#diameter_packet{header = undefined, - bin = Bin} - = Pkt, - S) -> - recv(diameter_codec:decode_header(Bin), Pkt, S); +recv(#diameter_packet{bin = Bin} = Pkt, S) -> + recv(Bin, Pkt, S); recv(Bin, S) -> - recv(#diameter_packet{bin = Bin}, S). + recv(Bin, Bin, S). + +%% recv/3 + +recv(Bin, Msg, S) -> + recv(diameter_codec:decode_header(Bin), Bin, Msg, S). + +%% recv/4 -%% recv1/3 +recv(false, Bin, _, #state{length_errors = E}) -> + invalid(E, truncated_header, Bin), + Bin; + +recv(#diameter_header{length = Len} = H, Bin, Msg, #state{length_errors = E, + incoming_maxlen = M, + dictionary = Dict0} + = S) + when E == handle; + 0 == Len rem 4, bit_size(Bin) == 8*Len, size(Bin) =< M -> + recv1(diameter_codec:msg_name(Dict0, H), H, Msg, S); -recv1(_, - #diameter_packet{header = H, bin = Bin}, - #state{incoming_maxlen = M}) +recv(H, Bin, _, #state{incoming_maxlen = M}) when M < size(Bin) -> invalid(false, incoming_maxlen_exceeded, {size(Bin), H}), H; +recv(H, Bin, _, #state{length_errors = E}) -> + T = {size(Bin), bit_size(Bin) rem 8, H}, + invalid(E, message_length_mismatch, T), + H. + +%% recv1/4 + %% Ignore anything but an expected CER/CEA if so configured. This is %% non-standard behaviour. -recv1(Name, #diameter_packet{header = H}, #state{state = {'Wait-CEA', _, _}, - strict = false}) +recv1(Name, H, _, #state{state = {'Wait-CEA', _, _}, + strict = false}) when Name /= 'CEA' -> H; -recv1(Name, #diameter_packet{header = H}, #state{state = recv_CER, - strict = false}) +recv1(Name, H, _, #state{state = recv_CER, + strict = false}) when Name /= 'CER' -> H; %% Incoming request after outgoing DPR: discard. Don't discard DPR, so %% both ends don't do so when sending simultaneously. -recv1(Name, - #diameter_packet{header = #diameter_header{is_request = true} = H}, - #state{dpr = {_,_,_}}) +recv1(Name, #diameter_header{is_request = true} = H, _, #state{dpr = {_,_,_}}) when Name /= 'DPR' -> invalid(false, recv_after_outgoing_dpr, H), H; %% Incoming request after incoming DPR: discard. -recv1(_, - #diameter_packet{header = #diameter_header{is_request = true} = H}, - #state{dpr = true}) -> +recv1(_, #diameter_header{is_request = true} = H, _, #state{dpr = true}) -> invalid(false, recv_after_incoming_dpr, H), H; %% DPA with identifier mismatch, or in response to a DPR initiated by %% the service. -recv1('DPA' = N, - #diameter_packet{header = #diameter_header{hop_by_hop_id = Hid, - end_to_end_id = Eid}} - = Pkt, - #state{dpr = {X,H,E}} +recv1('DPA' = Name, + #diameter_header{hop_by_hop_id = Hid, end_to_end_id = Eid} + = H, + Msg, + #state{dpr = {X,HI,EI}} = S) - when H /= Hid; - E /= Eid; + when HI /= Hid; + EI /= Eid; not X -> - rcv(N, Pkt, S); + Pkt = pkt(H, Msg), + handle(Name, Pkt, S); -%% Any other message with a header and no length errors: send to the -%% parent. -recv1(Name, Pkt, #state{}) -> - {recv, Name, Pkt}. +%% Any other message with a header and no length errors. +recv1(Name, H, Msg, #state{parent = Pid, ack = Ack} = S) -> + Pkt = pkt(H, Msg), + Pid ! {recv, self(), get_route(Ack, Pkt), Name, Pkt}, + handle(Name, Pkt, S). -%% recv/3 +%% pkt/2 -recv(#diameter_header{length = Len} - = H, - #diameter_packet{bin = Bin} - = Pkt, - #state{length_errors = E} - = S) - when E == handle; - 0 == Len rem 4, bit_size(Bin) == 8*Len -> - recv(Pkt#diameter_packet{header = H}, S); +pkt(H, Bin) + when is_binary(Bin) -> + #diameter_packet{header = H, + bin = Bin}; -recv(#diameter_header{} - = H, - #diameter_packet{bin = Bin}, - #state{length_errors = E}) -> - T = {size(Bin), bit_size(Bin) rem 8, H}, - invalid(E, message_length_mismatch, T), - Bin; +pkt(H, Pkt) -> + Pkt#diameter_packet{header = H}. -recv(false, #diameter_packet{bin = Bin}, #state{length_errors = E}) -> - invalid(E, truncated_header, Bin), - Bin. +%% invalid/3 %% Note that counters here only count discarded messages. invalid(E, Reason, T) -> @@ -774,39 +769,39 @@ invalid(E, Reason, T) -> ?LOG(Reason, T), ok. -%% rcv/3 +%% handle/3 %% Incoming CEA. -rcv('CEA' = N, - #diameter_packet{header = #diameter_header{end_to_end_id = Eid, - hop_by_hop_id = Hid}} - = Pkt, - #state{state = {'Wait-CEA', Hid, Eid}} - = S) -> +handle('CEA' = N, + #diameter_packet{header = #diameter_header{end_to_end_id = Eid, + hop_by_hop_id = Hid}} + = Pkt, + #state{state = {'Wait-CEA', Hid, Eid}} + = S) -> ?LOG(recv, N), handle_CEA(Pkt, S); %% Incoming CER -rcv('CER' = N, Pkt, #state{state = recv_CER} = S) -> +handle('CER' = N, Pkt, #state{state = recv_CER} = S) -> handle_request(N, Pkt, S); %% Anything but CER/CEA in a non-Open state is an error, as is %% CER/CEA in anything but recv_CER/Wait-CEA. -rcv(Name, _, #state{state = PS}) +handle(Name, _, #state{state = PS}) when PS /= 'Open'; Name == 'CER'; Name == 'CEA' -> {stop, {Name, PS}}; -rcv('DPR' = N, Pkt, S) -> +handle('DPR' = N, Pkt, S) -> handle_request(N, Pkt, S); %% DPA in response to DPR, with the expected identifiers. -rcv('DPA' = N, - #diameter_packet{header = #diameter_header{end_to_end_id = Eid, - hop_by_hop_id = Hid} - = H} - = Pkt, +handle('DPA' = N, + #diameter_packet{header = #diameter_header{end_to_end_id = Eid, + hop_by_hop_id = Hid} + = H} + = Pkt, #state{dictionary = Dict0, transport = TPid, dpr = {X, Hid, Eid}, @@ -825,13 +820,13 @@ rcv('DPA' = N, %% Ignore an unsolicited DPA in particular. Note that dpa_timeout %% deals with the case in which the peer sends the wrong identifiers %% in DPA. -rcv('DPA' = N, #diameter_packet{header = H}, _) -> +handle('DPA' = N, #diameter_packet{header = H}, _) -> ?LOG(ignored, N), %% Note that these aren't counted in the normal recv counter. diameter_stats:incr({diameter_codec:msg_id(H), recv, ignored}), ok; -rcv(_, _, _) -> +handle(_, _, _) -> ok. %% incr/3 -- cgit v1.2.3 From 90b18515763cc332a1f2e612a24c50f245b5dc72 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 8 Aug 2017 21:42:14 +0200 Subject: Split AVPs at decode Despite what the Efficiency Guide says about match being more efficient, split_binary appears to be slightly faster. (Although this one extraction is a drop in the bucket.) Binary optimizations aren't an issue during decode. --- lib/diameter/src/base/diameter_codec.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index c171ea9dca..63e39b12d1 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -321,7 +321,7 @@ decode_avps('', _, _, _, #diameter_packet{header = H, %% unknown message %% msg = undefined identifies this case. decode_avps(MsgName, Mod, AppMod, Opts, #diameter_packet{bin = Bin} = Pkt) -> - <<_:20/binary, Avps/binary>> = Bin, + {_, Avps} = split_binary(Bin, 20), {Rec, As, Errors} = Mod:decode_avps(MsgName, Avps, Opts#{dictionary => AppMod, -- cgit v1.2.3 From 2be3e00698dea8236e60f3262c50b37eebfb4a04 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 10 Aug 2017 11:13:51 +0200 Subject: Fix type spec That dialyzer hasn't noticed is broken. --- lib/diameter/src/base/diameter_traffic.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index cfa857e09a..84ae33ae9c 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -186,7 +186,7 @@ incr_error(Dir, Id, TPid) -> %% --------------------------------------------------------------------------- -spec incr_rc(send|recv, Pkt, TPid, DictT) - -> {Counter, non_neg_integer()} + -> Counter | Reason when Pkt :: #diameter_packet{}, TPid :: pid(), -- cgit v1.2.3 From eb5fa7e2a0f67924828ea3d81891b86d08a53b0c Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 10 Aug 2017 11:14:14 +0200 Subject: Add service_opt() traffic_counters To be able to disable the counting of messages for which application callbacks take place. Messages sent/handled by diameter itself are always counted. --- lib/diameter/src/base/diameter.erl | 1 + lib/diameter/src/base/diameter_config.erl | 3 + lib/diameter/src/base/diameter_service.erl | 1 + lib/diameter/src/base/diameter_traffic.erl | 295 ++++++++++++++++------------ lib/diameter/src/base/diameter_watchdog.erl | 2 +- 5 files changed, 171 insertions(+), 131 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index c919ff4c93..2e18a1d903 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -353,6 +353,7 @@ call(SvcName, App, Message) -> | {sequence, sequence() | evaluable()} | {share_peers, remotes()} | {decode_format, decode_format()} + | {traffic_counters, boolean()} | {string_decode, boolean()} | {strict_arities, true | strict_arities()} | {strict_mbit, boolean()} diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index 2b069f40cc..f1b6e56782 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -715,6 +715,7 @@ make_config(SvcName, Opts) -> {true, strict_arities}, {true, strict_mbit}, {record, decode_format}, + {true, traffic_counters}, {true, string_decode}, {[], spawn_opt}]), @@ -760,6 +761,7 @@ opt(K, false = B) K == strict_arities; K == strict_mbit; K == decode_format; + K == traffic_counters; K == string_decode -> B; @@ -768,6 +770,7 @@ opt(K, true = B) K == use_shared_peers; K == strict_arities; K == strict_mbit; + K == traffic_counters; K == string_decode -> B; diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl index c3c3c4b0e7..07f1fd3a4a 100644 --- a/lib/diameter/src/base/diameter_service.erl +++ b/lib/diameter/src/base/diameter_service.erl @@ -115,6 +115,7 @@ strict_arities => diameter:strict_arities(), strict_mbit := boolean(), decode_format := diameter:decode_format(), + traffic_counters := boolean(), string_decode := boolean(), spawn_opt := list() | {module(), atom(), list()}}}). diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index 84ae33ae9c..27a41d6eb0 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -70,12 +70,13 @@ timeout = 5000 :: 0..16#FFFFFFFF, %% for outgoing requests detach = false :: boolean()}). -%% Term passed back to receive_message/6 with every incoming message. +%% Term passed back to receive_message/5 with every incoming message. -record(recvdata, {peerT :: ets:tid(), service_name :: diameter:service_name(), apps :: [#diameter_app{}], sequence :: diameter:sequence(), + counters :: boolean(), codec :: #{decode_format := diameter:decode_format(), string_decode := boolean(), strict_arities => diameter:strict_arities(), @@ -98,12 +99,13 @@ %% --------------------------------------------------------------------------- make_recvdata([SvcName, PeerT, Apps, SvcOpts | _]) -> - #{sequence := {_,_} = Mask, spawn_opt := Opts} + #{sequence := {_,_} = Mask, spawn_opt := Opts, traffic_counters := B} = SvcOpts, {Opts, #recvdata{service_name = SvcName, peerT = PeerT, apps = Apps, sequence = Mask, + counters = B, codec = maps:with([decode_format, string_decode, strict_arities, @@ -197,18 +199,26 @@ incr_error(Dir, Id, TPid) -> | {'Experimental-Result', integer(), integer()}, Reason :: atom(). -incr_rc(Dir, Pkt, TPid, {_, AppDict, _} = DictT) -> - try - incr_result(Dir, Pkt, TPid, DictT) +incr_rc(Dir, Pkt, TPid, {MsgDict, AppDict, Dict0}) -> + incr_rc(Dir, Pkt, TPid, MsgDict, AppDict, Dict0); + +incr_rc(Dir, Pkt, TPid, Dict0) -> + incr_rc(Dir, Pkt, TPid, Dict0, Dict0, Dict0). + +%% incr_rc/6 + +incr_rc(Dir, Pkt, TPid, MsgDict, AppDict, Dict0) -> + try get_result(Dir, MsgDict, Dict0, Pkt) of + false -> + unknown; + Avp -> + incr_result(Dir, Avp, Pkt, TPid, AppDict) catch exit: {E,_} when E == no_result_code; E == invalid_error_bit -> incr(TPid, {msg_id(Pkt#diameter_packet.header, AppDict), Dir, E}), E - end; - -incr_rc(Dir, Pkt, TPid, Dict0) -> - incr_rc(Dir, Pkt, TPid, {Dict0, Dict0, Dict0}). + end. %% --------------------------------------------------------------------------- %% receive_message/5 @@ -307,14 +317,15 @@ recv_request(Ack, = Pkt, Dict0, #recvdata{peerT = PeerT, - apps = Apps} + apps = Apps, + counters = Count} = RecvData) -> Ack andalso (TPid ! {handler, self()}), case diameter_service:find_incoming_app(PeerT, TPid, Id, Apps) of {#diameter_app{id = Aid, dictionary = AppDict} = App, Caps} -> - incr(recv, Pkt, TPid, AppDict), + Count andalso incr(recv, Pkt, TPid, AppDict), DecPkt = decode(Aid, AppDict, RecvData, Pkt), - incr_error(recv, DecPkt, TPid, AppDict), + Count andalso incr_error(recv, DecPkt, TPid, AppDict), send_A(recv_R(App, TPid, Dict0, Caps, RecvData, DecPkt), TPid, App, @@ -535,6 +546,7 @@ send_A({call, Opts}, TPid, App, Dict0, RecvData, Pkt, Caps, Fs) -> MsgDict, AppDict, Dict0, + RecvData#recvdata.counters, Fs); RC -> send_answer(answer_message(RC, Dict0, Caps, Pkt), @@ -578,14 +590,22 @@ send_answer(Ans, TPid, MsgDict, AppDict, Dict0, RecvData, DecPkt, Fs) -> TPid, RecvData#recvdata.codec, make_answer_packet(Ans, DecPkt, MsgDict, Dict0)), - send_answer(Pkt, TPid, MsgDict, AppDict, Dict0, Fs). + send_answer(Pkt, + TPid, + MsgDict, + AppDict, + Dict0, + RecvData#recvdata.counters, + Fs). -%% send_answer/6 +%% send_answer/7 -send_answer(Pkt, TPid, MsgDict, AppDict, Dict0, [EvalPktFs | EvalFs]) -> +send_answer(Pkt, TPid, MsgDict, AppDict, Dict0, Count, [EvalPktFs | EvalFs]) -> eval_packet(Pkt, EvalPktFs), - incr(send, Pkt, TPid, AppDict), - incr_rc(send, Pkt, TPid, {MsgDict, AppDict, Dict0}), %% count outgoing + Count andalso begin + incr(send, Pkt, TPid, AppDict), + incr_rc(send, Pkt, TPid, MsgDict, AppDict, Dict0) + end, send(TPid, z(Pkt), _Route = self()), lists:foreach(fun diameter_lib:eval/1, EvalFs). @@ -1110,48 +1130,31 @@ find_avp(Code, VId, [_ | Avps]) -> %% Message sent as a header/avps list. incr_result(send = Dir, - #diameter_packet{msg = [#diameter_header{} = H | _]} - = Pkt, + Avp, + #diameter_packet{msg = [#diameter_header{} = H | _]}, TPid, - DictT) -> - incr_res(Dir, Pkt#diameter_packet{header = H}, TPid, DictT); - -%% Outgoing message as binary: don't count. (Sending binaries is only -%% partially supported.) -incr_result(send, #diameter_packet{header = undefined = No}, _, _) -> - No; + AppDict) -> + incr_result(Dir, Avp, H, [], TPid, AppDict); %% Incoming or outgoing. Outgoing with encode errors never gets here %% since encode fails. -incr_result(Dir, Pkt, TPid, DictT) -> - incr_res(Dir, Pkt, TPid, DictT). - -incr_res(Dir, - #diameter_packet{header = #diameter_header{is_error = E} - = Hdr, - errors = Es} - = Pkt, - TPid, - DictT) -> - {MsgDict, AppDict, Dict0} = DictT, +incr_result(Dir, Avp, Pkt, TPid, AppDict) -> + #diameter_packet{header = H, errors = Es} + = Pkt, + incr_result(Dir, Avp, H, Es, TPid, AppDict). +%% incr_result/6 + +incr_result(Dir, Avp, Hdr, Es, TPid, AppDict) -> Id = msg_id(Hdr, AppDict), %% Could be {relay, 0}, in which case the R-bit is redundant since %% only answers are being counted. Let it be however, so that the %% same tuple is in both send/recv and result code counters. %% Count incoming decode errors. - recv /= Dir orelse [] == Es orelse incr_error(Dir, Id, TPid, AppDict), - - %% Exit on a missing result code. - T = rc_counter(MsgDict, Dir, Pkt), - T == false andalso ?LOGX(no_result_code, {MsgDict, Dir, Hdr}), - {Ctr, RC, Avp} = T, - - %% Or on an inappropriate value. - is_result(RC, E, Dict0) - orelse ?LOGX(invalid_error_bit, {MsgDict, Dir, Hdr, Avp}), + send == Dir orelse [] == Es orelse incr_error(Dir, Id, TPid, AppDict), + Ctr = rcc(Avp), incr(TPid, {Id, Dir, Ctr}), Ctr. @@ -1196,7 +1199,50 @@ is_result(RC, true, _) -> incr(TPid, Counter) -> diameter_stats:incr(Counter, TPid, 1). -%% rc_counter/3 +%% rcc/1 + +rcc(#diameter_avp{name = 'Result-Code' = Name, value = V}) -> + {Name, head(V)}; + +rcc(#diameter_avp{name = 'Experimental-Result', value = V}) -> + head(V). + +%% head/1 + +head([V|_]) -> + V; +head(V) -> + V. + +%% rcv/1 + +rcv(#diameter_avp{name = N, value = V}) -> + rcv(N, head(V)). + +%% rcv/2 + +rcv('Experimental-Result', {_,_,N}) -> + N; + +rcv('Result-Code', N) -> + N. + +%% get_result/4 + +%% Message sent as binary: no checks or counting. +get_result(_, _, _, #diameter_packet{header = undefined}) -> + false; + +get_result(Dir, MsgDict, Dict0, Pkt) -> + Avp = get_result(MsgDict, msg(Dir, Pkt)), + Hdr = Pkt#diameter_packet.header, + %% Exit on a missing result code or inappropriate value. + Avp == false + andalso ?LOGX(no_result_code, {MsgDict, Dir, Hdr}), + E = Hdr#diameter_header.is_error, + is_result(rcv(Avp), E, Dict0) + orelse ?LOGX(invalid_error_bit, {MsgDict, Dir, Hdr, Avp}), + Avp. %% RFC 3588, 7.6: %% @@ -1204,46 +1250,29 @@ incr(TPid, Counter) -> %% applications MUST include either one Result-Code AVP or one %% Experimental-Result AVP. -rc_counter(Dict, Dir, #diameter_packet{header = H, - avps = As, - msg = Msg}) +%% msg/2 + +msg(Dir, #diameter_packet{header = H, + avps = As, + msg = Msg}) when Dir == recv; %% decoded incoming Msg == undefined -> %% relayed outgoing - rc_counter(Dict, [H|As]); - -rc_counter(Dict, _, #diameter_packet{msg = Msg}) -> - rc_counter(Dict, Msg). - -rc_counter(Dict, Msg) -> - rcc(get_result(Dict, Msg)). - -rcc(#diameter_avp{name = 'Result-Code' = Name, value = N} = A) - when is_integer(N) -> - {{Name, N}, N, A}; - -rcc(#diameter_avp{name = 'Result-Code' = Name, value = [N|_]} = A) - when is_integer(N) -> - {{Name, N}, N, A}; + [H|As]; -rcc(#diameter_avp{name = 'Experimental-Result', value = {_,_,N} = T} = A) - when is_integer(N) -> - {T, N, A}; - -rcc(#diameter_avp{name = 'Experimental-Result', value = [{_,_,N} = T|_]} = A) - when is_integer(N) -> - {T, N, A}; - -rcc(_) -> - false. +msg(_, #diameter_packet{msg = Msg}) -> + Msg. %% get_result/2 get_result(Dict, Msg) -> try [throw(A) || N <- ['Result-Code', 'Experimental-Result'], - #diameter_avp{} = A <- [get_avp(Dict, N, Msg)]] + #diameter_avp{} = A <- [get_avp(Dict, N, Msg)], + is_integer(catch rcv(A))], + false catch - #diameter_avp{} = A -> A + #diameter_avp{} = A -> + A end. x(T) -> @@ -1367,7 +1396,7 @@ make_opts([T | _], _, _, _, _, _) -> send_request({{TPid, _Caps} = TC, App} = Transport, - #{sequence := Mask} + #{sequence := Mask, traffic_counters := Count} = SvcOpts, Msg0, CallOpts, @@ -1383,9 +1412,15 @@ send_request({{TPid, _Caps} = TC, App} SvcOpts, ReqPkt), eval_packet(EncPkt, Fs), - T = send_R(ReqPkt, EncPkt, Transport, CallOpts, Caller, SvcName), + T = send_R(ReqPkt, + EncPkt, + Transport, + CallOpts, + Caller, + Count, + SvcName), Ans = recv_answer(SvcName, App, CallOpts, T), - handle_answer(SvcName, SvcOpts, App, Ans); + handle_answer(SvcName, Count, SvcOpts, App, Ans); {discard, Reason} -> {error, Reason}; discard -> @@ -1528,6 +1563,7 @@ send_R(ReqPkt, {{TPid, _Caps} = TC, #diameter_app{dictionary = AppDict}}, #options{timeout = Timeout}, {Pid, Ref}, + Count, SvcName) -> Req = #request{ref = Ref, caller = Pid, @@ -1535,7 +1571,7 @@ send_R(ReqPkt, peer = TC, packet = ReqPkt}, - incr(send, EncPkt, TPid, AppDict), + Count andalso incr(send, EncPkt, TPid, AppDict), {TRef, MRef} = zend_requezt(TPid, EncPkt, Req, SvcName, Timeout), Pid ! Ref, %% tell caller a send has been attempted {TRef, MRef, Req}. @@ -1567,15 +1603,16 @@ failover(SvcName, App, Req, CallOpts) -> CallOpts, SvcName). -%% handle_answer/4 +%% handle_answer/5 -handle_answer(SvcName, _, App, {error, Req, Reason}) -> +handle_answer(SvcName, _, _, App, {error, Req, Reason}) -> #request{packet = Pkt, peer = {_TPid, _Caps} = TC} = Req, cb(App, handle_error, [Reason, msg(Pkt), SvcName, TC]); handle_answer(SvcName, + Count, SvcOpts, #diameter_app{id = Id, dictionary = AppDict, @@ -1589,43 +1626,50 @@ handle_answer(SvcName, #request{peer = {TPid, _}} = Req, - incr(recv, DecPkt, TPid, AppDict), - - AnsPkt = try - incr_result(recv, DecPkt, TPid, {MsgDict, AppDict, Dict0}) - of - _ -> DecPkt - catch - exit: {no_result_code, _} -> - %% RFC 6733 requires one of Result-Code or - %% Experimental-Result, but the decode will have - %% detected a missing AVP. If both are optional in - %% the dictionary then this isn't a decode error: - %% just continue on. - DecPkt; - exit: {invalid_error_bit, {_, _, _, Avp}} -> - #diameter_packet{errors = Es} - = DecPkt, - E = {5004, Avp}, - DecPkt#diameter_packet{errors = [E|Es]} - end, - - handle_answer(AnsPkt, SvcName, App, AE, Req). + answer(answer(DecPkt, TPid, MsgDict, AppDict, Dict0, Count), + SvcName, + App, + AE, + Req). + +%% answer/6 + +answer(DecPkt, TPid, MsgDict, AppDict, Dict0, Count) -> + Count andalso incr(recv, DecPkt, TPid, AppDict), + try get_result(recv, MsgDict, Dict0, DecPkt) of + Avp -> + Count andalso false /= Avp + andalso incr_result(recv, Avp, DecPkt, TPid, AppDict), + DecPkt + catch + exit: {no_result_code, _} -> + %% RFC 6733 requires one of Result-Code or + %% Experimental-Result, but the decode will have + %% detected a missing AVP. If both are optional in + %% the dictionary then this isn't a decode error: + %% just continue on. + DecPkt; + exit: {invalid_error_bit, {_, _, _, Avp}} -> + #diameter_packet{errors = Es} + = DecPkt, + E = {5004, Avp}, + DecPkt#diameter_packet{errors = [E|Es]} + end. -%% handle_answer/5 +%% answer/5 -handle_answer(#diameter_packet{errors = Es} - = Pkt, - SvcName, - App, - AE, - #request{peer = {_TPid, _Caps} = TC, - packet = P}) +answer(#diameter_packet{errors = Es} + = Pkt, + SvcName, + App, + AE, + #request{peer = {_TPid, _Caps} = TC, + packet = P}) when callback == AE; [] == Es -> cb(App, handle_answer, [Pkt, msg(P), SvcName, TC]); -handle_answer(#diameter_packet{header = H}, SvcName, _, AE, _) -> +answer(#diameter_packet{header = H}, SvcName, _, AE, _) -> handle_error(H, SvcName, AE). %% handle_error/3 @@ -1838,10 +1882,8 @@ get_destination(Dict, Msg) -> [str(get_avp_value(Dict, D, Msg)) || D <- ['Destination-Realm', 'Destination-Host']]. -%% This is not entirely correct. The avp could have an arity 1, in -%% which case an empty list is a DiameterIdentity of length 0 rather -%% than the list of no values we treat it as by mapping to undefined. -%% This behaviour is documented. +%% A DiameterIdentity has length at least one, so an empty list is not +%% a Realm/Host. str([]) -> undefined; str(T) -> @@ -1871,10 +1913,8 @@ get_avp(?RELAY, Name, Msg) -> get_avp(Dict, Name, [#diameter_header{} | Avps]) -> try {Code, _, VId} = Dict:avp_header(Name), - find_avp(Code, VId, Avps) - of - A -> - (avp_decode(Dict, Name, ungroup(A)))#diameter_avp{name = Name} + A = find_avp(Code, VId, Avps), + (avp_decode(Dict, Name, ungroup(A)))#diameter_avp{name = Name} catch error: _ -> undefined @@ -1931,13 +1971,8 @@ avp_decode(Dict, Name, #diameter_avp{value = undefined, data = Bin} = Avp) when is_binary(Bin) -> - try Dict:avp(decode, Bin, Name, decode_opts(Dict)) of - V -> - Avp#diameter_avp{value = V} - catch - error:_ -> - Avp - end; + V = Dict:avp(decode, Bin, Name, decode_opts(Dict)), + Avp#diameter_avp{value = V}; avp_decode(_, _, #diameter_avp{} = Avp) -> Avp. diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl index 294325751e..b2172356ee 100644 --- a/lib/diameter/src/base/diameter_watchdog.erl +++ b/lib/diameter/src/base/diameter_watchdog.erl @@ -154,7 +154,7 @@ i({Ack, T, Pid, {Opts, receive_data = RecvData, dictionary = Dict0, config = - maps:without(CodecKeys, + maps:without([traffic_counters | CodecKeys], config(SvcOpts#{restrict => restrict(Nodes), suspect => 1, okay => 3}, -- cgit v1.2.3