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 +- lib/diameter/src/compiler/diameter_codegen.erl | 17 ++++++++--------- 2 files changed, 9 insertions(+), 10 deletions(-) (limited to 'lib/diameter/src') 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]). diff --git a/lib/diameter/src/compiler/diameter_codegen.erl b/lib/diameter/src/compiler/diameter_codegen.erl index f56e4a5249..4e6fe32d69 100644 --- a/lib/diameter/src/compiler/diameter_codegen.erl +++ b/lib/diameter/src/compiler/diameter_codegen.erl @@ -21,15 +21,14 @@ -module(diameter_codegen). %% -%% This module generates erl/hrl files for encode/decode modules -%% from the orddict parsed from a dictionary file (.dia) by -%% diameter_dict_util. The generated code is simple (one-liners), -%% the generated functions being called by code included iin the -%% generated modules from diameter_gen.hrl. The orddict itself is -%% returned by dict/0 in the generated module and diameter_dict_util -%% calls this function when importing dictionaries as a consequence -%% of @inherits sections. That is, @inherits introduces a dependency -%% on the beam file of another dictionary. +%% This module generates erl/hrl files for encode/decode modules from +%% the orddict parsed from a dictionary file by diameter_dict_util. +%% The generated code is simple (one-liners), and is called from +%% diameter_gen. The orddict itself is returned by dict/0 in the +%% generated module and diameter_dict_util calls this function when +%% importing dictionaries as a consequence of @inherits sections. That +%% is, @inherits introduces a dependency on the beam file of another +%% dictionary. %% -export([from_dict/4, -- 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') 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') 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') 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') 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') 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') 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') 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') 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') 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 baa65db0f00c84c7248d2401272569f1daf2e2b9 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 17 Jul 2017 11:14:32 +0200 Subject: Don't search forms unnecessarily in diameter_exprecs parse transform The forms being extracted are in the head of the split. --- lib/diameter/src/compiler/diameter_exprecs.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/diameter/src') diff --git a/lib/diameter/src/compiler/diameter_exprecs.erl b/lib/diameter/src/compiler/diameter_exprecs.erl index 9a0cb6baf2..143dede037 100644 --- a/lib/diameter/src/compiler/diameter_exprecs.erl +++ b/lib/diameter/src/compiler/diameter_exprecs.erl @@ -110,9 +110,9 @@ %% parse_transform/2 parse_transform(Forms, _Options) -> - Rs = [R || {attribute, _, record, R} <- Forms], - Es = lists:append([E || {attribute, _, export_records, E} <- Forms]), {H,T} = lists:splitwith(fun is_head/1, Forms), + Rs = [R || {attribute, _, record, R} <- H], + Es = lists:append([E || {attribute, _, export_records, E} <- H]), H ++ [a_export(Es) | f_accessors(Es, Rs)] ++ T. is_head(T) -> -- cgit v1.2.3