aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnders Svensson <[email protected]>2014-05-23 00:24:41 +0200
committerAnders Svensson <[email protected]>2014-05-26 17:51:54 +0200
commitc2c00fdd4de1b8883e47ec1b5b048659ef033302 (patch)
tree8db5258dc6e9d64b352c52c6760b26117a4af879
parent31a7c0b20dbcfdeee946064e610bd51b95be269c (diff)
downloadotp-c2c00fdd4de1b8883e47ec1b5b048659ef033302.tar.gz
otp-c2c00fdd4de1b8883e47ec1b5b048659ef033302.tar.bz2
otp-c2c00fdd4de1b8883e47ec1b5b048659ef033302.zip
Do best-effort decode of Failed-AVP
Commit 4ce2d3a6 (diameter-1.4.2, OTP-11007) disabled the decode of values in Failed-AVP components since any error caused the decode of Failed-AVP itself to fail. This is less than useful since (1) we should be able to decode it given that we've sent it (modulo mangling on the way to the peer and back), and (2) it's not unheard of to examine Failed-AVP to see what the peer objected to. This commits adds a best-effort decode: decode if possible, otherwise not, using the same abuse of the process dictionary as commit bbdb027c.
-rw-r--r--lib/diameter/include/diameter_gen.hrl102
-rw-r--r--lib/diameter/src/base/diameter_codec.erl3
2 files changed, 69 insertions, 36 deletions
diff --git a/lib/diameter/include/diameter_gen.hrl b/lib/diameter/include/diameter_gen.hrl
index ebc10b8918..7e91ce375f 100644
--- a/lib/diameter/include/diameter_gen.hrl
+++ b/lib/diameter/include/diameter_gen.hrl
@@ -30,6 +30,10 @@
%% error or not. See is_strict/0.
-define(STRICT_KEY, strict).
+%% Key that says whether or not we should do a best-effort decode
+%% within Failed-AVP.
+-define(FAILED_KEY, failed).
+
-type parent_name() :: atom(). %% parent = Message or AVP
-type parent_record() :: tuple(). %%
-type avp_name() :: atom().
@@ -286,15 +290,7 @@ decode(Name, 'AVP', Avp, Acc) ->
%% d/3
-%% Don't try to decode the value of a Failed-AVP component since it
-%% probably won't. Note that matching on 'Failed-AVP' assumes that
-%% this is the RFC AVP, with code 279. Strictly, this doesn't need to
-%% be the case, so we're assuming no one defines another Failed-AVP.
-d('Failed-AVP' = Name, Avp, Acc) ->
- decode_AVP(Name, Avp, Acc);
-
-%% Or try to decode.
-d(Name, Avp, {Avps, Acc}) ->
+d(Name, Avp, Acc) ->
#diameter_avp{name = AvpName,
data = Data,
type = Type,
@@ -307,53 +303,81 @@ d(Name, Avp, {Avps, Acc}) ->
%% value around through the entire decode. The solution here is
%% simple in comparison, both to implement and to understand.
- Reset = relax(Type, M),
+ Strict = relax(Type, M),
+ %% Use the process dictionary again to keep track of whether we're
+ %% decoding within Failed-AVP and should ignore decode errors
+ %% altogether.
+
+ Failed = relax(Name), %% Not AvpName or else a failed Failed-AVP
+ %% decode is packed into 'AVP'.
try avp(decode, Data, AvpName) of
V ->
+ {Avps, T} = Acc,
{H, A} = ungroup(V, Avp),
- {[H | Avps], pack_avp(Name, A, Acc)}
+ {[H | Avps], pack_avp(Name, A, T)}
catch
error: Reason ->
- %% Failures here won't be visible since they're a "normal"
- %% occurrence if the peer sends a faulty AVP that we need to
- %% respond sensibly to. Log the occurence for traceability,
- %% but the peer will also receive info in the resulting
- %% answer-message.
- Stack = diameter_lib:get_stacktrace(),
- diameter_lib:log(decode_error,
- ?MODULE,
- ?LINE,
- {Reason, AvpName, Stack}),
-
- {Rec, Failed} = Acc,
- {[Avp|Avps], {Rec, [rc(Reason, Avp) | Failed]}}
+ d(undefined == Failed orelse is_failed(), Reason, Name, Avp, Acc)
after
- relax(Reset)
+ reset(?STRICT_KEY, Strict),
+ reset(?FAILED_KEY, Failed)
end.
+%% Ignore a decode error within Failed-AVP ...
+d(true, _, Name, Avp, Acc) ->
+ decode_AVP(Name, Avp, Acc);
+
+%% ... or not. Failures here won't be visible since they're a "normal"
+%% occurrence if the peer sends a faulty AVP that we need to respond
+%% sensibly to. Log the occurence for traceability, but the peer will
+%% also receive info in the resulting answer message.
+d(false, Reason, Name, Avp, {Avps, Acc}) ->
+ Stack = diameter_lib:get_stacktrace(),
+ diameter_lib:log(decode_error,
+ ?MODULE,
+ ?LINE,
+ {Reason, Name, Avp#diameter_avp.name, Stack}),
+ {Rec, Failed} = Acc,
+ {[Avp|Avps], {Rec, [rc(Reason, Avp) | Failed]}}.
+
%% 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.
relax('Grouped', M) ->
- V = getr(?STRICT_KEY),
- if V == undefined andalso not M ->
+ case getr(?STRICT_KEY) of
+ undefined when not M ->
putr(?STRICT_KEY, M);
- true ->
+ _ ->
false
end;
relax(_, _) ->
false.
-%% Reset strictness.
-relax(undefined) ->
- eraser(?STRICT_KEY);
-relax(false) ->
- ok.
-
is_strict() ->
false /= getr(?STRICT_KEY).
+%% 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;
+relax(_) ->
+ is_failed().
+
+is_failed() ->
+ true == getr(?FAILED_KEY).
+
+reset(Key, undefined) ->
+ eraser(Key);
+reset(_, _) ->
+ ok.
+
%% decode_AVP/3
%%
%% Don't know this AVP: see if it can be packed in an 'AVP' field
@@ -441,7 +465,15 @@ pack_AVP(Name, #diameter_avp{is_mandatory = M} = Avp, Acc) ->
%% Give Failed-AVP special treatment since it'll contain any
%% unrecognized mandatory AVP's.
pack_arity(Name, M) ->
- case Name /= 'Failed-AVP' andalso M andalso is_strict() of
+ NF = Name /= 'Failed-AVP' andalso not 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
+ %% possible, and failing because a mandatory AVP couldn't be
+ %% 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 ->
diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl
index 0ca4a84d21..06a4f5de64 100644
--- a/lib/diameter/src/base/diameter_codec.erl
+++ b/lib/diameter/src/base/diameter_codec.erl
@@ -593,7 +593,8 @@ pack_avp(#diameter_avp{code = undefined, data = B})
Len = size(<<H:5/binary, _:24, T/binary>> = <<B/binary, 0:Pad>>),
<<H/binary, Len:24, T/binary>>;
-%% ... from a dictionary compiled against old code (diameter_gen) ...
+%% ... from a dictionary compiled against old code in diameter_gen ...
+%% ... when ignoring errors in Failed-AVP ...
pack_avp(#diameter_avp{data = <<0:1, B/binary>>} = A) ->
pack_avp(A#diameter_avp{data = B});