From 0447bd6e8bcd3b0249b9956883c213c434095ec5 Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Mon, 28 Aug 2017 23:53:40 +0200
Subject: Use unordered delivery on a lone outbound stream in diameter_sctp

RFC 6733 say the below about head-of-line blocking and SCTP, the
suggestion of unordered sending being new compared to the RFC 3588.
Until now, all delivery in diameter_sctp has been ordered, and
roundrobin over available streams unless the user passes a stream
identifier in an outgoing diameter_packet record. This commit changes
this to use unordered delivery when there's only a single outbound
stream to choose from.

The special case at capabilities exchange is handled by only starting to
send unordered after the second message from the peer has been received.
(First message after capabilities exchange, as the RFC probably means.)
The special case at DPR isn't handled, since there's no knowing the
order in which a peer will answer: a node that sends DPR while it has
other requests outstanding can't expect that the latter will be answered
before DPR, even if delivery is ordered since incoming requests are
handled concurrently. If it wants a guarantee then it simply has to wait
for answers before sending DPR.

A user can force all delivery to be unordered by specifying

  {sctp_default_send_params, #sctp_sndrcvinfo{flags = [unordered]}}

as a config option to diameter_sctp, but in this case there's no
handling of a request being sent directly after CEA since there's no
ordered flag to override the default.

RFC 6733:

2.1.1.  SCTP Guidelines

   Diameter messages SHOULD be mapped into SCTP streams in a way that
   avoids head-of-the-line (HOL) blocking.  Among different ways of
   performing the mapping that fulfill this requirement it is
   RECOMMENDED that a Diameter node send every Diameter message (request
   or response) over stream zero with the unordered flag set.  However,
   Diameter nodes MAY select and implement other design alternatives for
   avoiding HOL blocking such as using multiple streams with the
   unordered flag cleared (as originally instructed in RFC 3588).  On
   the receiving side, a Diameter entity MUST be ready to receive
   Diameter messages over any stream, and it is free to return responses
   over a different stream.  This way, both sides manage the available
   streams in the sending direction, independently of the streams chosen
   by the other side to send a particular Diameter message.  These
   messages can be out-of-order and belong to different Diameter
   sessions.

   Out-of-order delivery has special concerns during a connection
   establishment and termination.  When a connection is established, the
   responder side sends a CEA message and moves to R-Open state as
   specified in Section 5.6.  If an application message is sent shortly
   after the CEA and delivered out-of-order, the initiator side, still
   in Wait-I-CEA state, will discard the application message and close
   the connection.  In order to avoid this race condition, the receiver
   side SHOULD NOT use out-of-order delivery methods until the first
   message has been received from the initiator, proving that it has
   moved to I-Open state.  To trigger such a message, the receiver side
   could send a DWR immediately after sending a CEA.  Upon reception of
   the corresponding DWA, the receiver side should start using out-of-
   order delivery methods to counter the HOL blocking.

   Another race condition may occur when DPR and DPA messages are used.
   Both DPR and DPA are small in size; thus, they may be delivered to
   the peer faster than application messages when an out-of-order
   delivery mechanism is used.  Therefore, it is possible that a DPR/DPA
   exchange completes while application messages are still in transit,
   resulting in a loss of these messages.  An implementation could
   mitigate this race condition, for example, using timers, and wait for
   a short period of time for pending application level messages to
   arrive before proceeding to disconnect the transport connection.
   Eventually, lost messages are handled by the retransmission mechanism
   described in Section 5.5.4.
---
 lib/diameter/src/transport/diameter_sctp.erl | 30 ++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/lib/diameter/src/transport/diameter_sctp.erl b/lib/diameter/src/transport/diameter_sctp.erl
index 4f56475529..0d8fb37629 100644
--- a/lib/diameter/src/transport/diameter_sctp.erl
+++ b/lib/diameter/src/transport/diameter_sctp.erl
@@ -105,6 +105,7 @@
          packet = true :: boolean()        %% legacy transport_data?
                         | raw,
          message_cb = false :: false | diameter:eval(),
+         unordered = false :: boolean() | 0 | 1,   %% send unordered?
          send = false :: pid() | boolean()}).      %% sending process
 
 %% Monitor process state.
@@ -677,7 +678,11 @@ send(#diameter_packet{transport_data = {outstream, SId}}
      = S) ->
     send(SId rem OS, Msg, S);
 
-%% ... or not: rotate through all streams.
+%% ... or not: send unordered on a lone stream ...
+send(Msg, #transport{unordered = true} = S) ->
+    send(0, Msg, S);
+
+%% ... or rotate through all.
 send(Msg, #transport{streams = {_, OS},
                      os = N}
           = S) ->
