From 38a75600cbd5b51ae61921df78cc02348e563564 Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Fri, 3 May 2013 01:38:12 +0200
Subject: Detect all 5005 (MISSING_AVP) errors and don't reverse errors

The previous commit ensures that only one will be reported in an answer
message when diameter itself sets Result-Code/Failed-AVP.

The order of errors in #diameter_packet.errors is that in which they're
detected, not the reverse as previously.
---
 lib/diameter/include/diameter_gen.hrl              | 63 ++++++++--------------
 .../diameter_test_unknown.erl                      |  4 +-
 2 files changed, 24 insertions(+), 43 deletions(-)

diff --git a/lib/diameter/include/diameter_gen.hrl b/lib/diameter/include/diameter_gen.hrl
index b2f46db74d..2126a5623d 100644
--- a/lib/diameter/include/diameter_gen.hrl
+++ b/lib/diameter/include/diameter_gen.hrl
@@ -150,26 +150,17 @@ pack_AVP(Name, #diameter_avp{name = AvpName,
  when Failed :: [{5000..5999, #diameter_avp{}}].
 
 decode_avps(Name, Recs) ->
-    d_rc(Name, lists:foldl(fun(T,A) -> decode(Name, T, A) end,
-                           {[], {newrec(Name), []}},
-                           Recs)).
+    {Avps, {Rec, Failed}}
+        = 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.
 
 newrec(Name) ->
     '#new-'(name2rec(Name)).
 
-%% No errors so far: keep looking.
-d_rc(Name, {Avps, {Rec, [] = Failed}}) ->
-    try
-        true = have_required_avps(Rec, Name),
-        {Rec, Avps, Failed}
-    catch
-        throw: {?MODULE, {AvpName, Reason}} ->
-            diameter_lib:log({decode, error},
-                             ?MODULE,
-                             ?LINE,
-                             {AvpName, Reason, Rec}),
-            {Rec, Avps, [{5005, empty_avp(AvpName)}]}
-    end;
 %% 3588:
 %%
 %%   DIAMETER_MISSING_AVP               5005
@@ -180,9 +171,17 @@ d_rc(Name, {Avps, {Rec, [] = Failed}}) ->
 %%      Vendor-Id if applicable.  The value field of the missing AVP
 %%      should be of correct minimum length and contain zeroes.
 
-%% Or not. Only need to report the first error so look no further.
-d_rc(_, {Avps, {Rec, Failed}}) ->
-    {Rec, Avps, lists:reverse(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))]].
+
+%% Maximum arities have already been checked in building the record.
+
+have_arity({Min, _}, L) ->
+    Min =< length(L);
+have_arity(N, V) ->
+    N /= 1 orelse V /= undefined.
 
 %% empty_avp/1
 
@@ -197,25 +196,6 @@ empty_avp(Name) ->
                   data = empty_value(Name),
                   type = Type}.
 
-%% have_required_avps/2
-
-have_required_avps(Rec, Name) ->
-    lists:foreach(fun(F) -> hra(Name, F, Rec) end,
-                  '#info-'(element(1, Rec), fields)),
-    true.
-
-hra(Name, AvpName, Rec) ->
-    Arity = avp_arity(Name, AvpName),
-    hra(Arity, '#get-'(AvpName, Rec))
-        orelse ?THROW({AvpName, {insufficient_arity, Arity}}).
-
-%% Maximum arities have already been checked in building the record.
-
-hra({Min, _}, L) ->
-    Min =< length(L);
-hra(N, V) ->
-    N /= 1 orelse V /= undefined.
-
 %% 3588, ch 7:
 %%
 %%   The Result-Code AVP describes the error that the Diameter node
@@ -403,13 +383,14 @@ value(_, Avp) ->
 -spec grouped_avp(decode, avp_name(), binary())
    -> {avp_record(), [avp()]};
                  (encode, avp_name(), avp_record() | avp_values())
-   -> binary() | no_return().
+   -> binary()
+    | no_return().
 
 grouped_avp(decode, Name, Data) ->
     {Rec, Avps, []} = decode_avps(Name, diameter_codec:collect_avps(Data)),
     {Rec, Avps};
-%% Note that a failed match here will result in 5004. Note that this is
-%% the only AVP type that doesn't just return the decoded value, also
+%% A failed match here will result in 5004. Note that this is the only
+%% AVP type that doesn't just return the decoded record, also
 %% returning the list of component AVP's.
 
 grouped_avp(encode, Name, Data) ->
diff --git a/lib/diameter/test/diameter_codec_SUITE_data/diameter_test_unknown.erl b/lib/diameter/test/diameter_codec_SUITE_data/diameter_test_unknown.erl
index bce3d78a37..49f2158b1a 100644
--- a/lib/diameter/test/diameter_codec_SUITE_data/diameter_test_unknown.erl
+++ b/lib/diameter/test/diameter_codec_SUITE_data/diameter_test_unknown.erl
@@ -71,6 +71,6 @@ dec('AR', #diameter_packet
 dec('BR', #diameter_packet
            {msg = #recv_BR{'Origin-Host'  = ?HOST,
                            'Origin-Realm' = ?REALM},
-            errors = [{5008, ?NOT_MANDATORY_YYY},
-                      {5001, ?MANDATORY_XXX}]}) ->
+            errors = [{5001, ?MANDATORY_XXX},
+                      {5008, ?NOT_MANDATORY_YYY}]}) ->
     ok.
-- 
cgit v1.2.3