Merge branch 'anders/diameter/transport_data/OTP-10566' into maint

* anders/diameter/transport_data/OTP-10566:
  Add a testcase
  Allow a handle_request callback to return a #diameter_packet{}

3 files changed, 71 insertions, 27 deletions

diff --git a/lib/diameter/doc/src/diameter_app.xml b/lib/diameter/doc/src/diameter_app.xml
index ac056c2d39..b6870f7c28 100644
--- a/lib/diameter/doc/src/diameter_app.xml
+++ b/lib/diameter/doc/src/diameter_app.xml
@@ -382,7 +382,7 @@ communicate transport (or any other) data to the callback.</p>
 <p>
 A returned <seealso marker="#packet">packet()</seealso> can set
 the <c>header</c> field to a
-<c>#diameter_header{}</c> in order to specify values that should
+<c>#diameter_header{}</c> to specify values that should
 be preserved in the outgoing request, values otherwise being those in
 the header record contained in <c>Packet</c>.
 A returned <c>length</c>, <c>cmd_code</c> or <c>application_id</c> is
@@ -537,7 +537,8 @@ not selected.</p>
            | {relay, [Opt]}
            | discard
            | {eval|eval_packet, Action, PostF}</v>
-<v>Reply   = {reply, <seealso marker="#message">message()</seealso>}
+<v>Reply   = {reply, <seealso marker="#packet">packet()</seealso>
+                   | <seealso marker="#message">message()</seealso>}
            | {protocol_error, 3000..3999}</v>
 <v>Opt     = <seealso marker="diameter#call_opt">diameter:call_opt()</seealso></v>
 <v>PostF   = <seealso marker="diameter#evaluable">diameter:evaluable()</seealso></v>
@@ -568,7 +569,7 @@ The argument <seealso marker="#packet">packet()</seealso> has the following sign
 </code>
 
 <p>
-The <c>msg</c> field will be <c>undefined</c> only in case the request has
+The <c>msg</c> field will be <c>undefined</c> in case the request has
 been received in the relay application.
 Otherwise it contains the record representing the request as outlined
 in <seealso
@@ -590,19 +591,26 @@ the relay application.</p>
 The <c>transport_data</c> field contains an arbitrary term passed into
 diameter from the transport module in question, or the atom
 <c>undefined</c> if the transport specified no data.
-The term is preserved in the <seealso marker="#packet">packet()</seealso> containing any answer message
-sent back to the transport process unless another value is explicitly
-specified.</p>
+The term is preserved if a <seealso
+marker="#packet">message()</seealso> is returned but must be set
+explicitly in a returned <seealso marker="#packet">packet()</seealso>.</p>
 
 <p>
 The semantics of each of the possible return values are as follows.</p>
 
 <taglist>
 
-<tag><c>{reply, <seealso marker="#message">message()</seealso>}</c></tag>
+<tag><c>{reply, <seealso marker="#packet">packet()</seealso>
+              | <seealso marker="#message">message()</seealso>}</c></tag>
 <item>
 <p>
-Send the specified answer message to the peer.</p>
+Send the specified answer message to the peer.
+In the case of a <seealso marker="#packet">packet()</seealso>, the
+message to be sent must be set in the
+<c>msg</c> field and the <c>header</c> field can be set to a
+<c>#diameter_header{}</c> to specify values that should be
+preserved in the outgoing answer, appropriate values otherwise
+being set by diameter.</p>
 </item>
 
 <tag><c>{protocol_error, 3000..3999}</c></tag>
diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl
index 395d0b77af..29046e6462 100644
--- a/lib/diameter/src/base/diameter_service.erl
+++ b/lib/diameter/src/base/diameter_service.erl
@@ -2177,15 +2177,13 @@ reply([Msg], Dict, TPid, Fs, Pkt)
     reply(Msg, Dict, TPid, Fs, Pkt#diameter_packet{errors = []});
 
 %% No errors or a diameter_header/avp list.
-reply(Msg, Dict, TPid, Fs, #diameter_packet{errors = Es,
-                                            transport_data = TD}
-                           = ReqPkt)
+reply(Msg, Dict, TPid, Fs, #diameter_packet{errors = Es} = ReqPkt)
   when [] == Es;
        is_record(hd(Msg), diameter_header) ->
     Pkt = diameter_codec:encode(Dict, make_answer_packet(Msg, ReqPkt)),
     eval_packet(Pkt, Fs),
     incr(send, Pkt, Dict, TPid),  %% count result codes in sent answers
-    send(TPid, Pkt#diameter_packet{transport_data = TD});
+    send(TPid, Pkt);
 
 %% Or not: set Result-Code and Failed-AVP AVP's.
 reply(Msg, Dict, TPid, Fs, #diameter_packet{errors = [H|_] = Es} = Pkt) ->
@@ -2200,23 +2198,36 @@ eval_packet(Pkt, Fs) ->
     
 %% make_answer_packet/2
 
+%% A reply message clears the R and T flags and retains the P flag.
+%% The E flag will be set at encode. 6.2 of 3588 requires the same P
+%% flag on an answer as on the request. A #diameter_packet{} returned
+%% from a handle_request callback can circumvent this by setting its
+%% own header values.
+make_answer_packet(#diameter_packet{header = Hdr,
+                                    msg = Msg,
+                                    transport_data = TD},
+                   #diameter_packet{header = ReqHdr}) ->
+    Hdr0 = ReqHdr#diameter_header{version = ?DIAMETER_VERSION,
+                                  is_request = false,
+                                  is_error = undefined,
+                                  is_retransmitted = false},
+    #diameter_packet{header = fold_record(Hdr0, Hdr),
+                     msg = Msg,
+                     transport_data = TD};
+
 %% Binaries and header/avp lists are sent as-is.
-make_answer_packet(Bin, _)
+make_answer_packet(Bin, #diameter_packet{transport_data = TD})
   when is_binary(Bin) ->
-    #diameter_packet{bin = Bin};
-make_answer_packet([#diameter_header{} | _] = Msg, _) ->
-    #diameter_packet{msg = Msg};
-
-%% Otherwise a reply message clears the R and T flags and retains the
-%% P flag. The E flag will be set at encode. 6.2 of 3588 requires the
-%% same P flag on an answer as on the request.
-make_answer_packet(Msg, #diameter_packet{header = ReqHdr}) ->
-    Hdr = ReqHdr#diameter_header{version = ?DIAMETER_VERSION,
-                                 is_request = false,
-                                 is_error = undefined,
-                                 is_retransmitted = false},
-    #diameter_packet{header = Hdr,
-                     msg = Msg}.
+    #diameter_packet{bin = Bin,
+                     transport_data = TD};
+make_answer_packet([#diameter_header{} | _] = Msg,
+                   #diameter_packet{transport_data = TD}) ->
+    #diameter_packet{msg = Msg,
+                     transport_data = TD};
+
+%% Otherwise, preserve transport_data.
+make_answer_packet(Msg, #diameter_packet{transport_data = TD} = Pkt) ->
+    make_answer_packet(#diameter_packet{msg = Msg, transport_data = TD}, Pkt).
 
 %% rc/1
 
diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl
index 5744ff0307..fa9333a226 100644
--- a/lib/diameter/test/diameter_traffic_SUITE.erl
+++ b/lib/diameter/test/diameter_traffic_SUITE.erl
@@ -38,6 +38,7 @@
          result_codes/1,
          send_ok/1,
          send_nok/1,
+         send_bad_answer/1,
          send_arbitrary/1,
          send_unknown/1,
          send_unknown_mandatory/1,
@@ -208,6 +209,7 @@ end_per_testcase(_, _) ->
 tc() ->
     [send_ok,
      send_nok,
+     send_bad_answer,
      send_arbitrary,
      send_unknown,
      send_unknown_mandatory,
@@ -308,6 +310,14 @@ send_nok(Config) ->
     #'diameter_base_answer-message'{'Result-Code' = ?INVALID_AVP_BITS}
         = call(Config, Req).
 
+%% Send an accounting ACR that the server tries to answer with an
+%% inappropriate header, resulting in no answer being sent and the
+%% request timing out.
+send_bad_answer(Config) ->
+    Req = ['ACR', {'Accounting-Record-Type', ?EVENT_RECORD},
+                  {'Accounting-Record-Number', 2}],
+    {error, timeout} = call(Config, Req).
+
 %% Send an ASR with an arbitrary AVP and expect success and the same
 %% AVP in the reply.
 send_arbitrary(Config) ->
@@ -770,6 +780,21 @@ request(#diameter_base_accounting_ACR{'Accounting-Record-Number' = 0},
 
 request(#diameter_base_accounting_ACR{'Session-Id' = SId,
                                       'Accounting-Record-Type' = RT,
+                                      'Accounting-Record-Number' = 2 = RN},
+        #diameter_caps{origin_host = {OH, _},
+                       origin_realm = {OR, _}}) ->
+    Ans = ['ACA', {'Result-Code', ?SUCCESS},
+                  {'Session-Id', SId},
+                  {'Origin-Host', OH},
+                  {'Origin-Realm', OR},
+                  {'Accounting-Record-Type', RT},
+                  {'Accounting-Record-Number', RN}],
+
+    {reply, #diameter_packet{header = #diameter_header{is_error = true},%% not
+                             msg = Ans}};
+
+request(#diameter_base_accounting_ACR{'Session-Id' = SId,
+                                      'Accounting-Record-Type' = RT,
                                       'Accounting-Record-Number' = RN},
         #diameter_caps{origin_host = {OH, _},
                        origin_realm = {OR, _}}) ->