Merge branch 'anders/diameter/inappropriate_DWR/OTP-11184' into maint

* anders/diameter/inappropriate_DWR/OTP-11184:
  Ensure DWR isn't sent immediately after DWA
  Comment tweaks

3 files changed, 44 insertions, 47 deletions

diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl
index 4e55864168..2b99ecc59c 100644
--- a/lib/diameter/src/base/diameter_peer_fsm.erl
+++ b/lib/diameter/src/base/diameter_peer_fsm.erl
@@ -28,7 +28,8 @@
 -behaviour(gen_server).
 
 %% Interface towards diameter_watchdog.
--export([start/3]).
+-export([start/3,
+         result_code/2]).
 
 %% gen_server callbacks
 -export([init/1,
@@ -62,7 +63,6 @@
 %% Keys in process dictionary.
 -define(CB_KEY, cb).         %% capabilities callback
 -define(DPR_KEY, dpr).       %% disconnect callback
--define(DWA_KEY, dwa).       %% outgoing DWA
 -define(REF_KEY, ref).       %% transport_ref()
 -define(Q_KEY, q).           %% transport start queue
 -define(START_KEY, start).   %% start of connected transport
@@ -177,14 +177,9 @@ init(T) ->
     proc_lib:init_ack({ok, self()}),
     gen_server:enter_loop(?MODULE, [], i(T)).
 
-i({Ack, WPid, {M, Ref} = T, Opts, {Mask,
-                                   Nodes,
-                                   Dict0,
-                                   #diameter_service{capabilities = LCaps}
-                                   = Svc}}) ->
+i({Ack, WPid, {M, Ref} = T, Opts, {Mask, Nodes, Dict0, Svc}}) ->
     erlang:monitor(process, WPid),
     wait(Ack, WPid),
-    putr(?DWA_KEY, dwa(LCaps)),
     diameter_stats:reg(Ref),
     {[Cs,Ds], Rest} = proplists:split(Opts, [capabilities_cb, disconnect_cb]),
     putr(?CB_KEY, {Ref, [F || {_,F} <- Cs]}),
@@ -612,9 +607,7 @@ rcv(Name, _, #state{state = PS})
        Name == 'CEA' ->
     {stop, {Name, PS}};
 
-rcv(N, Pkt, S)
-  when N == 'DWR';
-       N == 'DPR' ->
+rcv('DPR' = N, Pkt, S) ->
     handle_request(N, Pkt, S);
 
 %% DPA in response to DPR and with the expected identifiers.
@@ -717,8 +710,8 @@ build_answer(Type,
                               errors = Es}
              = Pkt,
              S) ->
-    RC = rc(H, Es),
-    {answer(Type, RC, Es, S), post(Type, RC, Pkt, S)}.
+    {RC, FailedAVP} = result_code(H, Es),
+    {answer(Type, RC, FailedAVP, S), post(Type, RC, Pkt, S)}.
 
 inband_security([]) ->
     ?NO_INBAND_SECURITY;
@@ -734,7 +727,7 @@ cea(CEA, RC, Dict0) ->
 
 post('CER' = T, RC, Pkt, S) ->
     {T, caps(S), {RC, Pkt}};
-post(_, _, _, _) ->
+post('DPR', _, _, _) ->
     ok.
 
 rejected({capabilities_cb, _F, Reason}, T, S) ->
@@ -743,13 +736,10 @@ rejected({capabilities_cb, _F, Reason}, T, S) ->
 rejected(discard, T, _) ->
     close(T);
 rejected({N, Es}, T, S) ->
-    {answer('CER', N, Es, S), T};
+    {answer('CER', N, failed_avp(N, Es), S), T};
 rejected(N, T, S) ->
     rejected({N, []}, T, S).
 
-answer(Type, RC, Es, S) ->
-    set(answer(Type, RC, S), failed_avp(RC, Es)).
-
 failed_avp(RC, [{RC, Avp} | _]) ->
     [{'Failed-AVP', [{'AVP', [Avp]}]}];
 failed_avp(RC, [_ | Es]) ->
@@ -757,6 +747,9 @@ failed_avp(RC, [_ | Es]) ->
 failed_avp(_, [] = No) ->
     No.
 
+answer(Type, RC, FailedAVP, S) ->
+    set(answer(Type, RC, S), FailedAVP).
+
 answer(Type, RC, S) ->
     answer_message(answer(Type, S), RC).
 
@@ -784,29 +777,29 @@ set(['answer-message' | _] = Ans, FailedAvp) ->
 set([_|_] = Ans, FailedAvp) ->
     Ans ++ FailedAvp.
 
-%% rc/2
+%% result_code/2
 
-rc(#diameter_header{is_error = true}, _) ->
-    3008;  %% DIAMETER_INVALID_HDR_BITS
+result_code(#diameter_header{is_error = true}, _) ->
+    {3008, []};  %% DIAMETER_INVALID_HDR_BITS
 
-rc(_, [Bs|_])
+result_code(_, [Bs|_])
   when is_bitstring(Bs) ->  %% from old code
-    3009;  %% DIAMETER_INVALID_HDR_BITS
+    {3009, []};  %% DIAMETER_INVALID_HDR_BITS
 
-rc(#diameter_header{version = ?DIAMETER_VERSION}, Es) ->
+result_code(#diameter_header{version = ?DIAMETER_VERSION}, Es) ->
     rc(Es);
 
-rc(_, _) ->
-    5011.  %% DIAMETER_UNSUPPORTED_VERSION
+result_code(_, _) ->
+    {5011, []}.  %% DIAMETER_UNSUPPORTED_VERSION
 
 %% rc/1
 
 rc([]) ->
-    2001;  %% DIAMETER_SUCCESS
-rc([{RC,_}|_]) ->
-    RC;
+    {2001, []};  %% DIAMETER_SUCCESS
+rc([{RC, _} | _] = Es) ->
+    {RC, failed_avp(RC, Es)};
 rc([RC|_]) ->
-    RC.
+    {RC, []}.
 
 %%   DIAMETER_INVALID_HDR_BITS          3008
 %%      A request was received whose bits in the Diameter header were
@@ -832,9 +825,6 @@ rc([RC|_]) ->
 
 %% answer/2
 
-answer('DWR', _) ->
-    getr(?DWA_KEY);
-
 answer(Name, #state{service = #diameter_service{capabilities = Caps}}) ->
     a(Name, Caps).
 
@@ -1019,15 +1009,6 @@ report({M, _, _, _, _} = T)
 report(_) ->
     ok.
 
-%% dwa/1
-
-dwa(#diameter_caps{origin_host = OH,
-                   origin_realm = OR,
-                   origin_state_id = OSI}) ->
-    ['DWA', {'Origin-Host', OH},
-            {'Origin-Realm', OR},
-            {'Origin-State-Id', OSI}].
-
 %% dpr/2
 %%
 %% The RFC isn't clear on whether DPR should be send in a non-Open
diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl
index 88ccf630e2..7e75801718 100644
--- a/lib/diameter/src/base/diameter_watchdog.erl
+++ b/lib/diameter/src/base/diameter_watchdog.erl
@@ -201,7 +201,7 @@ common_dictionary(Apps) ->
             %% means a user won't be able either send of receive
             %% messages in the common dictionary: incoming request
             %% will be answered with 3007 and outgoing requests cannot
-            %% be sent. The dictionary returned here is oly used for
+            %% be sent. The dictionary returned here is only used for
             %% messages diameter sends and receives: CER/CEA, DPR/DPA
             %% and DWR/DWA.
             ?BASE
@@ -545,10 +545,15 @@ recv(Name, Pkt, S) ->
 
 %% rcv/3
 
+rcv('DWR', Pkt, #watchdog{transport = TPid,
+                          dictionary = Dict0,
+                          sequence = Mask}) ->
+    send(TPid, {send, encode(dwa(Pkt), Mask, Dict0)}),
+    ?LOG(send, 'DWA');
+
 rcv(N, _, _)
   when N == 'CER';
        N == 'CEA';
-       N == 'DWR';
        N == 'DWA';
        N == 'DPR';
        N == 'DPA' ->
@@ -642,6 +647,9 @@ rcv('DWA', #watchdog{status = reopen,
 
 %%   REOPEN        Receive non-DWA      Throwaway()          REOPEN
 
+rcv('DWR', #watchdog{status = reopen} = S) ->
+    S;  %% ensure DWA: the RFC isn't explicit about answering
+
 rcv(_, #watchdog{status = reopen} = S) ->
     throwaway(S).
 
@@ -782,6 +790,13 @@ dwr(#diameter_caps{origin_host = OH,
             {'Origin-Realm', OR},
             {'Origin-State-Id', OSI}].
 
+%% dwa/1
+
+dwa(#diameter_packet{header = H, errors = Es}) ->
+    {RC, FailedAVP} = diameter_peer_fsm:result_code(H, Es),
+    ['DWA', {'Result-Code', RC}
+          | tl(getr(dwr)) ++ FailedAVP].
+
 %% restrict_nodes/1
 
 restrict_nodes(false) ->
diff --git a/lib/diameter/test/diameter_transport_SUITE.erl b/lib/diameter/test/diameter_transport_SUITE.erl
index 97f4cec11f..9408fae62c 100644
--- a/lib/diameter/test/diameter_transport_SUITE.erl
+++ b/lib/diameter/test/diameter_transport_SUITE.erl
@@ -180,12 +180,13 @@ reconnect({listen, Ref}) ->
     [_] = diameter_reg:wait({diameter_tcp, listener, {LRef, '_'}}),
     true = diameter_reg:add_new({?MODULE, Ref, LRef}),
 
-    %% Wait for partner to request transport death: kill to force the
-    %% peer to reconnect.
+    %% Wait for partner to request transport death.
     TPid = abort(SvcName, LRef, Ref),
 
+    %% Kill transport to force the peer to reconnect.
     exit(TPid, kill),
 
+    %% Wait for the partner again.
     abort(SvcName, LRef, Ref);
 
 reconnect({connect, Ref}) ->
@@ -200,7 +201,7 @@ reconnect({connect, Ref}) ->
     %% reconnection attempts.
     abort(SvcName, Pid, Ref),
 
-    %% Transport does down and is reestablished.
+    %% Transport goes down and is reestablished.
     ?RECV(#diameter_event{service = SvcName, info = {down, CRef, _, _}}),
     ?RECV(#diameter_event{service = SvcName, info = {reconnect, CRef, _}}),
     ?RECV(#diameter_event{service = SvcName, info = {up, CRef, _, _, _}}),