aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnders Svensson <[email protected]>2017-07-13 01:09:57 +0200
committerAnders Svensson <[email protected]>2017-08-03 17:16:34 +0200
commit66bb5251e89487c5fb8c1f10b8ceb2c6c8f31eed (patch)
treef491e3ebbb78135006ccacad3ccaff13719206aa
parentdf5814ace0461c37389d96e87ef8aae297802b2e (diff)
downloadotp-66bb5251e89487c5fb8c1f10b8ceb2c6c8f31eed.tar.gz
otp-66bb5251e89487c5fb8c1f10b8ceb2c6c8f31eed.tar.bz2
otp-66bb5251e89487c5fb8c1f10b8ceb2c6c8f31eed.zip
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).
-rw-r--r--lib/diameter/doc/src/diameter.xml31
-rw-r--r--lib/diameter/src/base/diameter.erl7
-rw-r--r--lib/diameter/src/base/diameter_config.erl8
-rw-r--r--lib/diameter/src/base/diameter_gen.erl125
-rw-r--r--lib/diameter/src/base/diameter_peer_fsm.erl1
-rw-r--r--lib/diameter/src/base/diameter_service.erl3
-rw-r--r--lib/diameter/src/base/diameter_traffic.erl2
-rw-r--r--lib/diameter/src/base/diameter_watchdog.erl9
8 files changed, 145 insertions, 41 deletions
diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml
index 43cb3a538c..017f6bb01d 100644
--- a/lib/diameter/doc/src/diameter.xml
+++ b/lib/diameter/doc/src/diameter.xml
@@ -956,6 +956,37 @@ Defaults to the empty list.</p>
</item>
<tag>
+<marker id="strict_arities"/><c>{strict_arities, boolean()
+ | encode
+ | decode}</c></tag>
+<item>
+<p>
+Whether or not to require that the number of AVPs in a message or
+grouped AVP agree with those specified in the dictionary in question.
+If <c>true</c> then mismatches in an outgoing messages cause message
+encoding to fail, while mismatches in an incoming message are reported
+as 5005/5009 errors in the errors field of the diameter_packet record
+passed to &app_handle_request; or &app_handle_answer; callbacks.
+If <c>false</c> then neither error is enforced/detected.
+If <c>encode</c> or <c>decode</c> then errors are only
+enforced/detected on outgoing or incoming messages respectively.</p>
+
+<p>
+Defaults to <c>true</c>.</p>
+
+<note>
+<p>
+Disabling arity checks affects the form of messages at encode/decode.
+In particular, decoded AVPs are represented as lists of values,
+regardless of the AVP's arity (ie. expected number in the message/AVP
+grammar in question), and values are expected to be supplied as lists
+at encode.
+This differs from the historic decode behaviour of representing AVPs
+of arity 1 as bare values, not wrapped in a list.</p>
+</note>
+</item>
+
+<tag>
<marker id="strict_mbit"/><c>{strict_mbit, boolean()}</c></tag>
<item>
<p>
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