aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnders Svensson <[email protected]>2014-09-09 18:32:34 +0200
committerAnders Svensson <[email protected]>2014-09-09 18:32:34 +0200
commit681f390324315266e9480b3471c1c4aeda004b5d (patch)
treea956fc749918c1d996b377dd0c4bc2ab33e83cf3
parente688805e00faa6b547fa99f9b979782ce799852c (diff)
parent0f9cdbaf4d7fde93d319be7789dd4119092d91c6 (diff)
downloadotp-681f390324315266e9480b3471c1c4aeda004b5d.tar.gz
otp-681f390324315266e9480b3471c1c4aeda004b5d.tar.bz2
otp-681f390324315266e9480b3471c1c4aeda004b5d.zip
Merge branch 'anders/diameter/Failed-AVP/OTP-12094' into maint
* anders/diameter/Failed-AVP/OTP-12094: Fix best effort decode of Failed-AVP Fix decode of Failed-AVP in RFC 3588 answer-message
-rw-r--r--lib/diameter/include/diameter_gen.hrl71
-rw-r--r--lib/diameter/src/base/diameter_codec.erl30
-rw-r--r--lib/diameter/src/base/diameter_traffic.erl4
3 files changed, 80 insertions, 25 deletions
diff --git a/lib/diameter/include/diameter_gen.hrl b/lib/diameter/include/diameter_gen.hrl
index 7e91ce375f..233eb8dd15 100644
--- a/lib/diameter/include/diameter_gen.hrl
+++ b/lib/diameter/include/diameter_gen.hrl
@@ -311,7 +311,9 @@ d(Name, Avp, Acc) ->
Failed = relax(Name), %% Not AvpName or else a failed Failed-AVP
%% decode is packed into 'AVP'.
- try avp(decode, Data, AvpName) of
+ Mod = dict(Failed), %% Dictionary to decode in.
+
+ try Mod:avp(decode, Data, AvpName) of
V ->
{Avps, T} = Acc,
{H, A} = ungroup(V, Avp),
@@ -324,6 +326,25 @@ d(Name, Avp, Acc) ->
reset(?FAILED_KEY, Failed)
end.
+%% dict/1
+%%
+%% Retrieve the dictionary for the best-effort decode of Failed-AVP,
+%% as put by diameter_codec:decode/2. See that function for the
+%% explanation.
+
+dict(true) ->
+ case get({diameter_codec, dictionary}) of
+ undefined ->
+ ?MODULE;
+ Mod ->
+ Mod
+ end;
+
+dict(_) ->
+ ?MODULE.
+
+%% d/5
+
%% Ignore a decode error within Failed-AVP ...
d(true, _, Name, Avp, Acc) ->
decode_AVP(Name, Avp, Acc);
@@ -341,6 +362,8 @@ d(false, Reason, Name, Avp, {Avps, Acc}) ->
{Rec, Failed} = Acc,
{[Avp|Avps], {Rec, [rc(Reason, Avp) | Failed]}}.
+%% relax/2
+
%% Set false in the process dictionary as soon as we see a Grouped AVP
%% that doesn't set the M-bit, so that is_strict() can say whether or
%% not to ignore the M-bit on an encapsulated AVP.
@@ -357,22 +380,23 @@ relax(_, _) ->
is_strict() ->
false /= getr(?STRICT_KEY).
+%% relax/1
+%%
%% Set true in the process dictionary as soon as we see Failed-AVP.
%% Matching on 'Failed-AVP' assumes that this is the RFC AVP.
%% Strictly, this doesn't need to be the case.
+
relax('Failed-AVP') ->
- case getr(?FAILED_KEY) of
- undefined ->
- putr(?FAILED_KEY, true);
- true = Yes ->
- Yes
- end;
+ is_failed() orelse putr(?FAILED_KEY, true);
+
relax(_) ->
is_failed().
is_failed() ->
true == getr(?FAILED_KEY).
+%% reset/2
+
reset(Key, undefined) ->
eraser(Key);
reset(_, _) ->
@@ -453,8 +477,8 @@ pack_AVP(_, #diameter_avp{data = <<0:1, Data/binary>>} = Avp, Acc) ->
{Rec, Failed} = Acc,
{Rec, [{5014, Avp#diameter_avp{data = Data}} | Failed]};
-pack_AVP(Name, #diameter_avp{is_mandatory = M} = Avp, Acc) ->
- case pack_arity(Name, M) of
+pack_AVP(Name, #diameter_avp{is_mandatory = M, name = AvpName} = Avp, Acc) ->
+ case pack_arity(Name, AvpName, M) of
0 ->
{Rec, Failed} = Acc,
{Rec, [{if M -> 5001; true -> 5008 end, Avp} | Failed]};
@@ -462,10 +486,13 @@ pack_AVP(Name, #diameter_avp{is_mandatory = M} = Avp, Acc) ->
pack(Arity, 'AVP', Avp, Acc)
end.
-%% Give Failed-AVP special treatment since it'll contain any
-%% unrecognized mandatory AVP's.
-pack_arity(Name, M) ->
- NF = Name /= 'Failed-AVP' andalso not is_failed(),
+%% 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, AvpName, M) ->
+ IsFailed = Name == 'Failed-AVP' orelse is_failed(),
+
%% Not testing just Name /= 'Failed-AVP' means we're changing the
%% packing of AVPs nested within Failed-AVP, but the point of
%% ignoring errors within Failed-AVP is to decode as much as
@@ -473,12 +500,18 @@ pack_arity(Name, M) ->
%% packed into a dedicated field defeats that point. Note that we
%% can't just test not is_failed() since this will be 'true' when
%% packing an unknown AVP directly within Failed-AVP.
- case NF andalso M andalso is_strict() of
- true ->
- 0;
- false ->
- avp_arity(Name, 'AVP')
- end.
+
+ pack_arity(IsFailed
+ orelse {Name, AvpName} == {'answer-message', 'Failed-AVP'}
+ orelse not M
+ orelse not is_strict(),
+ Name).
+
+pack_arity(true, Name) ->
+ avp_arity(Name, 'AVP');
+
+pack_arity(false, _) ->
+ 0.
%% 3588:
%%
diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl
index 06a4f5de64..a2b04bfd63 100644
--- a/lib/diameter/src/base/diameter_codec.erl
+++ b/lib/diameter/src/base/diameter_codec.erl
@@ -237,15 +237,35 @@ rec2msg(Mod, Rec) ->
%% Unsuccessfully decoded AVPs will be placed in #diameter_packet.errors.
--spec decode(module(), #diameter_packet{} | binary())
+-spec decode(module() | {module(), module()}, #diameter_packet{} | binary())
-> #diameter_packet{}.
+%% An Answer setting the E-bit. The application dictionary is needed
+%% for the best-effort decode of Failed-AVP, and the best way to make
+%% this available to the AVP decode in diameter_gen.hrl, without
+%% having to rewrite the entire codec generation, is to place it in
+%% the process dictionary. It's the code in diameter_gen.hrl (that's
+%% included by every generated codec module) that looks for the entry.
+%% Not ideal, but it solves the problem relatively simply.
+decode({Mod, Mod}, Pkt) ->
+ decode(Mod, Pkt);
+decode({Mod, AppMod}, Pkt) ->
+ Key = {?MODULE, dictionary},
+ put(Key, AppMod),
+ try
+ decode(Mod, Pkt)
+ after
+ erase(Key)
+ end;
+
+%% Or not: a request, or an answer not setting the E-bit.
decode(Mod, Pkt) ->
decode(Mod:id(), Mod, Pkt).
-%% If we're a relay application then just extract the avp's without
-%% any decoding of their data since we don't know the application in
-%% question.
+%% decode/3
+
+%% Relay application: just extract the avp's without any decoding of
+%% their data since we don't know the application in question.
decode(?APP_ID_RELAY, _, #diameter_packet{} = Pkt) ->
case collect_avps(Pkt) of
{E, As} ->
@@ -274,6 +294,8 @@ decode(Id, Mod, Bin)
when is_binary(Bin) ->
decode(Id, Mod, #diameter_packet{header = decode_header(Bin), bin = Bin}).
+%% decode_avps/4
+
decode_avps(MsgName, Mod, Pkt, {E, Avps}) ->
?LOG(invalid_avp_length, Pkt#diameter_packet.header),
#diameter_packet{errors = Failed}
diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl
index c4875c5975..280d09d7e8 100644
--- a/lib/diameter/src/base/diameter_traffic.erl
+++ b/lib/diameter/src/base/diameter_traffic.erl
@@ -476,7 +476,7 @@ send_A({Caps, Pkt}, TPid, Dict0, _RecvData) -> %% unsupported application
#diameter_packet{errors = [RC|_]} = Pkt,
send_A(answer_message(RC, Caps, Dict0, Pkt),
TPid,
- Dict0,
+ {Dict0, Dict0},
Pkt,
[],
[]);
@@ -1457,7 +1457,7 @@ handle_answer(SvcName,
= App,
{answer, Req, Dict0, Pkt}) ->
Dict = dict(AppDict, Dict0, Pkt),
- handle_A(errors(Id, diameter_codec:decode(Dict, Pkt)),
+ handle_A(errors(Id, diameter_codec:decode({Dict, AppDict}, Pkt)),
SvcName,
Dict,
Dict0,