From 3cba7ec073e449db99173ad07a5652a2026f0aa5 Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Wed, 30 Aug 2017 16:12:10 +0200
Subject: Fix handling of Proxy-Info in answers formulated by diameter

RFC 6733 says this:

6.2.  Diameter Answer Processing

   When a request is locally processed, the following procedures MUST be
   applied to create the associated answer, in addition to any
   additional procedures that MAY be discussed in the Diameter
   application defining the command:

   ...

   o  Any Proxy-Info AVPs in the request MUST be added to the answer
      message, in the same order they were present in the request.

This wasn't done when a handle_request callback returned a Result-Code
in an 'answer-message' or protocol_error tuple, causing diameter itself
to construct the answer message. This form of answer is just a
convenience, since the callback can always return an answer that it
constructs itself.
---
 lib/diameter/src/base/diameter_traffic.erl   | 20 ++++++++++++++------
 lib/diameter/test/diameter_traffic_SUITE.erl | 23 ++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 7 deletions(-)

(limited to 'lib')

diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl
index 4388175c3a..5ee598f513 100644
--- a/lib/diameter/src/base/diameter_traffic.erl
+++ b/lib/diameter/src/base/diameter_traffic.erl
@@ -1031,15 +1031,15 @@ answer_message(RC,
                               origin_realm = {OR,_}},
                #diameter_packet{avps = Avps,
                                 errors = Es}) ->
-    {Code, _, Vid} = Dict0:avp_header('Session-Id'),
     ['answer-message', {'Origin-Host', OH},
                        {'Origin-Realm', OR},
-                       {'Result-Code', RC}]
-        ++ session_id(Code, Vid, Avps)
-        ++ failed_avp(RC, Es).
+                       {'Result-Code', RC}
+     | session_id(Dict0, Avps)
+       ++ failed_avp(RC, Es)
+       ++ proxy_info(Dict0, Avps)].
 
-session_id(Code, Vid, Avps)
-  when is_list(Avps) ->
+session_id(Dict0, Avps) ->
+    {Code, _, Vid} = Dict0:avp_header('Session-Id'),
     try
         #diameter_avp{data = Bin} = find_avp(Code, Vid, Avps),
         [{'Session-Id', [Bin]}]
@@ -1057,6 +1057,14 @@ failed_avp(RC, [_ | Es]) ->
 failed_avp(_, [] = No) ->
     No.
 
+proxy_info(Dict0, Avps) ->
+    {Code, _, Vid} = Dict0:avp_header('Proxy-Info'),
+    [{'AVP', [A#diameter_avp{value = undefined}
+              || [#diameter_avp{code = C, vendor_id = I} = A | _]
+                     <- Avps,
+                 C == Code,
+                 I == Vid]}].
+
 %% find_avp/3
 
 %% Grouped ...
diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl
index d6d69eafa1..2bc00bf4a1 100644
--- a/lib/diameter/test/diameter_traffic_SUITE.erl
+++ b/lib/diameter/test/diameter_traffic_SUITE.erl
@@ -47,6 +47,7 @@
          send_protocol_error/1,
          send_experimental_result/1,
          send_arbitrary/1,
+         send_proxy_info/1,
          send_unknown/1,
          send_unknown_short/1,
          send_unknown_mandatory/1,
@@ -133,6 +134,7 @@
 
 -define(A, list_to_atom).
 -define(L, atom_to_list).
+-define(B, iolist_to_binary).
 
 %% Don't use is_record/2 since dictionary hrl's aren't included.
 %% (Since they define conflicting records with the same names.)
@@ -430,6 +432,7 @@ tc() ->
      send_protocol_error,
      send_experimental_result,
      send_arbitrary,
+     send_proxy_info,
      send_unknown,
      send_unknown_short,
      send_unknown_mandatory,
@@ -672,6 +675,19 @@ send_arbitrary(Config) ->
         = call(Config, Req),
     "XXX" = string(V, Config).
 
+%% Send Proxy-Info in an ASR that the peer answers with 3xxx, and
+%% ensure that the AVP is returned.
+send_proxy_info(Config) ->
+    H0 = ?B(?util:unique_string()),
+    S0 = ?B(?util:unique_string()),
+    Req = ['ASR', {'Proxy-Info', #{'Proxy-Host'  => H0,
+                                   'Proxy-State' => S0}}],
+    ['answer-message' | #{'Result-Code' := 3999,
+                          'Proxy-Info' := [Rec]}]
+        = call(Config, Req),
+    {H, S, []} = proxy_info(Rec, Config),
+    [H0, S0] = [?B(X) || X <- [H,S]].
+
 %% Send an unknown AVP (to some client) and check that it comes back.
 send_unknown(Config) ->
     Req = ['ASR', {'AVP', [#diameter_avp{code = 999,
@@ -764,7 +780,7 @@ send_grouped_error(Config) ->
     [[#diameter_avp{name = 'Proxy-Info', value = V}]]
         = failed_avps(Avps, Config),
     {Empty, undefined, []} = proxy_info(V, Config),
-    <<0>> = iolist_to_binary(Empty).
+    <<0>> = ?B(Empty).
 
 %% Send an STR that the server ignores.
 send_noreply(Config) ->
@@ -1721,6 +1737,11 @@ request(['ACR' | #{'Accounting-Record-Number' := 4}],
                              {'Origin-Realm', OR}],
     {reply, Ans};
 
+%% send_proxy_info
+request(['ASR' | #{'Proxy-Info' := _}],
+        _) ->
+    {protocol_error, 3999};
+
 request(['ASR' | #{'Session-Id' := SId} = Avps],
         #diameter_caps{origin_host = {OH, _},
                        origin_realm = {OR, _}}) ->
-- 
cgit v1.2.3