From a1df50b38826370b84ec557de401bc1b09d56de7 Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Sat, 2 May 2015 10:14:58 +0200
Subject: Don't confuse Result-Code and Experimental-Result

Decode of an answer message not setting the E-bit, and containing
Experiment-Result but not Result-Code, identified Result-Code as the
erroneous when Erroneous-Result-Code was 3xxx. Here's an example (from
trace) of a the errors field after decode:

  [{5004,
    {diameter_avp,undefined,undefined,false,false,undefined,'Result-Code',
     3001,undefined,undefined}}],

The diameter_avp was just constructed from the AVP name and decoded
result, without regard for which result code AVP contained the value.
Fix by extracting the AVP from the incoming message.
---
 lib/diameter/test/diameter_traffic_SUITE.erl | 60 +++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 2 deletions(-)

(limited to 'lib/diameter/test/diameter_traffic_SUITE.erl')

diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl
index 7dd9f39f85..9e1d5e1435 100644
--- a/lib/diameter/test/diameter_traffic_SUITE.erl
+++ b/lib/diameter/test/diameter_traffic_SUITE.erl
@@ -41,6 +41,7 @@
          send_eval/1,
          send_bad_answer/1,
          send_protocol_error/1,
+         send_experimental_result/1,
          send_arbitrary/1,
          send_unknown/1,
          send_unknown_short/1,
@@ -301,6 +302,7 @@ tc() ->
      send_eval,
      send_bad_answer,
      send_protocol_error,
+     send_experimental_result,
      send_arbitrary,
      send_unknown,
      send_unknown_short,
@@ -480,6 +482,14 @@ send_protocol_error(Config) ->
     ?answer_message(?TOO_BUSY)
         = call(Config, Req).
 
+%% Send a 3xxx Experimental-Result in an answer not setting the E-bit
+%% and missing a Result-Code.
+send_experimental_result(Config) ->
+    Req = ['ACR', {'Accounting-Record-Type', ?EVENT_RECORD},
+                  {'Accounting-Record-Number', 5}],
+    ['ACA', _SessionId | _]
+        = call(Config, Req).
+
 %% Send an ASR with an arbitrary non-mandatory AVP and expect success
 %% and the same AVP in the reply.
 send_arbitrary(Config) ->
@@ -1144,6 +1154,13 @@ answer(Pkt, Req, _Peer, Name, #group{client_dict0 = Dict0}) ->
     [R | Vs] = Dict:'#get-'(answer(Ans, Es, Name)),
     [Dict:rec2msg(R) | Vs].
 
+%% Missing Result-Codec and inapproriate Experimental-Result-Code.
+answer(Rec, Es, send_experimental_result) ->
+    [{5004, #diameter_avp{name = 'Experimental-Result'}},
+     {5005, #diameter_avp{name = 'Result-Code'}}]
+        =  Es,
+    Rec;
+
 %% An inappropriate E-bit results in a decode error ...
 answer(Rec, Es, send_bad_answer) ->
     [{5004, #diameter_avp{name = 'Result-Code'}} | _] = Es,
@@ -1175,7 +1192,9 @@ handle_error(Reason, _Req, [$C|_], _Peer, _, _Time) ->
 %% Note that diameter will set Result-Code and Failed-AVPs if
 %% #diameter_packet.errors is non-null.
 
-handle_request(#diameter_packet{header = H, msg = M}, _, {_Ref, Caps}) ->
+handle_request(#diameter_packet{header = H, msg = M, avps = As},
+               _,
+               {_Ref, Caps}) ->
     #diameter_header{end_to_end_id = EI,
                      hop_by_hop_id = HI}
         = H,
@@ -1183,10 +1202,12 @@ handle_request(#diameter_packet{header = H, msg = M}, _, {_Ref, Caps}) ->
     V = EI bsr B,  %% assert
     V = HI bsr B,  %%
     #diameter_caps{origin_state_id = {_,[Id]}} = Caps,
-    answer(origin(Id), request(M, Caps)).
+    answer(origin(Id), request(M, [H|As], Caps)).
 
 answer(T, {Tag, Action, Post}) ->
     {Tag, answer(T, Action), Post};
+answer(_, {reply, [#diameter_header{} | _]} = T) ->
+    T;
 answer({A,C}, {reply, Ans}) ->
     answer(C, {reply, msg(Ans, A, diameter_gen_base_rfc3588)});
 answer(pkt, {reply, Ans})
@@ -1195,6 +1216,41 @@ answer(pkt, {reply, Ans})
 answer(_, T) ->
     T.
 
+%% request/3
+
+%% send_experimental_result
+request(#diameter_base_accounting_ACR{'Accounting-Record-Number' = 5},
+        [Hdr | Avps],
+        #diameter_caps{origin_host = {OH, _},
+                       origin_realm = {OR, _}}) ->
+    [H,R|T] = [A || N <- ['Origin-Host',
+                          'Origin-Realm',
+                          'Session-Id',
+                          'Accounting-Record-Type',
+                          'Accounting-Record-Number'],
+                    #diameter_avp{} = A
+                        <- [lists:keyfind(N, #diameter_avp.name, Avps)]],
+    Ans = [Hdr#diameter_header{is_request = false},
+           H#diameter_avp{data = OH},
+           R#diameter_avp{data = OR},
+           #diameter_avp{name = 'Experimental-Result',
+                         code = 297,
+                         need_encryption = false,
+                         data = [#diameter_avp{data = {?DIAMETER_DICT_COMMON,
+                                                       'Vendor-Id',
+                                                       123}},
+                                 #diameter_avp{data
+                                               = {?DIAMETER_DICT_COMMON,
+                                                  'Experimental-Result-Code',
+                                                  3987}}]}
+           | T],
+    {reply, Ans};
+
+request(Msg, _Avps, Caps) ->
+    request(Msg, Caps).
+
+%% request/2
+
 %% send_nok
 request(#diameter_base_accounting_ACR{'Accounting-Record-Number' = 0},
         _) ->
-- 
cgit v1.2.3