From abd6c7494f973eb91c6a582f3370e9896d28992b Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 29 Jun 2015 09:09:13 +0200 Subject: Don't flag AVP as missing as a consequence of decode error The decode of an incoming Diameter message uses the record representation to determine whether or not an AVP has been received with the expected arity, the number of AVPs in each field following decode being compared with the arity specified in the message grammar. The problem with this is that decode failure isn't reflected in the record representation, so that an AVP can be appended to the errors field of a diameter_packet record despite an entry for the same AVP already existing. This isn't a fault as much as a misleading error indication, but now only append AVPs that aren't already represented. --- lib/diameter/include/diameter_gen.hrl | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/diameter/include/diameter_gen.hrl b/lib/diameter/include/diameter_gen.hrl index e8ffe7f92c..0b36389088 100644 --- a/lib/diameter/include/diameter_gen.hrl +++ b/lib/diameter/include/diameter_gen.hrl @@ -185,9 +185,10 @@ decode_avps(Name, Recs) -> = lists:foldl(fun(T,A) -> decode(Name, T, A) end, {[], {newrec(Name), []}}, Recs), - {Rec, Avps, Failed ++ missing(Rec, Name)}. -%% Append 5005 errors so that a 5014 for the same AVP will take -%% precedence in a Result-Code/Failed-AVP setting. + {Rec, Avps, Failed ++ missing(Rec, Name, Failed)}. +%% 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. newrec(Name) -> '#new-'(name2rec(Name)). @@ -202,10 +203,18 @@ newrec(Name) -> %% Vendor-Id if applicable. The value field of the missing AVP %% should be of correct minimum length and contain zeroes. +missing(Rec, Name, Failed) -> + [{5005, A} || #diameter_avp{code = MC, vendor_id = MV} = A + <- missing(Rec, Name), + lists:all(fun({_, #diameter_avp{code = C, vendor_id = V}}) -> + MC /= C orelse MV /= V + end, + Failed)]. + missing(Rec, Name) -> - [{5005, empty_avp(F)} || F <- '#info-'(element(1, Rec), fields), - A <- [avp_arity(Name, F)], - false <- [have_arity(A, '#get-'(F, Rec))]]. + [empty_avp(F) || F <- '#info-'(element(1, Rec), fields), + A <- [avp_arity(Name, F)], + not have_arity(A, '#get-'(F, Rec))]. %% Maximum arities have already been checked in building the record. -- cgit v1.2.3