@@ -725,6 +730,7 @@ recv({_, #sctp_assoc_change{state = comm_up,
     %% Deal with different association id after peeloff on Solaris by
     %% taking the id from the first reception.
     up(S#transport{assoc_id = T == accept orelse Id,
+                   unordered = 1 == OS andalso 1,
                    streams = {IS, OS}});
 
 %% ... or not: try the next address.
@@ -749,7 +755,7 @@ recv({[#sctp_sndrcvinfo{assoc_id = Id}], _Bin}
 %% Inbound Diameter message.
 recv({[#sctp_sndrcvinfo{}], Bin} = Msg, S)
   when is_binary(Bin) ->
-    message(recv, Msg, S);
+    message(recv, Msg, recv(S));
 
 recv({_, #sctp_shutdown_event{}}, _) ->
     stop;
@@ -769,6 +775,26 @@ recv({_, #sctp_paddr_change{}}, _) ->
 recv({_, #sctp_pdapi_event{}}, _) ->
     ok.
 
+%% recv/1
+%%
+%% Start sending unordered on a lone outbound stream after the second
+%% reception, so that an outgoing CER/CEA will arrive at the peer
+%% before another request.
+
+recv(#transport{unordered = B} = S)
+  when is_boolean(B) ->
+    S;
+
+recv(#transport{unordered = 0, socket = Sock} = S) ->
+    ok = inet:setopts(Sock, [{sctp_default_send_param,
+                              #sctp_sndrcvinfo{flags = [unordered]}}]),
+    S#transport{unordered = true};
+
+recv(#transport{unordered = N} = S) ->
+    S#transport{unordered = N-1}.
+
+%% publish/4
+
 publish(T, Ref, Id, Sock) ->
     true = diameter_reg:add_new({?MODULE, T, {Ref, {Id, Sock}}}),
     putr(?INFO_KEY, {gen_sctp, Sock}).  %% for info/1
-- 
cgit v1.2.3


From 549a82df3ae250b7c5598a9451b7b9802073d6f9 Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Tue, 29 Aug 2017 09:08:38 +0200
Subject: Exercise unordered delivery in traffic suite

By randomly setting the number of outbound streams on associations.
---
 lib/diameter/test/diameter_traffic_SUITE.erl | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl
index 1760d7c5dc..f058ed65b8 100644
--- a/lib/diameter/test/diameter_traffic_SUITE.erl
+++ b/lib/diameter/test/diameter_traffic_SUITE.erl
@@ -112,6 +112,8 @@
 %% diameter_{tcp,sctp} callbacks
 -export([message/3]).
 
+-include_lib("kernel/include/inet_sctp.hrl").
+
 -include("diameter.hrl").
 -include("diameter_gen_base_rfc3588.hrl").
 -include("diameter_gen_base_accounting.hrl").
@@ -486,7 +488,7 @@ add_transports(Config) ->
                          | server_apps()]
                         ++ [{spawn_opt, {erlang, spawn, []}} || CS]),
     Cs = [?util:connect(CN,
-                        [T, {sender, CS}],
+                        [T, {sender, CS} | client_opts(T)],
                         LRef,
                         [{id, Id}
                          | client_apps(R, [{'Origin-State-Id', origin(Id)}])])
@@ -496,6 +498,14 @@ add_transports(Config) ->
              Id <- [{D,E}]],
     ?util:write_priv(Config, "transport", [LRef | Cs]).
 
+client_opts(tcp) ->
+    [];
+client_opts(sctp) ->
+    [{sctp_initmsg, #sctp_initmsg{num_ostreams = N,
+                                  max_instreams = 5}}
+     || N <- [rand:uniform(8)],
+        N =< 6].
+
 server_apps() ->
     B = have_nas(),
     [{applications, [diameter_gen_base_rfc3588,
-- 
cgit v1.2.3


From 1fd9b919608b54e4d08340c45fe1cabb6975880c Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Tue, 29 Aug 2017 14:33:47 +0200
Subject: Delay rotation of diameter_sctp outbound streams

For the same reason as unordered delivery is delayed in the grandparent
commit. Delivery is ordered only within a stream, so until a second
message is received from the peer, there's no guarantee that a second
outgoing request won't be received before the initial capablities
exchange message. Rotation begins upon reception of a second message
from the peer, messages being sent on stream 0 until then.
---
 lib/diameter/src/transport/diameter_sctp.erl | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/lib/diameter/src/transport/diameter_sctp.erl b/lib/diameter/src/transport/diameter_sctp.erl
index 0d8fb37629..86a98776c5 100644
--- a/lib/diameter/src/transport/diameter_sctp.erl
+++ b/lib/diameter/src/transport/diameter_sctp.erl
@@ -105,7 +105,7 @@
          packet = true :: boolean()        %% legacy transport_data?
                         | raw,
          message_cb = false :: false | diameter:eval(),
-         unordered = false :: boolean() | 0 | 1,   %% send unordered?
+         unordered = 1 :: boolean() | 0 | 1,       %% send unordered?
          send = false :: pid() | boolean()}).      %% sending process
 
 %% Monitor process state.
@@ -678,15 +678,16 @@ send(#diameter_packet{transport_data = {outstream, SId}}
      = S) ->
     send(SId rem OS, Msg, S);
 
-%% ... or not: send unordered on a lone stream ...
-send(Msg, #transport{unordered = true} = S) ->
-    send(0, Msg, S);
-
-%% ... or rotate through all.
-send(Msg, #transport{streams = {_, OS},
+%% ... or not: rotate when sending ordered ...
+send(Msg, #transport{unordered = false,
+                     streams = {_, OS},
                      os = N}
           = S) ->
-    send(N, Msg, S#transport{os = (N + 1) rem OS}).
+    send(N, Msg, S#transport{os = (N + 1) rem OS});
+
+%% ... or send only on the first stream, possibly unordered.
+send(Msg, S) ->
+    send(0, Msg, S).
 
 %% send/3
 
@@ -730,7 +731,6 @@ recv({_, #sctp_assoc_change{state = comm_up,
     %% Deal with different association id after peeloff on Solaris by
     %% taking the id from the first reception.
     up(S#transport{assoc_id = T == accept orelse Id,
-                   unordered = 1 == OS andalso 1,
                    streams = {IS, OS}});
 
 %% ... or not: try the next address.
@@ -785,11 +785,14 @@ recv(#transport{unordered = B} = S)
   when is_boolean(B) ->
     S;
 
-recv(#transport{unordered = 0, socket = Sock} = S) ->
+recv(#transport{unordered = 0, streams = {_, 1}, socket = Sock} = S) ->
     ok = inet:setopts(Sock, [{sctp_default_send_param,
                               #sctp_sndrcvinfo{flags = [unordered]}}]),
     S#transport{unordered = true};
 
+recv(#transport{unordered = 0} = S) ->
+    S#transport{unordered = false};
+
 recv(#transport{unordered = N} = S) ->
     S#transport{unordered = N-1}.
 
-- 
cgit v1.2.3


From a3749fd240260958053f90539b0f7e04e720d070 Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Tue, 29 Aug 2017 22:26:51 +0200
Subject: Send unordered on all outbound diameter_sctp streams

There's no reason for sending ordered since request handling is
concurrent: different processes handling incoming requests can't know in
which order they were received on the transport, and different processes
sending requests can't know the order in which they're sent.
---
 lib/diameter/src/transport/diameter_sctp.erl | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/lib/diameter/src/transport/diameter_sctp.erl b/lib/diameter/src/transport/diameter_sctp.erl
index 86a98776c5..2af11729a9 100644
--- a/lib/diameter/src/transport/diameter_sctp.erl
+++ b/lib/diameter/src/transport/diameter_sctp.erl
@@ -102,10 +102,10 @@
          streams  :: {uint(), uint()}      %% {InStream, OutStream} counts
                    | undefined,
          os = 0   :: uint(),               %% next output stream
+         rotate = 1 :: boolean() | 0 | 1,  %% rotate os?
          packet = true :: boolean()        %% legacy transport_data?
                         | raw,
          message_cb = false :: false | diameter:eval(),
-         unordered = 1 :: boolean() | 0 | 1,       %% send unordered?
          send = false :: pid() | boolean()}).      %% sending process
 
 %% Monitor process state.
@@ -678,14 +678,14 @@ send(#diameter_packet{transport_data = {outstream, SId}}
      = S) ->
     send(SId rem OS, Msg, S);
 
-%% ... or not: rotate when sending ordered ...
-send(Msg, #transport{unordered = false,
+%% ... or not: rotate when sending on multiple streams ...
+send(Msg, #transport{rotate = true,
                      streams = {_, OS},
                      os = N}
           = S) ->
     send(N, Msg, S#transport{os = (N + 1) rem OS});
 
-%% ... or send only on the first stream, possibly unordered.
+%% ... or send on the only stream available.
 send(Msg, S) ->
     send(0, Msg, S).
 
@@ -777,24 +777,20 @@ recv({_, #sctp_pdapi_event{}}, _) ->
 
 %% recv/1
 %%
-%% Start sending unordered on a lone outbound stream after the second
-%% reception, so that an outgoing CER/CEA will arrive at the peer
-%% before another request.
+%% Start sending unordered after the second reception, so that an
+%% outgoing CER/CEA will arrive at the peer before another request.
 
-recv(#transport{unordered = B} = S)
+recv(#transport{rotate = B} = S)
   when is_boolean(B) ->
     S;
 
-recv(#transport{unordered = 0, streams = {_, 1}, socket = Sock} = S) ->
+recv(#transport{rotate = 0, streams = {_,N}, socket = Sock} = S) ->
     ok = inet:setopts(Sock, [{sctp_default_send_param,
                               #sctp_sndrcvinfo{flags = [unordered]}}]),
-    S#transport{unordered = true};
+    S#transport{rotate = 1 < N};
 
-recv(#transport{unordered = 0} = S) ->
-    S#transport{unordered = false};
-
-recv(#transport{unordered = N} = S) ->
-    S#transport{unordered = N-1}.
+recv(#transport{rotate = N} = S) ->
+    S#transport{rotate = N-1}.
 
 %% publish/4
 
-- 
cgit v1.2.3


From 5c0024cd76782f67499a09f7c524845d913408fc Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Wed, 30 Aug 2017 23:32:44 +0200
Subject: Enumerate AVPs in diameter_avp.index (again)

Commit 96cd627a changed the way the index field was used, but the
enumeration is used in at least one known application (as a pointer from
elements of diameter_packet.errors to elements of diameter_packet.avps)
and the motivation for the change is questionable: the lookup that was
avoided was unnecessary given that it was already performed in
incrementing a counter. Revert to enumerating as before in the non-relay
case, but not in the relay case since there's no corresponding usecase.
---
 lib/diameter/src/base/diameter_gen.erl | 63 ++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 30 deletions(-)

diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl
index 7a1a46ec52..2ef27e4b2e 100644
--- a/lib/diameter/src/base/diameter_gen.erl
+++ b/lib/diameter/src/base/diameter_gen.erl
@@ -238,7 +238,7 @@ enc(AvpName, Value, Opts, Mod) ->
 decode_avps(Name, Bin, #{module := Mod, decode_format := Fmt} = Opts) ->
     Strict = mget(strict_arities, Opts, decode),
     [AM, Avps, Failed | Rec]
-        = decode(Bin, Name, Mod, Fmt, Strict, Opts, #{}),
+        = decode(Bin, Name, Mod, Fmt, Strict, Opts, 0, #{}),
     %% AM counts the number of top-level AVPs, which missing/5 then
     %% uses when appending 5005 errors.
     {reformat(Name, Rec, Strict, Mod, Fmt),
@@ -249,7 +249,7 @@ decode_avps(Name, Bin, #{module := Mod, decode_format := Fmt} = Opts) ->
 %% encountered. Failed-AVP should typically contain the first
 %% error encountered.
 
-%% decode/7
+%% decode/8
 
 decode(<<Code:32, V:1, M:1, P:1, _:5, Len:24, I:V/unit:32, Rest/binary>>,
        Name,
@@ -257,6 +257,7 @@ decode(<<Code:32, V:1, M:1, P:1, _:5, Len:24, I:V/unit:32, Rest/binary>>,
        Fmt,
        Strict,
        Opts,
+       Idx,
        AM) ->
     decode(Rest,
            Code,
@@ -270,21 +271,23 @@ decode(<<Code:32, V:1, M:1, P:1, _:5, Len:24, I:V/unit:32, Rest/binary>>,
            Fmt,
            Strict,
            Opts,
+           Idx,
            AM);
 
-decode(<<>>, Name, Mod, Fmt, Strict, _, AM) ->
+decode(<<>>, Name, Mod, Fmt, Strict, _, _, AM) ->
     [AM, [], [] | newrec(Fmt, Mod, Name, Strict)];
 
-decode(Bin, Name, Mod, Fmt, Strict, _, AM) ->
-    Avp = #diameter_avp{data = Bin},
+decode(Bin, Name, Mod, Fmt, Strict, _, Idx, AM) ->
+    Avp = #diameter_avp{data = Bin, index = Idx},
     [AM, [Avp], [{5014, Avp}] | newrec(Fmt, Mod, Name, Strict)].
 
-%% decode/13
+%% decode/14
 
-decode(Bin, Code, Vid, DataLen, Pad, M, P, Name, Mod, Fmt, Strict, Opts0, AM0) ->
+decode(Bin, Code, Vid, DataLen, Pad, M, P, Name, Mod, Fmt, Strict, Opts0,
+       Idx, AM0) ->
     case Bin of
         <<Data:DataLen/binary, _:Pad/binary, T/binary>> ->
-            {NameT, AvpName, Arity, {Idx, AM}}
+            {NameT, AvpName, Arity, {I, AM}}
                 = incr(Name, Code, Vid, M, Mod, Strict, Opts0, AM0),
 
             Opts = setopts(NameT, Name, M, Opts0),
@@ -300,11 +303,11 @@ decode(Bin, Code, Vid, DataLen, Pad, M, P, Name, Mod, Fmt, Strict, Opts0, AM0) -
                                 type = type(NameT),
                                 index = Idx},
 
-            Dec = decode(Data, Name, NameT, Mod, Opts, Avp),    %% decode
-            Acc = decode(T, Name, Mod, Fmt, Strict, Opts, AM),  %% recurse
-            acc(Acc, Dec, Name, AvpName, Arity, Strict, Mod, Opts);
+            Dec = decode(Data, Name, NameT, Mod, Opts, Avp),         %% decode
+            Acc = decode(T, Name, Mod, Fmt, Strict, Opts, Idx+1, AM),%% recurse
+            acc(Acc, Dec, I, Name, AvpName, Arity, Strict, Mod, Opts);
         _ ->
-            {NameT, _AvpName, _Arity, {Idx, AM}}
+            {NameT, _AvpName, _Arity, {_, AM}}
                 = incr(Name, Code, Vid, M, Mod, Strict, Opts0, AM0),
 
             Avp = #diameter_avp{code = Code,
@@ -577,57 +580,57 @@ set_failed('Failed-AVP', #{failed_avp := false} = Opts) ->
 set_failed(_, Opts) ->
     Opts.
 
-%% acc/8
+%% acc/9
 
-acc([AM | Acc], As, Name, AvpName, Arity, Strict, Mod, Opts) ->
-    [AM | acc1(Acc, As, Name, AvpName, Arity, Strict, Mod, Opts)].
+acc([AM | Acc], As, I, Name, AvpName, Arity, Strict, Mod, Opts) ->
+    [AM | acc1(Acc, As, I, Name, AvpName, Arity, Strict, Mod, Opts)].
 
-%% acc1/8
+%% acc1/9
 
 %% Faulty AVP, not grouped.
-acc1(Acc, {_RC, Avp} = E, _, _, _, _, _, _) ->
+acc1(Acc, {_RC, Avp} = E, _, _, _, _, _, _, _) ->
     [Avps, Failed | Rec] = Acc,
     [[Avp | Avps], [E | Failed] | Rec];
 
 %% Faulty component in grouped AVP.
-acc1(Acc, {RC, As, Avp}, _, _, _, _, _, _) ->
+acc1(Acc, {RC, As, Avp}, _, _, _, _, _, _, _) ->
     [Avps, Failed | Rec] = Acc,
     [[As | Avps], [{RC, Avp} | Failed] | Rec];
 
 %% Grouped AVP ...
-acc1([Avps | Acc], [Avp|_] = As, Name, AvpName, Arity, Strict, Mod, Opts) ->
-    [[As|Avps] | acc2(Acc, Avp, Name, AvpName, Arity, Strict, Mod, Opts)];
+acc1([Avps | Acc], [Avp|_] = As, I, Name, AvpName, Arity, Strict, Mod, Opts) ->
+    [[As|Avps] | acc2(Acc, Avp, I, Name, AvpName, Arity, Strict, Mod, Opts)];
 
 %% ... or not.
-acc1([Avps | Acc], Avp, Name, AvpName, Arity, Strict, Mod, Opts) ->
-    [[Avp|Avps] | acc2(Acc, Avp, Name, AvpName, Arity, Strict, Mod, Opts)].
+acc1([Avps | Acc], Avp, I, Name, AvpName, Arity, Strict, Mod, Opts) ->
+    [[Avp|Avps] | acc2(Acc, Avp, I, Name, AvpName, Arity, Strict, Mod, Opts)].
 
-%% acc2/8
+%% acc2/9
 
 %% No errors, but nowhere to pack.
-acc2(Acc, Avp, _, 'AVP', 0, _, _, _) ->
+acc2(Acc, Avp, _, _, 'AVP', 0, _, _, _) ->
     [Failed | Rec] = Acc,
     [[{rc(Avp), Avp} | Failed] | Rec];
 
 %% No AVP of this name: try to pack as 'AVP'.
-acc2(Acc, Avp, Name, AvpName, 0, Strict, Mod, Opts) ->
+acc2(Acc, Avp, I, Name, AvpName, 0, Strict, Mod, Opts) ->
     M = Avp#diameter_avp.is_mandatory,
     Arity = pack_arity(Name, AvpName, Opts, Mod, M),
-    acc2(Acc, Avp, Name, 'AVP', Arity, Strict, Mod, Opts);
+    acc2(Acc, Avp, I, Name, 'AVP', Arity, Strict, Mod, Opts);
 
 %% Relaxed arities.
-acc2(Acc, Avp, _, AvpName, Arity, Strict, Mod, _)
+acc2(Acc, Avp, _, _, AvpName, Arity, Strict, Mod, _)
   when Strict /= decode ->
     pack(Arity, AvpName, Avp, Mod, Acc);
 
 %% No maximum arity.
-acc2(Acc, Avp, _, AvpName, {_,'*'} = Arity, _, Mod, _) ->
+acc2(Acc, Avp, _, _, AvpName, {_,'*'} = Arity, _, Mod, _) ->
     pack(Arity, AvpName, Avp, Mod, Acc);
 
 %% Or check.
-acc2(Acc, Avp, _, AvpName, Arity, _, Mod, _) ->
+acc2(Acc, Avp, I, _, AvpName, Arity, _, Mod, _) ->
     Mx = max_arity(Arity),
-    if Mx =< Avp#diameter_avp.index ->
+    if Mx =< I ->
             [Failed | Rec] = Acc,
             [[{5009, Avp} | Failed] | Rec];
        true ->
-- 
cgit v1.2.3


From 883ad0559b1a3551688866d88fa3089728e13b0c Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Thu, 31 Aug 2017 00:28:05 +0200
Subject: Fix decode of too many generic AVPs

That is, when the arity of an 'AVP' field has an upper bound. This
shouldn't happen in practice, but if an AVP is known but its name not
explicit in the message grammar then its count was confused with that
of AVPs packed into the 'AVP' field.
---
 lib/diameter/src/base/diameter_gen.erl | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl
index 2ef27e4b2e..2f84b2eae6 100644
--- a/lib/diameter/src/base/diameter_gen.erl
+++ b/lib/diameter/src/base/diameter_gen.erl
@@ -326,9 +326,14 @@ decode(Bin, Code, Vid, DataLen, Pad, M, P, Name, Mod, Fmt, Strict, Opts0,
 
 incr(Name, Code, Vid, M, Mod, Strict, Opts, AM0) ->
     NameT = Mod:avp_name(Code, Vid), %% {AvpName, Type} | 'AVP'
-    AvpName = field(NameT),
-    Arity = avp_arity(Name, AvpName, Mod, Opts, M),
-    {NameT, AvpName, Arity, incr(AvpName, Arity, Strict, AM0)}.
+    Field = field(NameT),            %% AvpName | 'AVP'
+    Arity = avp_arity(Name, Field, Mod, Opts, M),
+    if 0 == Arity, 'AVP' /= Field ->
+            A = pack_arity(Name, Field, Opts, Mod, M),
+            {NameT, 'AVP', A, incr('AVP', A, Strict, AM0)};
+       true ->
+            {NameT, Field, Arity, incr(Field, Arity, Strict, AM0)}
+    end.
 
 %% Data is a truncated header if command_code = undefined, otherwise
 %% payload bytes. The former is padded to the length of a header if
@@ -345,9 +350,8 @@ setopts({_, Type}, Name, M, Opts) ->
 
 %% incr/4
 
-incr(F, A, SA, AM)
-  when F == 'AVP';
-       A == ?ANY;
+incr(_, A, SA, AM)
+  when A == ?ANY;
        A == 0;
        SA /= decode ->
     {undefined, AM};
@@ -612,12 +616,6 @@ acc2(Acc, Avp, _, _, 'AVP', 0, _, _, _) ->
     [Failed | Rec] = Acc,
     [[{rc(Avp), Avp} | Failed] | Rec];
 
-%% No AVP of this name: try to pack as 'AVP'.
-acc2(Acc, Avp, I, Name, AvpName, 0, Strict, Mod, Opts) ->
-    M = Avp#diameter_avp.is_mandatory,
-    Arity = pack_arity(Name, AvpName, Opts, Mod, M),
-    acc2(Acc, Avp, I, Name, 'AVP', Arity, Strict, Mod, Opts);
-
 %% Relaxed arities.
 acc2(Acc, Avp, _, _, AvpName, Arity, Strict, Mod, _)
   when Strict /= decode ->
-- 
cgit v1.2.3


From ff4a0956d132c3af002bfbe5d48f18e492d3763f Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Thu, 31 Aug 2017 00:32:53 +0200
Subject: Rename variable

It's no longer the AVP name as of the parent commit, but the name of
the field/member the value will be stored in. Typically the AVP name,
but possibly 'AVP'.
---
 lib/diameter/src/base/diameter_gen.erl | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl
index 2f84b2eae6..f9172ec59d 100644
--- a/lib/diameter/src/base/diameter_gen.erl
+++ b/lib/diameter/src/base/diameter_gen.erl
@@ -287,7 +287,7 @@ decode(Bin, Code, Vid, DataLen, Pad, M, P, Name, Mod, Fmt, Strict, Opts0,
        Idx, AM0) ->
     case Bin of
         <<Data:DataLen/binary, _:Pad/binary, T/binary>> ->
-            {NameT, AvpName, Arity, {I, AM}}
+            {NameT, Field, Arity, {I, AM}}
                 = incr(Name, Code, Vid, M, Mod, Strict, Opts0, AM0),
 
             Opts = setopts(NameT, Name, M, Opts0),
@@ -305,9 +305,9 @@ decode(Bin, Code, Vid, DataLen, Pad, M, P, Name, Mod, Fmt, Strict, Opts0,
 
             Dec = decode(Data, Name, NameT, Mod, Opts, Avp),         %% decode
             Acc = decode(T, Name, Mod, Fmt, Strict, Opts, Idx+1, AM),%% recurse
-            acc(Acc, Dec, I, Name, AvpName, Arity, Strict, Mod, Opts);
+            acc(Acc, Dec, I, Name, Field, Arity, Strict, Mod, Opts);
         _ ->
-            {NameT, _AvpName, _Arity, {_, AM}}
+            {NameT, _Field, _Arity, {_, AM}}
                 = incr(Name, Code, Vid, M, Mod, Strict, Opts0, AM0),
 
             Avp = #diameter_avp{code = Code,
@@ -586,8 +586,8 @@ set_failed(_, Opts) ->
 
 %% acc/9
 
-acc([AM | Acc], As, I, Name, AvpName, Arity, Strict, Mod, Opts) ->
-    [AM | acc1(Acc, As, I, Name, AvpName, Arity, Strict, Mod, Opts)].
+acc([AM | Acc], As, I, Name, Field, Arity, Strict, Mod, Opts) ->
+    [AM | acc1(Acc, As, I, Name, Field, Arity, Strict, Mod, Opts)].
 
 %% acc1/9
 
@@ -602,12 +602,12 @@ acc1(Acc, {RC, As, Avp}, _, _, _, _, _, _, _) ->
     [[As | Avps], [{RC, Avp} | Failed] | Rec];
 
 %% Grouped AVP ...
-acc1([Avps | Acc], [Avp|_] = As, I, Name, AvpName, Arity, Strict, Mod, Opts) ->
-    [[As|Avps] | acc2(Acc, Avp, I, Name, AvpName, Arity, Strict, Mod, Opts)];
+acc1([Avps | Acc], [Avp|_] = As, I, Name, Field, Arity, Strict, Mod, Opts) ->
+    [[As|Avps] | acc2(Acc, Avp, I, Name, Field, Arity, Strict, Mod, Opts)];
 
 %% ... or not.
-acc1([Avps | Acc], Avp, I, Name, AvpName, Arity, Strict, Mod, Opts) ->
-    [[Avp|Avps] | acc2(Acc, Avp, I, Name, AvpName, Arity, Strict, Mod, Opts)].
+acc1([Avps | Acc], Avp, I, Name, Field, Arity, Strict, Mod, Opts) ->
+    [[Avp|Avps] | acc2(Acc, Avp, I, Name, Field, Arity, Strict, Mod, Opts)].
 
 %% acc2/9
 
@@ -617,22 +617,22 @@ acc2(Acc, Avp, _, _, 'AVP', 0, _, _, _) ->
     [[{rc(Avp), Avp} | Failed] | Rec];
 
 %% Relaxed arities.
-acc2(Acc, Avp, _, _, AvpName, Arity, Strict, Mod, _)
+acc2(Acc, Avp, _, _, Field, Arity, Strict, Mod, _)
   when Strict /= decode ->
-    pack(Arity, AvpName, Avp, Mod, Acc);
+    pack(Arity, Field, Avp, Mod, Acc);
 
 %% No maximum arity.
-acc2(Acc, Avp, _, _, AvpName, {_,'*'} = Arity, _, Mod, _) ->
-    pack(Arity, AvpName, Avp, Mod, Acc);
+acc2(Acc, Avp, _, _, Field, {_,'*'} = Arity, _, Mod, _) ->
+    pack(Arity, Field, Avp, Mod, Acc);
 
 %% Or check.
-acc2(Acc, Avp, I, _, AvpName, Arity, _, Mod, _) ->
+acc2(Acc, Avp, I, _, Field, Arity, _, Mod, _) ->
     Mx = max_arity(Arity),
     if Mx =< I ->
             [Failed | Rec] = Acc,
             [[{5009, Avp} | Failed] | Rec];
        true ->
-            pack(Arity, AvpName, Avp, Mod, Acc)
+            pack(Arity, Field, Avp, Mod, Acc)
     end.
 
 %% 3588/6733:
-- 
cgit v1.2.3


From ee62825c1ca752f401662529af7fe8fe247c2165 Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Wed, 30 Aug 2017 18:04:04 +0200
Subject: Fix extraction of Experimental-Result for counter keys

The introduction of decode_format in commit 722fa415 (and then 55e65b26)
meant the value was not necessarily the intended tuple.
---
 lib/diameter/src/base/diameter_traffic.erl | 67 ++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 23 deletions(-)

diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl
index 27a41d6eb0..4388175c3a 100644
--- a/lib/diameter/src/base/diameter_traffic.erl
+++ b/lib/diameter/src/base/diameter_traffic.erl
@@ -1891,16 +1891,12 @@ str(T) ->
 
 %% get_avp/3
 %%
-%% Find an AVP in a message of one of three forms:
-%%
-%% - a message record (as generated from a .dia spec) or
-%% - a list of an atom message name followed by 2-tuple, avp name/value pairs.
-%% - a list of a #diameter_header{} followed by #diameter_avp{} records,
-%%
-%% In the first two forms a dictionary module is used at encode to
-%% identify the type of the AVP and its arity in the message in
-%% question. The third form allows messages to be sent as is, without
-%% a dictionary, which is needed in the case of relay agents, for one.
+%% Find an AVP in a message in one of the decoded formats, or as a
+%% header/avps list. There are only four AVPs that are extracted here:
+%% Result-Code and Experimental-Result in order when constructing
+%% counter keys, and Destination-Host/Realm when selecting a next-hop
+%% peer. Experimental-Result is the only of type Grouped, and is given
+%% special treatment in order to return the value as a record.
 
 %% Messages will be header/avps list as a relay and the only AVP's we
 %% look for are in the common dictionary. This is required since the
@@ -1909,12 +1905,12 @@ str(T) ->
 get_avp(?RELAY, Name, Msg) ->
     get_avp(?BASE, Name, Msg);
 
-%% Message is a header/avps list.
+%% Message as header/avps list.
 get_avp(Dict, Name, [#diameter_header{} | Avps]) ->
     try
-        {Code, _, VId} = Dict:avp_header(Name),
-        A = find_avp(Code, VId, Avps),
-        (avp_decode(Dict, Name, ungroup(A)))#diameter_avp{name = Name}
+        {Code, _, Vid} = Dict:avp_header(Name),
+        A = find_avp(Code, Vid, Avps),
+        avp_decode(Dict, Name, ungroup(A))
     catch
         error: _ ->
             undefined
@@ -1924,20 +1920,33 @@ get_avp(Dict, Name, [#diameter_header{} | Avps]) ->
 get_avp(_, Name, [_MsgName | Avps]) ->
     case find(Name, Avps) of
         {_, V} ->
-            #diameter_avp{name = Name, value = V};
+            #diameter_avp{name = Name, value = value(Name, V)};
         _ ->
             undefined
     end;
 
-%% ... or record (but not necessarily).
+%% ... or record.
 get_avp(Dict, Name, Rec) ->
-    try
-        #diameter_avp{name = Name, value = Dict:'#get-'(Name, Rec)}
+    try Dict:'#get-'(Name, Rec) of
+        V ->
+            #diameter_avp{name = Name, value = value(Name, V)}
     catch
         error:_ ->
             undefined
     end.
 
+value('Experimental-Result' = N, #{'Vendor-Id' := Vid,
+                                   'Experimental-Result-Code' := RC}) ->
+    {N, Vid, RC};
+value('Experimental-Result' = N, [{'Experimental-Result-Code', RC},
+                                  {'Vendor-Id', Vid}]) ->
+    {N, Vid, RC};
+value('Experimental-Result' = N, [{'Vendor-Id', Vid},
+                                  {'Experimental-Result-Code', RC}]) ->
+    {N, Vid, RC};
+value(_, V) ->
+    V.
+
 %% find/2
 
 find(Key, Map)
@@ -1967,14 +1976,25 @@ ungroup(Avp) ->
 
 %% avp_decode/3
 
-avp_decode(Dict, Name, #diameter_avp{value = undefined,
+%% Ensure Experimental-Result is decoded as record, since this format
+%% is used for counter keys.
+avp_decode(Dict, 'Experimental-Result' = N, #diameter_avp{data = Bin}
+                                            = Avp)
+  when is_binary(Bin) ->
+    {V,_} = Dict:avp(decode, Bin, N, decode_opts(Dict)),
+    Avp#diameter_avp{name = N, value = V};
+
+avp_decode(Dict, Name, #diameter_avp{value = X,
                                      data = Bin}
                        = Avp)
-  when is_binary(Bin) ->
+  when is_binary(Bin), X == undefined orelse X == false ->
     V = Dict:avp(decode, Bin, Name, decode_opts(Dict)),
-    Avp#diameter_avp{value = V};
-avp_decode(_, _, #diameter_avp{} = Avp) ->
-    Avp.
+    Avp#diameter_avp{name = Name, value = V};
+
+avp_decode(_, Name, #diameter_avp{} = Avp) ->
+    Avp#diameter_avp{name = Name}.
+
+%% cb/3
 
 cb(#diameter_app{module = [_|_] = M}, F, A) ->
     eval(M, F, A).
@@ -1991,4 +2011,5 @@ decode_opts(Dict) ->
       string_decode => false,
       strict_mbit => false,
       failed_avp => false,
+      module => Dict,
       dictionary => Dict}.
-- 
cgit v1.2.3


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(-)

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


From 3ec3cc239999cb59d1260b6c7790eb112d9da47c Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Thu, 31 Aug 2017 16:09:11 +0200
Subject: Fix minor error-handling blunder

Leading to this admonition from dialyzer:

diameter_config.erl:670: The variable No can never match since previous
clauses completely covered the type 'ok'

The throw was caught, but resulted in an error return without the
intended information.
---
 lib/diameter/src/base/diameter_config.erl | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl
index 4721be1ca0..6fc4277ac8 100644
--- a/lib/diameter/src/base/diameter_config.erl
+++ b/lib/diameter/src/base/diameter_config.erl
@@ -665,9 +665,8 @@ opt(transport, {applications, As}) ->
     is_list(As);
 
 opt(transport, {capabilities, Os}) ->
-    is_list(Os) andalso case encode_CER(Os) of
-                            ok -> true;
-                            No -> {error, No}
+    is_list(Os) andalso try ok = encode_CER(Os), true
+                        catch ?FAILURE(No) -> {error, No}
                         end;
 
 opt(_, {K, Tmo})
-- 
cgit v1.2.3


From ae6966491183c62f6513b175aaede785871240cf Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Fri, 1 Sep 2017 16:58:50 +0200
Subject: Fix strict_arities blunder

Remove value from the merged map, not from the maps being merged.

Bundled in commit 5f3becad.
---
 lib/diameter/src/base/diameter_service.erl | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl
index c7b0e706a5..1e104f9e65 100644
--- a/lib/diameter/src/base/diameter_service.erl
+++ b/lib/diameter/src/base/diameter_service.erl
@@ -718,9 +718,13 @@ init_peers() ->
                                %%  TPid}
 
 service_opts(Opts) ->
-    T = {strict_arities, true},
-    maps:merge(maps:from_list([{monitor, false} | def_opts() -- [T]]),
-               maps:from_list(Opts -- [T])).
+    remove([{strict_arities, true}],
+           maps:merge(maps:from_list([{monitor, false} | def_opts()]),
+                      maps:from_list(Opts))).
+
+remove(List, Map) ->
+    maps:filter(fun(K,V) -> not lists:member({K,V}, List) end,
+                Map).
 
 def_opts() ->  %% defaults on the service map
     [{share_peers, false},
-- 
cgit v1.2.3


From a1b2f90096a70e521fdbcb2176aacf1dcc7c7f0e Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Thu, 31 Aug 2017 09:04:45 +0200
Subject: Fix dialyzer spec

Which dialyzer hasn't noticed.
---
 lib/diameter/src/base/diameter_gen.erl | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl
index f9172ec59d..763f70d877 100644
--- a/lib/diameter/src/base/diameter_gen.erl
+++ b/lib/diameter/src/base/diameter_gen.erl
@@ -45,7 +45,7 @@
 -define(THROW(T), throw({?MODULE, T})).
 
 -type parent_name()   :: atom().  %% parent = Message or AVP
--type parent_record() :: tuple(). %%
+-type parent_record() :: tuple() | avp_values() | map().
 -type avp_name()   :: atom().
 -type avp_record() :: tuple().
 -type avp_values() :: [{avp_name(), term()}].
@@ -61,9 +61,7 @@
 %% # encode_avps/3
 %% ---------------------------------------------------------------------------
 
--spec encode_avps(parent_name(),
-                  parent_record() | avp_values() | map(),
-                  map())
+-spec encode_avps(parent_name(), parent_record(), map())
    -> iolist()
     | no_return().
 
@@ -232,7 +230,7 @@ enc(AvpName, Value, Opts, Mod) ->
 %% ---------------------------------------------------------------------------
 
 -spec decode_avps(parent_name(), binary(), map())
-   -> {parent_record(), [avp()], Failed}
+   -> {parent_record() | false, [avp()], Failed}
  when Failed :: [{5000..5999, #diameter_avp{}}].
 
 decode_avps(Name, Bin, #{module := Mod, decode_format := Fmt} = Opts) ->
-- 
cgit v1.2.3


From 99db256e992591ab5cd03d77adcd4ad8d8a16a7e Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Thu, 31 Aug 2017 08:57:57 +0200
Subject: Tweak {decode_format, false} semantics

Represent the decoded message by its atom-valued name in
diameter_packet.msg, which makes trace much more readable. A
diameter_avp.value is untouched (ie. undefined): the AVP name is already
in the name field.
---
 lib/diameter/doc/src/diameter.xml            |  4 +--
 lib/diameter/src/base/diameter_gen.erl       | 45 ++++++++++++++++++----------
 lib/diameter/src/base/diameter_traffic.erl   |  4 +--
 lib/diameter/test/diameter_traffic_SUITE.erl |  5 ++--
 4 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml
index b7be184058..c31929c3f0 100644
--- a/lib/diameter/doc/src/diameter.xml
+++ b/lib/diameter/doc/src/diameter.xml
@@ -810,8 +810,8 @@ dictionary file in question.
 If <c>list</c> or <c>map</c> then a <c>[Name | Avps]</c> pair where
 <c>Avps</c> is either a list of AVP name/values pairs or a map keyed on
 AVP names respectively.
-If <c>false</c> then the representation is omitted and <c>msg</c> and
-<c>value</c> fields are set to <c>false</c>.
+If <c>false</c> then the representation is as the atom-value message
+name, or <c>undefined</c> for a Grouped AVP.
 See also &codec_message;.</p>
 
 <p>
diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl
index 763f70d877..e8f637dd53 100644
--- a/lib/diameter/src/base/diameter_gen.erl
+++ b/lib/diameter/src/base/diameter_gen.erl
@@ -230,7 +230,7 @@ enc(AvpName, Value, Opts, Mod) ->
 %% ---------------------------------------------------------------------------
 
 -spec decode_avps(parent_name(), binary(), map())
-   -> {parent_record() | false, [avp()], Failed}
+   -> {parent_record() | parent_name(), [avp()], Failed}
  when Failed :: [{5000..5999, #diameter_avp{}}].
 
 decode_avps(Name, Bin, #{module := Mod, decode_format := Fmt} = Opts) ->
@@ -301,7 +301,7 @@ decode(Bin, Code, Vid, DataLen, Pad, M, P, Name, Mod, Fmt, Strict, Opts0,
                                 type = type(NameT),
                                 index = Idx},
 
-            Dec = decode(Data, Name, NameT, Mod, Opts, Avp),         %% decode
+            Dec = decode1(Data, Name, NameT, Mod, Fmt, Opts, Avp),
             Acc = decode(T, Name, Mod, Fmt, Strict, Opts, Idx+1, AM),%% recurse
             acc(Acc, Dec, I, Name, Field, Arity, Strict, Mod, Opts);
         _ ->
@@ -449,10 +449,10 @@ field({AvpName, _}) ->
 field(_) ->
     'AVP'.
 
-%% decode/6
+%% decode1/7
 
 %% AVP not in dictionary.
-decode(_Data, _Name, 'AVP', _Mod, _Opts, Avp) ->
+decode1(_Data, _Name, 'AVP', _Mod, _Fmt, _Opts, Avp) ->
     Avp;
 
 %% 6733, 4.4:
@@ -502,7 +502,7 @@ decode(_Data, _Name, 'AVP', _Mod, _Opts, Avp) ->
 %% defined the RFC's "unrecognized", which is slightly stronger than
 %% "not defined".)
 
-decode(Data, Name, {AvpName, Type}, Mod, Opts, Avp) ->
+decode1(Data, Name, {AvpName, Type}, Mod, Fmt, Opts, Avp) ->
     #{dictionary := AppMod, failed_avp := Failed}
         = Opts,
 
@@ -516,26 +516,39 @@ decode(Data, Name, {AvpName, Type}, Mod, Opts, Avp) ->
     %% list of component AVPs.
 
     try avp_decode(Data, AvpName, Opts, DecMod, Mod) of
-        {Rec, As} when Type == 'Grouped' ->
-            A = Avp#diameter_avp{value = Rec},
-            [A | As];
-        V when Type /= 'Grouped' ->
-            Avp#diameter_avp{value = V}
+        V ->
+            set(Type, Fmt, Avp, V)
     catch
         throw: {?MODULE, T} ->
-            decode_error(Failed, T, Avp);
+            decode_error(Failed, Fmt, T, Avp);
         error: Reason ->
             decode_error(Failed, Reason, Name, Mod, Opts, Avp)
     end.
 
-%% decode_error/3
+%% set/4
+
+set('Grouped', false, Avp, V) ->
+    {_Rec, As} = V,
+    [Avp | As];
+
+set('Grouped', _, Avp, V) ->
+    {Rec, As} = V,
+    [Avp#diameter_avp{value = Rec} | As];
+
+set(_, _, Avp, V) ->
+    Avp#diameter_avp{value = V}.
+
+%% decode_error/4
 %%
 %% Error when decoding a grouped AVP.
 
-decode_error(true, {Rec, _, _}, Avp) ->
+decode_error(true, false, _, Avp) ->
+    Avp;
+
+decode_error(true, _, {Rec, _, _}, Avp) ->
     Avp#diameter_avp{value = Rec};
 
-decode_error(false, {_, ComponentAvps, [{RC,A} | _]}, Avp) ->
+decode_error(false, _, {_, ComponentAvps, [{RC,A} | _]}, Avp) ->
     {RC, [Avp | ComponentAvps], Avp#diameter_avp{data = [A]}}.
 
 %% decode_error/6
@@ -817,8 +830,8 @@ empty(Name, #{module := Mod} = Opts) ->
 
 %% newrec/4
 
-newrec(false = No, _, _, _) ->
-    No;
+newrec(false, _, Name, _) ->
+    Name;
 
 newrec(record, Mod, Name, T)
   when T /= decode ->
diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl
index 5ee598f513..3cc1c7cce5 100644
--- a/lib/diameter/src/base/diameter_traffic.erl
+++ b/lib/diameter/src/base/diameter_traffic.erl
@@ -1992,10 +1992,10 @@ avp_decode(Dict, 'Experimental-Result' = N, #diameter_avp{data = Bin}
     {V,_} = Dict:avp(decode, Bin, N, decode_opts(Dict)),
     Avp#diameter_avp{name = N, value = V};
 
-avp_decode(Dict, Name, #diameter_avp{value = X,
+avp_decode(Dict, Name, #diameter_avp{value = undefined,
                                      data = Bin}
                        = Avp)
-  when is_binary(Bin), X == undefined orelse X == false ->
+  when is_binary(Bin) ->
     V = Dict:avp(decode, Bin, Name, decode_opts(Dict)),
     Avp#diameter_avp{name = Name, value = V};
 
diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl
index 2bc00bf4a1..403d417636 100644
--- a/lib/diameter/test/diameter_traffic_SUITE.erl
+++ b/lib/diameter/test/diameter_traffic_SUITE.erl
@@ -1161,7 +1161,7 @@ to_map(#diameter_packet{header = H, msg = Rec},
 
 %% No record decode: do it ourselves.
 to_map(#diameter_packet{header = H,
-                        msg = false,
+                        msg = Name,
                         bin = Bin},
       #group{server_decoding = false,
              strings = B}) ->
@@ -1169,8 +1169,9 @@ to_map(#diameter_packet{header = H,
              string_decode => B,
              strict_mbit => true,
              rfc => 6733},
-    #diameter_packet{msg = [_MsgName | _Map] = Msg}
+    #diameter_packet{msg = [MsgName | _Map] = Msg}
         = diameter_codec:decode(dict(H), Opts, Bin),
+    {MsgName, _} = {Name, Msg},  %% assert
     Msg.
 
 dict(#diameter_header{application_id = Id,
-- 
cgit v1.2.3


From 36a64980960077558dcfd0e181a65753856f331c Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Thu, 31 Aug 2017 09:53:45 +0200
Subject: Rename decode_format false to none

Which reads better and makes it easier to distinguish this false from
others.
---
 lib/diameter/doc/src/diameter.xml            |  8 ++++----
 lib/diameter/src/base/diameter.erl           |  2 +-
 lib/diameter/src/base/diameter_config.erl    |  4 ++--
 lib/diameter/src/base/diameter_gen.erl       | 11 ++++++-----
 lib/diameter/src/base/diameter_watchdog.erl  |  4 ++--
 lib/diameter/test/diameter_traffic_SUITE.erl |  8 ++++----
 6 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml
index c31929c3f0..0169afb619 100644
--- a/lib/diameter/doc/src/diameter.xml
+++ b/lib/diameter/doc/src/diameter.xml
@@ -799,7 +799,7 @@ be matched by corresponding &capability; configuration, of
 
 <tag>
 <marker id="decode_format"/>
-<c>{decode_format, record | list | map | false}</c></tag>
+<c>{decode_format, record | list | map | none}</c></tag>
 <item>
 <p>
 The format of decoded messages and grouped AVPs in the <c>msg</c> field
@@ -808,10 +808,10 @@ records respectively.
 If <c>record</c> then a record whose definition is generated from the
 dictionary file in question.
 If <c>list</c> or <c>map</c> then a <c>[Name | Avps]</c> pair where
-<c>Avps</c> is either a list of AVP name/values pairs or a map keyed on
+<c>Avps</c> is a list of AVP name/values pairs or a map keyed on
 AVP names respectively.
-If <c>false</c> then the representation is as the atom-value message
-name, or <c>undefined</c> for a Grouped AVP.
+If <c>none</c> then the atom-value message name, or <c>undefined</c>
+for a Grouped AVP.
 See also &codec_message;.</p>
 
 <p>
diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl
index 3b41feac0d..69ef6f4ec0 100644
--- a/lib/diameter/src/base/diameter.erl
+++ b/lib/diameter/src/base/diameter.erl
@@ -340,7 +340,7 @@ call(SvcName, App, Message) ->
    :: record
     | list
     | map
-    | false
+    | none
     | record_from_map.
 
 -type strict_arities()
diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl
index 6fc4277ac8..284f885884 100644
--- a/lib/diameter/src/base/diameter_config.erl
+++ b/lib/diameter/src/base/diameter_config.erl
@@ -588,8 +588,7 @@ opt(service, {K, false})
        K == use_shared_peers;
        K == monitor;
        K == restrict_connections;
-       K == strict_arities;
-       K == decode_format ->
+       K == strict_arities ->
     true;
 
 opt(service, {K, true})
@@ -602,6 +601,7 @@ opt(service, {decode_format, T})
   when T == record;
        T == list;
        T == map;
+       T == none;
        T == record_from_map ->
     true;
 
diff --git a/lib/diameter/src/base/diameter_gen.erl b/lib/diameter/src/base/diameter_gen.erl
index e8f637dd53..0aea982a54 100644
--- a/lib/diameter/src/base/diameter_gen.erl
+++ b/lib/diameter/src/base/diameter_gen.erl
@@ -527,7 +527,7 @@ decode1(Data, Name, {AvpName, Type}, Mod, Fmt, Opts, Avp) ->
 
 %% set/4
 
-set('Grouped', false, Avp, V) ->
+set('Grouped', none, Avp, V) ->
     {_Rec, As} = V,
     [Avp | As];
 
@@ -542,7 +542,7 @@ set(_, _, Avp, V) ->
 %%
 %% Error when decoding a grouped AVP.
 
-decode_error(true, false, _, Avp) ->
+decode_error(true, none, _, Avp) ->
     Avp;
 
 decode_error(true, _, {Rec, _, _}, Avp) ->
@@ -735,8 +735,9 @@ pack(Arity, F, Avp, Mod, [Failed | Rec]) ->
 
 %% set/5
 
-set(_, _, _, _, false = No) ->
-    No;
+set(_, _, _, _, None)
+  when is_atom(None) ->
+    None;
 
 set(1, F, Value, _, Map)
   when is_map(Map) ->
@@ -830,7 +831,7 @@ empty(Name, #{module := Mod} = Opts) ->
 
 %% newrec/4
 
-newrec(false, _, Name, _) ->
+newrec(none, _, Name, _) ->
     Name;
 
 newrec(record, Mod, Name, T)
diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl
index bb671e9860..c08e2da672 100644
--- a/lib/diameter/src/base/diameter_watchdog.erl
+++ b/lib/diameter/src/base/diameter_watchdog.erl
@@ -72,7 +72,7 @@
                      restrict := boolean(),
                      suspect := non_neg_integer(), %% OKAY -> SUSPECT
                      okay := non_neg_integer()},   %% REOPEN -> OKAY
-         codec :: #{decode_format := false,
+         codec :: #{decode_format := none,
                     string_decode := false,
                     strict_arities => diameter:strict_arities(),
                     strict_mbit := boolean(),
@@ -157,7 +157,7 @@ i({Ack, T, Pid, {Opts,
                                  string_decode,
                                  rfc,
                                  ordered_encode],
-                                SvcOpts#{decode_format := false,
+                                SvcOpts#{decode_format := none,
                                          string_decode := false,
                                          ordered_encode => false})}.
 
diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl
index 403d417636..559ec14c2b 100644
--- a/lib/diameter/test/diameter_traffic_SUITE.erl
+++ b/lib/diameter/test/diameter_traffic_SUITE.erl
@@ -154,7 +154,7 @@
 -define(ENCODINGS, [list, record, map]).
 
 %% How to decode incoming messages.
--define(DECODINGS, [record, false, map, list, record_from_map]).
+-define(DECODINGS, [record, none, map, list, record_from_map]).
 
 %% Which dictionary to use in the clients.
 -define(RFCS, [rfc3588, rfc6733, rfc4005]).
@@ -1109,12 +1109,12 @@ origin(N) ->
 decode(record) -> 0;
 decode(list)   -> 1;
 decode(map)    -> 2;
-decode(false)  -> 3;
+decode(none)   -> 3;
 decode(record_from_map) -> 4;
 decode(0) -> record;
 decode(1) -> list;
 decode(2) -> map;
-decode(3) -> false;
+decode(3) -> none;
 decode(4) -> record_from_map.
 
 encode(record) -> 0;
@@ -1163,7 +1163,7 @@ to_map(#diameter_packet{header = H, msg = Rec},
 to_map(#diameter_packet{header = H,
                         msg = Name,
                         bin = Bin},
-      #group{server_decoding = false,
+      #group{server_decoding = none,
              strings = B}) ->
     Opts = #{decode_format => map,
              string_decode => B,
-- 
cgit v1.2.3


From 1717f3214a1a41950c0da9ca95c89824c4c9898f Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Thu, 31 Aug 2017 10:34:14 +0200
Subject: Fix decode_format doc oversights

diameter_codec(3) referred only to the record format in places. The
configurable format was added in commits 722fa415 and then 55e65b26.
---
 lib/diameter/doc/src/diameter_codec.xml | 18 +++++++++++-------
 lib/diameter/doc/src/seealso.ent        |  1 +
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/lib/diameter/doc/src/diameter_codec.xml b/lib/diameter/doc/src/diameter_codec.xml
index 0846334d23..5124b49484 100644
--- a/lib/diameter/doc/src/diameter_codec.xml
+++ b/lib/diameter/doc/src/diameter_codec.xml
@@ -4,7 +4,10 @@
   '<seealso marker="diameter_dict#MESSAGE_RECORDS">diameter_dict(4)</seealso>'>
   <!ENTITY types
   '<seealso marker="diameter_dict#DATA_TYPES">diameter_dict(4)</seealso>'>
-  <!ENTITY % also SYSTEM "seealso.ent" >
+  <!ENTITY decode_format
+  '<seealso marker="diameter#decode_format">decode format</seealso>'>
+
+<!ENTITY % also SYSTEM "seealso.ent" >
   <!ENTITY % here SYSTEM "seehere.ent" >
   %also;
   %here;
@@ -145,7 +148,8 @@ question.</p>
 <p>
 The decoded value of an AVP.
 Will be <c>undefined</c> on decode if the data bytes could
-not be decoded or the AVP is unknown.
+not be decoded, the AVP is unknown, or if the &decode_format; is
+<c>none</c>.
 The type of a decoded value is as document in &types;.</p>
 </item>
 
@@ -243,8 +247,7 @@ Equivalently, a message can also be encoded as a list whose head is
 the atom-valued message name (as specified in the relevant dictionary
 file) and whose tail is either a list of AVP name/values
 pairs or a map with values keyed on AVP names.
-The format at decode is determined by &mod_service_opt;
-<c>decode_format</c>.
+The format at decode is determined by &mod_decode_format;.
 Any of the formats is accepted at encode.</p>
 
 <p>
@@ -288,15 +291,16 @@ value other than <c>undefined</c>.</p>
 <item>
 <p>
 The incoming/outgoing message.
-For an incoming message, a record if the message can be
-decoded in a non-relay application, <c>undefined</c> otherwise.
+For an incoming message, a term corresponding to the configured
+&decode_format; if the message can be decoded in a non-relay
+application, <c>undefined</c> otherwise.
 For an outgoing message, setting a <c>[&header; | &avp;]</c> list is
 equivalent to setting the <c>header</c> and <c>avps</c> fields to the
 corresponding values.</p>
 
 <warning>
 <p>
-A record-valued <c>msg</c> field does <em>not</em> imply an absence of
+A value in the <c>msg</c> field does <em>not</em> imply an absence of
 decode errors.
 The <c>errors</c> field should also be examined.</p>
 </warning>
diff --git a/lib/diameter/doc/src/seealso.ent b/lib/diameter/doc/src/seealso.ent
index ef6af1a3d0..c5a53670d0 100644
--- a/lib/diameter/doc/src/seealso.ent
+++ b/lib/diameter/doc/src/seealso.ent
@@ -72,6 +72,7 @@ significant.
 <!ENTITY watchdog_timer '<seealso marker="#watchdog_timer">watchdog_timer</seealso>'>
 
 <!ENTITY mod_string_decode '<seealso marker="diameter#service_opt">diameter:service_opt()</seealso> <seealso marker="diameter#string_decode">string_decode</seealso>'>
+<!ENTITY mod_decode_format '<seealso marker="diameter#service_opt">diameter:service_opt()</seealso> <seealso marker="diameter#decode_format">decode_format</seealso>'>
 
 <!-- diameter_app -->
 
-- 
cgit v1.2.3


From 70b228f55a34a29f0019ab22affa9f0b70acdabf Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Thu, 31 Aug 2017 13:20:36 +0200
Subject: Map less in traffic suite

By just decoding to map in the client, instead of to record first. The
record decode is exercised enough in the server and in other suites.
---
 lib/diameter/test/diameter_traffic_SUITE.erl | 97 +++++++++++-----------------
 1 file changed, 36 insertions(+), 61 deletions(-)

diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl
index 559ec14c2b..b4656562c4 100644
--- a/lib/diameter/test/diameter_traffic_SUITE.erl
+++ b/lib/diameter/test/diameter_traffic_SUITE.erl
@@ -498,6 +498,7 @@ start_services(Config) ->
                                      | ?SERVICE(SN, Grp)]),
     ok = diameter:start_service(CN, [{traffic_counters, bool()},
                                      {sequence, ?CLIENT_MASK},
+                                     {decode_format, map},
                                      {strict_arities, decode}
                                      | ?SERVICE(CN, Grp)]).
 
@@ -683,9 +684,9 @@ send_proxy_info(Config) ->
     Req = ['ASR', {'Proxy-Info', #{'Proxy-Host'  => H0,
                                    'Proxy-State' => S0}}],
     ['answer-message' | #{'Result-Code' := 3999,
-                          'Proxy-Info' := [Rec]}]
+                          'Proxy-Info' := [#{'Proxy-Host' := H,
+                                             'Proxy-State' := S}]}]
         = 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.
@@ -711,12 +712,12 @@ send_unknown_short(Config, M, RC) ->
                                          data = <<17>>}]}],
     ['ASA' | #{'Session-Id' := _,
                'Result-Code' := RC,
-               'Failed-AVP' := Avps}]
+               'Failed-AVP' := [#{'AVP' := [Avp]}]}]
         = call(Config, Req),
-    [[#diameter_avp{code = 999,
-                    is_mandatory = M,
-                    data = <<17, _/binary>>}]] %% extra bits from padding
-        = failed_avps(Avps, Config).
+    #diameter_avp{code = 999,
+                  is_mandatory = M,
+                  data = <<17, _/binary>>} %% extra bits from padding
+        = Avp.
 
 %% Ditto but set the M flag.
 send_unknown_mandatory(Config) ->
@@ -725,12 +726,12 @@ send_unknown_mandatory(Config) ->
                                          data = <<17>>}]}],
     ['ASA' | #{'Session-Id' := _,
                'Result-Code' := ?AVP_UNSUPPORTED,
-               'Failed-AVP' := Avps}]
+               'Failed-AVP' := [#{'AVP' := [Avp]}]}]
         = call(Config, Req),
-    [[#diameter_avp{code = 999,
-                    is_mandatory = true,
-                    data = <<17>>}]]
-        = failed_avps(Avps, Config).
+    #diameter_avp{code = 999,
+                  is_mandatory = true,
+                  data = <<17>>}
+        = Avp.
 
 %% Ditto, and point the AVP length past the end of the message. Expect
 %% 5014 instead of 5001.
@@ -745,13 +746,13 @@ send_unexpected_mandatory_decode(Config) ->
                                          data = <<12:32>>}]}],
     ['ASA' | #{'Session-Id' := _,
                'Result-Code' := ?AVP_UNSUPPORTED,
-               'Failed-AVP' := Avps}]
+               'Failed-AVP' := [#{'AVP' := [Avp]}]}]
         = call(Config, Req),
-    [[#diameter_avp{code = 27,
-                    is_mandatory = true,
-                    value = 12,
-                    data = <<12:32>>}]]
-        = failed_avps(Avps, Config).
+    #diameter_avp{code = 27,
+                  is_mandatory = true,
+                  value = 12,
+                  data = <<12:32>>}
+        = Avp.
 
 %% Try to two Auth-Application-Id in ASR expect 5009.
 send_too_many(Config) ->
@@ -759,11 +760,11 @@ send_too_many(Config) ->
 
     ['ASA' | #{'Session-Id' := _,
                'Result-Code' := ?TOO_MANY,
-               'Failed-AVP' := Avps}]
+               'Failed-AVP' := [#{'AVP' := [Avp]}]}]
         = call(Config, Req),
-    [[#diameter_avp{name = 'Auth-Application-Id',
-                    value = 44}]]
-        = failed_avps(Avps, Config).
+    #diameter_avp{name = 'Auth-Application-Id',
+                  value = 44}
+        = Avp.
 
 %% Send an containing a faulty Grouped AVP (empty Proxy-Host in
 %% Proxy-Info) and expect that only the faulty AVP is sent in
@@ -775,12 +776,11 @@ send_grouped_error(Config) ->
                                    {'Proxy-State', ""}]]}],
     ['ASA' | #{'Session-Id' := _,
                'Result-Code' := ?INVALID_AVP_LENGTH,
-               'Failed-AVP' := Avps}]
+               'Failed-AVP' := [#{'AVP' := [Avp]}]}]
         = call(Config, Req),
-    [[#diameter_avp{name = 'Proxy-Info', value = V}]]
-        = failed_avps(Avps, Config),
-    {Empty, undefined, []} = proxy_info(V, Config),
-    <<0>> = ?B(Empty).
+    #diameter_avp{name = 'Proxy-Info', value = #{'Proxy-Host' := H}}
+        = Avp,
+    <<0>> = ?B(H).
 
 %% Send an STR that the server ignores.
 send_noreply(Config) ->
@@ -833,9 +833,8 @@ send_invalid_avp_length(Config) ->
                'Result-Code' := ?INVALID_AVP_LENGTH,
                'Origin-Host' := _,
                'Origin-Realm' := _,
-               'Failed-AVP' := Avps}]
-        = call(Config, Req),
-    [[_]] = failed_avps(Avps, Config).
+               'Failed-AVP' := [#{'AVP' := [_]}]}]
+        = call(Config, Req).
 
 %% Send a request containing 5xxx errors that the server rejects with
 %% 3xxx.
@@ -1046,29 +1045,6 @@ send_anything(Config) ->
 
 %% ===========================================================================
 
-failed_avps(Avps, Config) ->
-    #group{client_dict = D} = proplists:get_value(group, Config),
-    [failed_avp(D, T) || T <- Avps].
-
-failed_avp(nas4005, {'nas_Failed-AVP', As}) ->
-    As;
-failed_avp(_, #'diameter_base_Failed-AVP'{'AVP' = As}) ->
-    As.
-
-proxy_info(Rec, Config) ->
-    #group{client_dict = D} = proplists:get_value(group, Config),
-    if D == nas4005 ->
-            {'nas_Proxy-Info', H, S, As}
-                = Rec,
-            {H,S,As};
-       true ->
-            #'diameter_base_Proxy-Info'{'Proxy-Host' = H,
-                                        'Proxy-State' = S,
-                                        'AVP' = As}
-                = Rec,
-            {H,S,As}
-    end.
-
 group(Config) ->
     #group{} = proplists:get_value(group, Config).
 
@@ -1539,24 +1515,23 @@ answer(Pkt, Req, _Peer, Name, #group{client_dict = Dict0}) ->
     #diameter_packet{header = H, msg = Ans, errors = Es} = Pkt,
     ApplId = app(Req, Name, Dict0),
     #diameter_header{application_id = ApplId} = H,  %% assert
-    Dict = dict(Ans, Dict0),
-    rec_to_map(answer(Ans, Es, Name), Dict).
+    answer(Ans, Es, Name).
 
 %% Missing Result-Code and inappropriate Experimental-Result-Code.
-answer(Rec, Es, send_experimental_result) ->
+answer(Ans, Es, send_experimental_result) ->
     [{5004, #diameter_avp{name = 'Experimental-Result'}},
      {5005, #diameter_avp{name = 'Result-Code'}}]
         =  Es,
-    Rec;
+    Ans;
 
 %% An inappropriate E-bit results in a decode error ...
-answer(Rec, Es, send_bad_answer) ->
+answer(Ans, Es, send_bad_answer) ->
     [{5004, #diameter_avp{name = 'Result-Code'}} | _] = Es,
-    Rec;
+    Ans;
 
 %% ... while other errors are reflected in Failed-AVP.
-answer(Rec, [], _) ->
-    Rec.
+answer(Ans, [], _) ->
+    Ans.
 
 app(_, send_unsupported_app, _) ->
     ?BAD_APP;
-- 
cgit v1.2.3