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') 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') 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') 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