Merge branch 'anders/diameter/5xxx_errors/OTP-10759'

* anders/diameter/5xxx_errors/OTP-10759:
  Add testcases for application_opt() request_errors = answer
  Answer 5xxx errors with application_opt() request_errors = answer
  Comments and minor Result-Code fix

7 files changed, 400 insertions, 222 deletions

diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml
index accf21cf98..379e9f0738 100644
--- a/lib/diameter/doc/src/diameter.xml
+++ b/lib/diameter/doc/src/diameter.xml
@@ -188,7 +188,7 @@ Defaults to the value of the <c>alias</c> option if unspecified.</p>
 <item>
 <p>
 Specifies whether or not the &app_pick_peer;
-application callback can modify the application state,
+application callback can modify the application state.
 Defaults to <c>false</c> if unspecified.</p>
 
 <note>
@@ -226,22 +226,37 @@ question is as if a callback had taken place and returned
 Defaults to <c>report</c> if unspecified.</p>
 </item>
 
-<tag><c>{request_errors, answer_3xxx|callback}</c></tag>
+<tag><c>{request_errors, answer_3xxx|answer|callback}</c></tag>
 <item>
 <p>
 Determines the manner in which incoming requests are handled when an
-error other than 3007, DIAMETER_APPLICATION_UNSUPPORTED. (With which no
-application callback module can be associated.)</p>
+error other than 3007, DIAMETER_APPLICATION_UNSUPPORTED (which cannot
+be associated with an application callback module), is detected.</p>
 
 <p>
-If <c>answer_3xxx</c> then the request is answered by diameter
-without a &app_handle_request; callback taking place if a 3xxx series
-error (protocol errors) is detected.
-If <c>callback</c> then even 3xxx errors result in an application
-&app_handle_request; callback.</p>
+If <c>answer_3xxx</c> then requests are answered without a
+&app_handle_request; callback taking place.
+If <c>answer</c> then even 5xxx errors are answered without a
+callback unless the connection in question has configured the RFC 3588
+common dictionary as noted below.
+If <c>callback</c> then a &app_handle_request; callback always takes
+place and the return value determines the answer sent to the peer.</p>
 
 <p>
 Defaults to <c>answer_3xxx</c> if unspecified.</p>
+
+<note>
+<p>
+Answers sent by diameter set the E-bit in the Diameter Header.
+Since RFC 3588 allowed only 3xxx result codes in an
+<c>answer-message</c>, <c>answer</c> has the same semantics as
+<c>answer_3xxx</c> if the peer connection in question has configured
+the RFC 3588 common dictionary, <c>diameter_gen_base_rfc3588</c>.
+RFC 6733 allows both 3xxx and 5xxx result codes in an
+<c>answer-message</c> so a connection configured with the RFC 6733
+common dictionary, <c>diameter_gen_base_rfc6733</c>, does
+distinguish between <c>answer_3xxx</c> and <c>answer</c>.</p>
+</note>
 </item>
 
 </taglist>
diff --git a/lib/diameter/doc/src/diameter_app.xml b/lib/diameter/doc/src/diameter_app.xml
index 5c23c9d683..d0f1b22ebd 100644
--- a/lib/diameter/doc/src/diameter_app.xml
+++ b/lib/diameter/doc/src/diameter_app.xml
@@ -475,6 +475,7 @@ not selected.</p>
            | discard
            | {eval|eval_packet, Action, PostF}</v>
 <v>Reply   = {reply, &packet; | &message;}
+           | {answer_message, 3000..3999|5000..5999}
            | {protocol_error, 3000..3999}</v>
 <v>Opt     = &mod_call_opt;</v>
 <v>PostF   = &mod_evaluable;</v>
@@ -545,22 +546,25 @@ preserved in the outgoing answer, appropriate values otherwise
 being set by diameter.</p>
 </item>
 
-<tag><c>{protocol_error, 3000..3999}</c></tag>
+<tag><c>{answer_message, 3000..3999|5000..5999}</c></tag>
 <item>
 <p>
 Send an answer message to the peer containing the specified
-protocol error.
+Result-Code.
 Equivalent to</p>
 <pre>
 {reply, ['answer-message' | Avps]
 </pre>
 <p>
 where <c>Avps</c> sets the Origin-Host, Origin-Realm, the specified
-Result-Code and (if the request sent one) Session-Id AVP's.</p>
+Result-Code and (if the request contained one) Session-Id AVP's.</p>
 
 <p>
-Returning a non-3xxx value in a <c>protocol_error</c> tuple
-will cause the request process in question to fail.</p>
+Returning a value other than 3xxx or 5xxx will cause the request
+process in question to fail, as will returning a 5xxx value if the
+peer connection in question has been configured with the RFC 3588
+common dictionary <c>diameter_gen_base_rfc3588</c>.
+(Since RFC 3588 only allows 3xxx values in an answer-message.)</p>
 </item>
 
 <tag><c>{relay, Opts}</c></tag>
@@ -613,11 +617,20 @@ containing the encoded binary.
 The return value is ignored.</p>
 </item>
 
+<tag><c>{protocol_error, 3000..3999}</c></tag>
+<item>
+<p>
+Equivalent to <c>{answer_message, 3000..3999}</c>.</p>
+</item>
+
 </taglist>
 
+<note>
 <p>
-Note that protocol errors detected by diameter will result in an
-answer message without <c>handle_request/3</c> being invoked.</p>
+Requests containing errors may be answered by diameter, without a
+callback taking place, depending on the value of the
+&mod_application_opt; <c>request_errors</c>.</p>
+</note>
 
 </desc>
 </func>
diff --git a/lib/diameter/include/diameter.hrl b/lib/diameter/include/diameter.hrl
index 513665cec1..79c4dce541 100644
--- a/lib/diameter/include/diameter.hrl
+++ b/lib/diameter/include/diameter.hrl
@@ -144,5 +144,5 @@
          id,         %% 32-bit unsigned application identifier = Dict:id()
          mutable = false, %% boolean(), do traffic callbacks modify state?
          options = [{answer_errors, report},      %% | callback | discard
-                    {request_errors, answer_3xxx}]}). %% | callback
+                    {request_errors, answer_3xxx}]}). %% | callback | answer
 -endif. %% -ifdef(diameter_hrl).
diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl
index 7359688404..c67fba5f89 100644
--- a/lib/diameter/src/base/diameter.erl
+++ b/lib/diameter/src/base/diameter.erl
@@ -307,7 +307,7 @@ call(SvcName, App, Message) ->
     | {state, any()}
     | {call_mutates_state, boolean()}
     | {answer_errors, callback|report|discard}
-    | {request_errors, callback|answer_3xxx}.
+    | {request_errors, answer_3xxx|answer|callback}.
 
 -type app_alias()
    :: any().
diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl
index 889c75e3da..9f73815756 100644
--- a/lib/diameter/src/base/diameter_config.erl
+++ b/lib/diameter/src/base/diameter_config.erl
@@ -670,17 +670,17 @@ app_acc({application, Opts}, Acc) ->
     [Dict, Mod] = get_opt([dictionary, module], Opts),
     Alias = get_opt(alias, Opts, Dict),
     ModS  = get_opt(state, Opts, Alias),
-    M = get_opt(call_mutates_state, Opts, false),
-    A = get_opt(answer_errors, Opts, report),
-    R = get_opt(request_errors, Opts, answer_3xxx),
+    M = get_opt(call_mutates_state, Opts, false, [true]),
+    A = get_opt(answer_errors, Opts, report, [callback, discard]),
+    P = get_opt(request_errors, Opts, answer_3xxx, [answer, callback]),
     [#diameter_app{alias = Alias,
                    dictionary = Dict,
                    id = cb(Dict, id),
                    module = init_mod(Mod),
                    init_state = ModS,
-                   mutable = init_mutable(M),
-                   options = [{answer_errors, init_answers(A)},
-                              {request_errors, init_request_errors(R)}]}
+                   mutable = M,
+                   options = [{answer_errors, A},
+                              {request_errors, P}]}
      | Acc];
 app_acc(_, Acc) ->
     Acc.
@@ -709,27 +709,16 @@ init_cb(List) ->
                    V <- [proplists:get_value(F, List, D)]],
     #diameter_callback{} = list_to_tuple([diameter_callback | Values]).
 
-init_mutable(M)
-  when M == true;
-       M == false ->
-    M;
-init_mutable(M) ->
-    ?THROW({call_mutates_state, M}).
-
-init_answers(A)
-  when callback == A;
-       report == A;
-       discard == A ->
-    A;
-init_answers(A) ->
-    ?THROW({answer_errors, A}).
-
-init_request_errors(P)
-  when callback == P;
-       answer_3xxx == P ->
-    P;
-init_request_errors(P) ->
-    ?THROW({request_errors, P}).
+%% Retreive and validate.
+get_opt(Key, List, Def, Other) ->
+    init_opt(Key, get_opt(Key, List, Def), [Def|Other]).
+
+init_opt(_, V, [V|_]) ->
+    V;
+init_opt(Name, V, [_|Vals]) ->
+    init_opt(Name, V, Vals);
+init_opt(Name, V, []) ->
+    ?THROW({Name, V}).
 
 %% Get a single value at the specified key.
 get_opt(Keys, List)
diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl
index d6cb5ff0fa..f527f7c754 100644
--- a/lib/diameter/src/base/diameter_traffic.erl
+++ b/lib/diameter/src/base/diameter_traffic.erl
@@ -190,19 +190,21 @@ recv_request(TPid,
     send_A(recv_R(diameter_service:find_incoming_app(PeerT, TPid, Id, Apps),
                   TPid,
                   Pkt,
+                  Dict0,
                   RecvData),
            TPid,
            Dict0,
            RecvData).
 
-%% recv_R/4
+%% recv_R/5
 
 recv_R({#diameter_app{id = Id, dictionary = Dict} = App, Caps},
        TPid,
        Pkt0,
+       Dict0,
        RecvData) ->
     Pkt = errors(Id, diameter_codec:decode(Id, Dict, Pkt0)),
-    {Caps, Pkt, App, recv_R(App, TPid, Caps, RecvData, Pkt)};
+    {Caps, Pkt, App, recv_R(App, TPid, Dict0, Caps, RecvData, Pkt)};
 %% Note that the decode is different depending on whether or not Id is
 %% ?APP_ID_RELAY.
 
@@ -214,11 +216,12 @@ recv_R(#diameter_caps{}
        _TPid,
        #diameter_packet{errors = Es}
        = Pkt,
+       _Dict0,
        _RecvData) ->
     {Caps, Pkt#diameter_packet{avps = collect_avps(Pkt),
                                errors = [3007 | Es]}};
 
-recv_R(false = No, _, _, _) ->  %% transport has gone down
+recv_R(false = No, _, _, _, _) ->  %% transport has gone down
     No.
 
 collect_avps(Pkt) ->
@@ -229,21 +232,24 @@ collect_avps(Pkt) ->
             As
     end.
 
-%% recv_R/5
+%% recv_R/6
 
-%% Answer 3xxx errors ourselves ...
-recv_R(#diameter_app{options = [_, {request_errors, answer_3xxx} | _]},
+%% Answer errors ourselves ...
+recv_R(#diameter_app{options = [_, {request_errors, E} | _]},
        _TPid,
+       Dict0,
        _Caps,
        _RecvData,
        #diameter_packet{errors = [RC|_]})  %% a detected 3xxx is hd
-  when 3 == RC div 1000 ->
-    {{protocol_error, RC}, [], []};
+  when E == answer, (Dict0 /= ?BASE orelse 3 == RC div 1000);
+       E == answer_3xxx, 3 == RC div 1000 ->
+    {{answer_message, rc(RC)}, [], []};
 
 %% ... or make a handle_request callback. Note that
 %% Pkt#diameter_packet.msg = undefined in the 3001 case.
 recv_R(App,
        TPid,
+       _Dict0,
        Caps,
        #recvdata{service_name = SvcName},
        Pkt) ->
@@ -252,6 +258,11 @@ recv_R(App,
                [],
                []).
 
+rc({N,_}) ->
+    N;
+rc(N) ->
+    N.
+
 %% errors/1
 %%
 %% Look for additional errors in a decoded message, prepending the
@@ -331,8 +342,13 @@ request_cb({reply, _Ans} = T, _App, EvalPktFs, EvalFs) ->
     {T, EvalPktFs, EvalFs};
 
 %% An 3xxx result code, for which the E-bit is set in the header.
-request_cb({protocol_error, RC} = T, _App, EvalPktFs, EvalFs)
+request_cb({protocol_error, RC}, _App, EvalPktFs, EvalFs)
   when 3 == RC div 1000 ->
+    {{answer_message, RC}, EvalPktFs, EvalFs};
+
+request_cb({answer_message, RC} = T, _App, EvalPktFs, EvalFs)
+  when 3 == RC div 1000;
+       5 == RC div 1000 ->
     {T, EvalPktFs, EvalFs};
 
 %% RFC 3588 says we must reply 3001 to anything unrecognized or
@@ -340,7 +356,7 @@ request_cb({protocol_error, RC} = T, _App, EvalPktFs, EvalFs)
 %% backwards compatibility for this, protocol_error the documented
 %% alternative.
 request_cb(noreply, _App, EvalPktFs, EvalFs) ->
-    {{protocol_error, 3001}, EvalPktFs, EvalFs};
+    {{answer_message, 3001}, EvalPktFs, EvalFs};
 
 %% Relay a request to another peer. This is equivalent to doing an
 %% explicit call/4 with the message in question except that (1) a loop
@@ -372,15 +388,16 @@ request_cb({eval_packet, RC, F}, App, Fs, EvalFs) ->
 request_cb({eval, RC, F}, App, EvalPktFs, Fs) ->
     request_cb(RC, App, EvalPktFs, [F|Fs]);
 
-request_cb(T, #diameter_app{module = ModX}, _, _) ->
-    ?ERROR({invalid_return, T, {ModX, handle_request}}).
+request_cb(T, App, _, _) ->
+    ?ERROR({invalid_return, T, handle_request, App}).
 
 %% send_A/4
 
 send_A({Caps, Pkt}, TPid, Dict0, _RecvData) ->  %% unsupported application
     #diameter_packet{errors = [RC|_]} = Pkt,
-    send_A(protocol_error(RC, Caps, Dict0, Pkt),
+    send_A(answer_message(RC, Caps, Dict0, Pkt),
            TPid,
+           Dict0,
            Pkt,
            [],
            []);
@@ -388,6 +405,7 @@ send_A({Caps, Pkt}, TPid, Dict0, _RecvData) ->  %% unsupported application
 send_A({Caps, Pkt, App, {T, EvalPktFs, EvalFs}}, TPid, Dict0, RecvData) ->
     send_A(answer(T, Caps, Pkt, App, Dict0, RecvData),
            TPid,
+           Dict0,
            Pkt,
            EvalPktFs,
            EvalFs);
@@ -395,10 +413,10 @@ send_A({Caps, Pkt, App, {T, EvalPktFs, EvalFs}}, TPid, Dict0, RecvData) ->
 send_A(_, _, _, _) ->
     ok.
 
-%% send_A/5
+%% send_A/6
 
-send_A(T, TPid, ReqPkt, EvalPktFs, EvalFs) ->
-    reply(T, TPid, EvalPktFs, ReqPkt),
+send_A(T, TPid, Dict0, ReqPkt, EvalPktFs, EvalFs) ->
+    reply(T, TPid, Dict0, EvalPktFs, ReqPkt),
     lists:foreach(fun diameter_lib:eval/1, EvalFs).
 
 %% answer/6
@@ -420,8 +438,12 @@ answer({call, Opts}, Caps, Pkt, App, Dict0, RecvData) ->
            Dict0,
            RecvData);
 
-answer({protocol_error, RC}, Caps, Pkt, _App, Dict0, _RecvData) ->
-    protocol_error(RC, Caps, Dict0, Pkt).
+%% RFC 3588 only allows 3xxx errors in an answer-message. RFC 6733
+%% added the possibility of setting 5xxx.
+answer({answer_message, RC} = T, Caps, Pkt, App, Dict0, _RecvData)  ->
+    Dict0 /= ?BASE orelse 3 == RC div 1000
+        orelse ?ERROR({invalid_return, T, handle_request, App}),
+    answer_message(RC, Caps, Dict0, Pkt).
 
 %% dict/3
 
@@ -451,9 +473,9 @@ is_answer_message(Rec, Dict) ->
         error:_ -> false
     end.
 
-%% protocol_error/4
+%% answer_message/4
 
-protocol_error(RC,
+answer_message(RC,
                #diameter_caps{origin_host  = {OH,_},
                               origin_realm = {OR,_}},
                Dict0,
@@ -471,7 +493,7 @@ protocol_error(RC,
 %%      identified a configuration problem.
 
 resend(true, _Opts, Caps, Pkt, _App, Dict0, _RecvData) ->
-    protocol_error(3005, Caps, Dict0, Pkt);
+    answer_message(3005, Caps, Dict0, Pkt);
 
 %% 6.1.8.  Relaying and Proxying Requests
 %%
@@ -528,7 +550,7 @@ resend(#diameter_packet{bin = B}
 
 %% Or not: DIAMETER_UNABLE_TO_DELIVER.
 resend(_, Caps, Dict0, Pkt) ->
-    protocol_error(3002, Caps, Dict0, Pkt).
+    answer_message(3002, Caps, Dict0, Pkt).
 
 %% is_loop/5
 %%
@@ -551,32 +573,32 @@ is_loop(Code, Vid, OH, Dict0, [_ | Avps])
 is_loop(Code, Vid, OH, Dict0, Avps) ->
     is_loop(Code, Vid, Dict0:avp(encode, OH, 'Route-Record'), Dict0, Avps).
 
-%% reply/4
+%% reply/5
 
 %% Local answer ...
-reply({Dict, Ans}, TPid, Fs, ReqPkt) ->
-    reply(Ans, Dict, TPid, Fs, ReqPkt);
+reply({Dict, Ans}, TPid, Dict0, Fs, ReqPkt) ->
+    reply(Ans, Dict, TPid, Dict0, Fs, ReqPkt);
 
 %% ... or relayed.
-reply(#diameter_packet{} = Pkt, TPid, Fs, _ReqPkt) ->
+reply(#diameter_packet{} = Pkt, TPid, _Dict0, Fs, _ReqPkt) ->
     eval_packet(Pkt, Fs),
     send(TPid, Pkt).
 
-%% reply/5
+%% reply/6
 %%
 %% Send a locally originating reply.
 
 %% Skip the setting of Result-Code and Failed-AVP's below. This is
 %% undocumented and shouldn't be relied on.
-reply([Msg], Dict, TPid, Fs, ReqPkt)
+reply([Msg], Dict, TPid, Dict0, Fs, ReqPkt)
   when is_list(Msg);
        is_tuple(Msg) ->
-    reply(Msg, Dict, TPid, Fs, ReqPkt#diameter_packet{errors = []});
+    reply(Msg, Dict, TPid, Dict0, Fs, ReqPkt#diameter_packet{errors = []});
 
 %% No errors or a diameter_header/avp list.
-reply(Msg, Dict, TPid, Fs, ReqPkt) ->
+reply(Msg, Dict, TPid, Dict0, Fs, ReqPkt) ->
     Pkt = encode(Dict, reset(make_answer_packet(Msg, ReqPkt), Dict), Fs),
-    incr(send, Pkt, Dict, TPid),  %% count result codes in sent answers
+    incr(send, Pkt, Dict, TPid, Dict0),  %% count outgoing result codes
     send(TPid, Pkt).
 
 %% reset/2
@@ -910,32 +932,48 @@ find(Pred, [H|T]) ->
 
 %% incr/4
 %%
-%% Increment a stats counter for an incoming or outgoing message.
+%% Increment a stats counter for result codes in incoming and outgoing
+%% answers.
 
 %% Outgoing message as binary: don't count. (Sending binaries is only
 %% partially supported.)
-incr(_, #diameter_packet{msg = undefined}, _, _) ->
+incr(_, #diameter_packet{msg = undefined}, _, _, _) ->
     ok;
 
-incr(recv = D, #diameter_packet{header = H, errors = [_|_]}, _, TPid) ->
+%% Incoming with decode errors.
+incr(recv = D, #diameter_packet{header = H, errors = [_|_]}, _, TPid, _) ->
     incr(TPid, {diameter_codec:msg_id(H), D, error});
 
-incr(Dir, Pkt, Dict, TPid) ->
+%% Incoming without errors or outgoing. Outgoing with encode errors
+%% never gets here since encode fails.
+incr(Dir, Pkt, Dict, TPid, Dict0) ->
     #diameter_packet{header = #diameter_header{is_error = E}
                             = Hdr,
                      msg = Rec}
         = Pkt,
 
     RC = int(get_avp_value(Dict, 'Result-Code', Rec)),
-    PE = is_protocol_error(RC),
 
-    %% Check that the E bit is set only for 3xxx result codes.
-    (not (E orelse PE))
-        orelse (E andalso PE)
+    %% Exit on an improper Result-Code.
+    is_result(RC, E, Dict0)
         orelse x({invalid_error_bit, RC}, answer, [Dir, Pkt]),
 
     irc(TPid, Hdr, Dir, rc_counter(Dict, Rec, RC)).
 
+%% No E-bit: can't be 3xxx.
+is_result(RC, false, _Dict0) ->
+    RC < 3000 orelse 4000 =< RC;
+
+%% E-bit in RFC 3588: only 3xxx.
+is_result(RC, true, ?BASE) ->
+    3000 =< RC andalso RC < 4000;
+
+%% E-bit in RFC 6733: 3xxx or 5xxx.
+is_result(RC, true, _) ->
+    3000 =< RC andalso RC < 4000
+        orelse
+        5000 =< RC andalso RC < 6000.
+
 irc(_, _, _, undefined) ->
     false;
 
@@ -947,7 +985,7 @@ irc(TPid, Hdr, Dir, Ctr) ->
 incr(TPid, Counter) ->
     diameter_stats:incr(Counter, TPid, 1).
 
-%% error_counter/2
+%% rc_counter/2
 
 %% RFC 3588, 7.6:
 %%
@@ -987,9 +1025,6 @@ int(N)
 int(_) ->
     undefined.
 
-is_protocol_error(RC) ->
-    3000 =< RC andalso RC < 4000.
-
 -spec x(any(), atom(), list()) -> no_return().
 
 %% Warn and exit request process on errors in an incoming answer.
@@ -1139,7 +1174,7 @@ send_R({eval_packet, RC, F}, Pkt, T, Opts, Caller, SvcName, Fs) ->
     send_R(RC, Pkt, T, Opts, Caller, SvcName, [F|Fs]);
 
 send_R(E, _, {_, _, App}, _, _, _, _) ->
-    ?ERROR({invalid_return, prepare_request, App, E}).
+    ?ERROR({invalid_return, E, prepare_request, App}).
 
 %% make_prepare_packet/2
 %%
@@ -1291,24 +1326,28 @@ handle_answer(SvcName,
               = App,
               {answer, Req, Dict0, Pkt}) ->
     Mod = dict(Dict, Dict0, Pkt),
-    answer(errors(Id, diameter_codec:decode(Mod, Pkt)),
-           SvcName,
-           Mod,
-           App,
-           Req).
+    handle_A(errors(Id, diameter_codec:decode(Mod, Pkt)),
+             SvcName,
+             Mod,
+             Dict0,
+             App,
+             Req).
 
 %% We don't really need to do a full decode if we're a relay and will
 %% just resend with a new hop by hop identifier, but might a proxy
 %% want to examine the answer?
 
-answer(Pkt, SvcName, Dict, App, #request{transport = TPid} = Req) ->
+handle_A(Pkt, SvcName, Dict, Dict0, App, #request{transport = TPid} = Req) ->
     try
-        incr(recv, Pkt, Dict, TPid)
+        incr(recv, Pkt, Dict, TPid, Dict0) %% count incoming result codes
     of
         _ -> answer(Pkt, SvcName, App, Req)
     catch
-        exit: {invalid_error_bit, _} = E ->
-            answer(Pkt#diameter_packet{errors = [E]}, SvcName, App, Req)
+        exit: {invalid_error_bit, RC} ->
+            #diameter_packet{errors = Es}
+                = Pkt,
+            E = {5004, #diameter_avp{name = 'Result-Code', value = RC}},
+            answer(Pkt#diameter_packet{errors = [E|Es]}, SvcName, App, Req)
     end.
 
 answer(Pkt,
@@ -1498,7 +1537,7 @@ retransmit({eval_packet, RC, F}, Transport, Req, SvcName, Timeout, Fs) ->
     retransmit(RC, Transport, Req, SvcName, Timeout, [F|Fs]);
 
 retransmit(T, {_, _, App}, _, _, _, _) ->
-    ?ERROR({invalid_return, prepare_retransmit, App, T}).
+    ?ERROR({invalid_return, T, prepare_retransmit, App}).
 
 resend_request(Pkt0,
                {TPid, Caps, #diameter_app{dictionary = Dict}},
diff --git a/lib/diameter/test/diameter_3xxx_SUITE.erl b/lib/diameter/test/diameter_3xxx_SUITE.erl
index c780fd9859..89c78d8b57 100644
--- a/lib/diameter/test/diameter_3xxx_SUITE.erl
+++ b/lib/diameter/test/diameter_3xxx_SUITE.erl
@@ -37,10 +37,15 @@
 
 %% testcases
 -export([start/1,
+         send_unknown_application/1,
          send_unknown_command/1,
-         send_ok_override/1,
+         send_ok/1,
          send_invalid_avp_bits/1,
+         send_missing_avp/1,
+         send_ignore_missing_avp/1,
          send_double_error/1,
+         send_3xxx/1,
+         send_5xxx/1,
          stop/1]).
 
 %% diameter callbacks
@@ -54,58 +59,59 @@
 
 -include("diameter.hrl").
 -include("diameter_gen_base_rfc6733.hrl").
+%% Use the fact that STR/STA is identical in RFC's 3588 and 6733.
 
 %% ===========================================================================
 
 -define(util, diameter_util).
+-define(testcase(), proplists:get_value(testcase, get(?MODULE))).
+-define(group(Config), begin
+                           put(?MODULE, Config),
+                           ?util:name(proplists:get_value(group, Config))
+                       end).
+
+-define(L, atom_to_list).
+-define(A, list_to_atom).
 
 -define(CLIENT, "CLIENT").
 -define(SERVER, "SERVER").
 -define(REALM, "erlang.org").
 -define(HOST(Host, Realm), Host ++ [$.|Realm]).
--define(DICT, diameter_gen_base_rfc6733).
+
+-define(ERRORS, [answer, answer_3xxx, callback]).
+-define(RFCS, [rfc3588, rfc6733]).
+-define(DICT(RFC), ?A("diameter_gen_base_" ++ ?L(RFC))).
+-define(DICT, ?DICT(rfc6733)).
+
+-define(COMMON, ?DIAMETER_APP_ID_COMMON).
 
 %% Config for diameter:start_service/2.
--define(SERVICE(Name, RequestErrors),
+-define(SERVICE(Name, Errors, RFC),
         [{'Origin-Host', Name ++ "." ++ ?REALM},
          {'Origin-Realm', ?REALM},
          {'Host-IP-Address', [{127,0,0,1}]},
          {'Vendor-Id', 12345},
          {'Product-Name', "OTP/diameter"},
-         {'Auth-Application-Id', [?DIAMETER_APP_ID_COMMON]},
-         {application, [{dictionary, ?DICT},
+         {'Auth-Application-Id', [?COMMON]},
+         {application, [{dictionary, ?DICT(RFC)},
                         {module, ?MODULE},
                         {answer_errors, callback},
-                        {request_errors, RequestErrors}]}]).
-
--define(SUCCESS,              2001).
--define(UNSUPPORTED_COMMAND,  3001).
--define(INVALID_AVP_BITS,     3009).
--define(UNKNOWN_SESSION_ID,   5002).
--define(MISSING_AVP,          5005).
+                        {request_errors, Errors}]}]).
 
 -define(LOGOUT, ?'DIAMETER_BASE_TERMINATION-CAUSE_LOGOUT').
 
--define(GROUPS, [answer_3xxx, callback]).
--define(L, atom_to_list).
--define(A, list_to_atom).
--define(v, proplists:get_value).
-
--define(testcase(Config), put({?MODULE, testcase}, ?v(testcase, Config))).
--define(testcase(), get({?MODULE, testcase})).
--define(group(Config), ?v(group, Config)).
-
 %% ===========================================================================
 
 suite() ->
     [{timetrap, {seconds, 60}}].
 
 all() ->
-    [{group, G} || G <- ?GROUPS].
+    [{group, ?util:name([E,D])} || E <- ?ERRORS, D <- ?RFCS].
 
 groups() ->
     Tc = tc(),
-    [{G, [], [start] ++ Tc ++ [stop]} || G <- ?GROUPS].
+    [{?util:name([E,D]), [], [start] ++ Tc ++ [stop]}
+     || E <- ?ERRORS, D <- ?RFCS].
 
 init_per_suite(Config) ->
     ok = diameter:start(),
@@ -126,17 +132,16 @@ init_per_testcase(Name, Config) ->
 end_per_testcase(_, _) ->
     ok.
 
-origin(answer_3xxx)   -> 0;
-origin(callback) -> 1;
-
-origin(0) -> answer_3xxx;
-origin(1) -> callback.
-
 tc() ->
-    [send_unknown_command,
-     send_ok_override,
+    [send_unknown_application,
+     send_unknown_command,
+     send_ok,
      send_invalid_avp_bits,
-     send_double_error].
+     send_missing_avp,
+     send_ignore_missing_avp,
+     send_double_error,
+     send_3xxx,
+     send_5xxx].
 
 %% ===========================================================================
 
@@ -144,13 +149,15 @@ tc() ->
 
 start(Config) ->
     Group = proplists:get_value(group, Config),
-    ok = diameter:start_service(?SERVER, ?SERVICE(?L(Group), Group)),
-    ok = diameter:start_service(?CLIENT, ?SERVICE(?CLIENT, callback)),
+    [Errors, RFC] = ?util:name(Group),
+    ok = diameter:start_service(?SERVER, ?SERVICE(?L(Group),
+                                                  Errors,
+                                                  RFC)),
+    ok = diameter:start_service(?CLIENT, ?SERVICE(?CLIENT,
+                                                  callback,
+                                                  rfc6733)),
     LRef = ?util:listen(?SERVER, tcp),
-    ?util:connect(?CLIENT,
-                  tcp,
-                  LRef,
-                  [{capabilities, [{'Origin-State-Id', origin(Group)}]}]).
+    ?util:connect(?CLIENT, tcp, LRef).
 
 %% stop/1
 
@@ -160,79 +167,174 @@ stop(_Config) ->
     ok = diameter:stop_service(?SERVER),
     ok = diameter:stop_service(?CLIENT).
 
+%% send_unknown_application/1
+%%
+%% Send an unknown application that a callback (which shouldn't take
+%% place) fails on.
+
+%% diameter answers.
+send_unknown_application([_,_]) ->
+    #'diameter_base_answer-message'{'Result-Code' = 3007,
+                                                 %% UNSUPPORTED_APPLICATION
+                                    'Failed-AVP' = [],
+                                    'AVP' = []}
+        = call();
+
+send_unknown_application(Config) ->
+    send_unknown_application(?group(Config)).
+
 %% send_unknown_command/1
 %%
-%% Send a unknown command and expect a different result depending on
-%% whether or not the server gets a handle_request callback.
+%% Send a unknown command that a callback discards.
 
-%% Server handle_request discards the request.
-send_unknown_command(callback) ->
+%% handle_request discards the request.
+send_unknown_command([callback, _]) ->
     {error, timeout} = call();
 
-%% No handle_request, diameter answers.
-send_unknown_command(answer_3xxx) ->
-    #'diameter_base_answer-message'{'Result-Code' = ?UNSUPPORTED_COMMAND}
+%% diameter answers.
+send_unknown_command([_,_]) ->
+    #'diameter_base_answer-message'{'Result-Code' = 3001,
+                                                 %% UNSUPPORTED_COMMAND
+                                    'Failed-AVP' = [],
+                                    'AVP' = []}
         = call();
 
 send_unknown_command(Config) ->
-    ?testcase(Config),
     send_unknown_command(?group(Config)).
 
-%% send_ok_override/1
+%% send_ok/1
 %%
-%% Send a correct STA and expect the same answer from handle_request
-%% in both cases.
+%% Send a correct STR that a callback answers with 5002.
 
-send_ok_override(A)
-  when is_atom(A) ->
-    #diameter_base_STA{'Result-Code' = ?UNKNOWN_SESSION_ID}
+%% Callback answers.
+send_ok([_,_]) ->
+    #diameter_base_STA{'Result-Code' = 5002,  %% UNKNOWN_SESSION_ID
+                       'Failed-AVP' = [],
+                       'AVP' = []}
         = call();
 
-send_ok_override(Config) ->
-    ?testcase(Config),
-    send_ok_override(?group(Config)).
+send_ok(Config) ->
+    send_ok(?group(Config)).
 
 %% send_invalid_avp_bits/1
 %%
 %% Send a request with an incorrect length on the optional
-%% Origin-State-Id and expect a callback to ignore the error.
+%% Origin-State-Id that a callback ignores.
 
-send_invalid_avp_bits(callback) ->
-    #diameter_base_STA{'Result-Code' = ?SUCCESS,
-                       'Failed-AVP' = []}
+%% Callback answers.
+send_invalid_avp_bits([callback, _]) ->
+    #diameter_base_STA{'Result-Code' = 2001,  %% SUCCESS
+                       'Failed-AVP' = [],
+                       'AVP' = []}
         = call();
 
-send_invalid_avp_bits(answer_3xxx) ->
-    #'diameter_base_answer-message'{'Result-Code' = ?INVALID_AVP_BITS,
-                                    'Failed-AVP' = []}
+%% diameter answers.
+send_invalid_avp_bits([_,_]) ->
+    #'diameter_base_answer-message'{'Result-Code' = 3009, %% INVALID_AVP_BITS
+                                    'Failed-AVP' = [],
+                                    'AVP' = []}
         = call();
 
 send_invalid_avp_bits(Config) ->
-    ?testcase(Config),
     send_invalid_avp_bits(?group(Config)).
 
+%% send_missing_avp/1
+%%
+%% Send a request with a missing AVP that a callback answers.
+
+%% diameter answers.
+send_missing_avp([answer, rfc6733]) ->
+    #'diameter_base_answer-message'{'Result-Code' = 5005,  %% MISSING_AVP
+                                    'Failed-AVP' = [_],
+                                    'AVP' = []}
+        = call();
+
+%% Callback answers.
+send_missing_avp([_,_]) ->
+    #diameter_base_STA{'Result-Code' = 5005,  %% MISSING_AVP
+                       'Failed-AVP' = [_],
+                       'AVP' = []}
+        = call();
+
+send_missing_avp(Config) ->
+    send_missing_avp(?group(Config)).
+
+%% send_ignore_missing_avp/1
+%%
+%% Send a request with a missing AVP that a callback ignores.
+
+%% diameter answers.
+send_ignore_missing_avp([answer, rfc6733]) ->
+    #'diameter_base_answer-message'{'Result-Code' = 5005,  %% MISSING_AVP
+                                    'Failed-AVP' = [_],
+                                    'AVP' = []}
+        = call();
+
+%% Callback answers, ignores the error
+send_ignore_missing_avp([_,_]) ->
+    #diameter_base_STA{'Result-Code' = 2001,  %% SUCCESS
+                       'Failed-AVP' = [],
+                       'AVP' = []}
+        = call();
+
+send_ignore_missing_avp(Config) ->
+    send_ignore_missing_avp(?group(Config)).
+
 %% send_double_error/1
 %%
 %% Send a request with both an incorrect length on the optional
-%% Origin-State-Id and a missing AVP and see that it's answered
-%% differently.
+%% Origin-State-Id and a missing AVP.
 
-%% diameter answers with the 3xxx error.
-send_double_error(answer_3xxx) ->
-    #'diameter_base_answer-message'{'Result-Code' = ?INVALID_AVP_BITS,
-                                    'Failed-AVP' = [_]}
+%% Callback answers with STA.
+send_double_error([callback, _]) ->
+    #diameter_base_STA{'Result-Code' = 5005,  %% MISSING_AVP
+                       'Failed-AVP' = [_],
+                       'AVP' = []}
         = call();
 
-%% handle_request answers with STA and diameter resets Result-Code.
-send_double_error(callback) ->
-    #diameter_base_STA{'Result-Code' = ?MISSING_AVP,
-                       'Failed-AVP' = [_]}
+%% diameter answers with answer-message.
+send_double_error([_,_]) ->
+    #'diameter_base_answer-message'{'Result-Code' = 3009, %% INVALID_AVP_BITS
+                                    'Failed-AVP' = [_],
+                                    'AVP' = []}
         = call();
 
 send_double_error(Config) ->
-    ?testcase(Config),
     send_double_error(?group(Config)).
 
+%% send_3xxx/1
+%%
+%% Send a request that's answered with a 3xxx result code.
+
+%% Callback answers.
+send_3xxx([_,_]) ->
+    #'diameter_base_answer-message'{'Result-Code' = 3999,
+                                    'Failed-AVP' = [],
+                                    'AVP' = []}
+        = call();
+
+send_3xxx(Config) ->
+    send_3xxx(?group(Config)).
+
+%% send_5xxx/1
+%%
+%% Send a request that's answered with a 5xxx result code.
+
+%% Callback answers but fails since 5xxx isn't allowed in an RFC 3588
+%% answer-message.
+send_5xxx([_, rfc3588]) ->
+    {error, timeout} = call();
+
+%% Callback answers.
+send_5xxx([_,_]) ->
+    #'diameter_base_answer-message'{'Result-Code' = 5999,
+                                    'Failed-AVP' = [],
+                                    'AVP' = []}
+        = call();
+
+send_5xxx(Config) ->
+    send_5xxx(?group(Config)).
+
 %% ===========================================================================
 
 call() ->
@@ -241,7 +343,7 @@ call() ->
                   ?DICT,
                   #diameter_base_STR
                   {'Termination-Cause' = ?LOGOUT,
-                   'Auth-Application-Id' = ?DIAMETER_APP_ID_COMMON,
+                   'Auth-Application-Id' = ?COMMON,
                    'Class' = [?L(Name)]},
                   [{extra, [Name]}]).
 
@@ -268,57 +370,62 @@ pick_peer([Peer], _, ?CLIENT, _State, _Name) ->
 prepare_request(Pkt, ?CLIENT, {_Ref, Caps}, Name) ->
     {send, prepare(Pkt, Caps, Name)}.
 
+prepare(Pkt0, Caps, send_unknown_application) ->
+    Req = sta(Pkt0, Caps),
+    #diameter_packet{bin = <<H:8/binary, 0:32, T/binary>>}
+        = Pkt
+        = diameter_codec:encode(?DICT, Pkt0#diameter_packet{msg = Req}),
+
+    Pkt#diameter_packet{bin = <<H/binary, 23:32, T/binary>>};
+
 prepare(Pkt0, Caps, send_unknown_command) ->
-    #diameter_packet{msg = Req0}
-        = Pkt0,
-    #diameter_caps{origin_host  = {OH, _},
-                   origin_realm = {OR, DR}}
-        = Caps,
-    Req = Req0#diameter_base_STR{'Session-Id' = diameter:session_id(OH),
-                                 'Origin-Host' = OH,
-                                 'Origin-Realm' = OR,
-                                 'Destination-Realm' = DR},
+    Req = sta(Pkt0, Caps),
     #diameter_packet{bin = <<H:5/binary, 275:24, T/binary>>}
         = Pkt
         = diameter_codec:encode(?DICT, Pkt0#diameter_packet{msg = Req}),
 
     Pkt#diameter_packet{bin = <<H/binary, 572:24, T/binary>>};
 
-prepare(Pkt, Caps, send_ok_override) ->
-    #diameter_packet{msg = Req}
-        = Pkt,
-    #diameter_caps{origin_host  = {OH, _},
-                   origin_realm = {OR, DR}}
-        = Caps,
-    Req#diameter_base_STR{'Session-Id' = diameter:session_id(OH),
-                          'Origin-Host' = OH,
-                          'Origin-Realm' = OR,
-                          'Destination-Realm' = DR};
+prepare(Pkt, Caps, T)
+  when T == send_ok;
+       T == send_3xxx;
+       T == send_5xxx ->
+    sta(Pkt, Caps);
 
-prepare(Pkt, Caps, send_invalid_avp_bits) ->
-    #diameter_packet{msg = Req0}
-        = Pkt,
-    #diameter_caps{origin_host  = {OH, _},
-                   origin_realm = {OR, DR}}
-        = Caps,
+prepare(Pkt0, Caps, send_invalid_avp_bits) ->
+    Req0 = sta(Pkt0, Caps),
     %% Append an Origin-State-Id with an incorrect AVP Length in order
     %% to force 3009.
-    Req = Req0#diameter_base_STR{'Session-Id' = diameter:session_id(OH),
-                                 'Origin-Host' = OH,
-                                 'Origin-Realm' = OR,
-                                 'Destination-Realm' = DR,
-                                 'Origin-State-Id' = [7]},
+    Req = Req0#diameter_base_STR{'Origin-State-Id' = [7]},
     #diameter_packet{bin = Bin}
-        = diameter_codec:encode(?DICT, Pkt#diameter_packet{msg = Req}),
+        = Pkt
+        = diameter_codec:encode(?DICT, Pkt0#diameter_packet{msg = Req}),
     Offset = size(Bin) - 12 + 5,
     <<H:Offset/binary, Len:24, T/binary>> = Bin,
     Pkt#diameter_packet{bin = <<H/binary, (Len + 2):24, T/binary>>};
 
 prepare(Pkt0, Caps, send_double_error) ->
-    #diameter_packet{bin = Bin}
-        = Pkt
-        = prepare(Pkt0, Caps, send_invalid_avp_bits),
-    %% Keep Session-Id but remove Origin-Host.
+    dehost(prepare(Pkt0, Caps, send_invalid_avp_bits));
+
+prepare(Pkt, Caps, T)
+  when T == send_missing_avp;
+       T == send_ignore_missing_avp ->
+    Req = sta(Pkt, Caps),
+    dehost(diameter_codec:encode(?DICT, Pkt#diameter_packet{msg = Req})).
+
+sta(Pkt, Caps) ->
+    #diameter_packet{msg = Req}
+        = Pkt,
+    #diameter_caps{origin_host  = {OH, _},
+                   origin_realm = {OR, DR}}
+        = Caps,
+    Req#diameter_base_STR{'Session-Id' = diameter:session_id(OH),
+                          'Origin-Host' = OH,
+                          'Origin-Realm' = OR,
+                          'Destination-Realm' = DR}.
+
+%% Strip Origin-Host.
+dehost(#diameter_packet{bin = Bin} = Pkt) ->
     <<V, Len:24, H:16/binary, T0/binary>>
         = Bin,
     {SessionId, T1} = split_avp(T0),
@@ -351,18 +458,27 @@ pad(N) ->
 
 %% handle_request/3
 
-%% send_unknown_command
-handle_request(#diameter_packet{msg = undefined}, ?SERVER, _) ->
+handle_request(#diameter_packet{header = #diameter_header{application_id = 0},
+                                msg = Msg},
+               ?SERVER,
+               {_, Caps}) ->
+    request(Msg, Caps).
+
+request(undefined, _) ->  %% unknown command
     discard;
 
-handle_request(#diameter_packet{msg = Req}, ?SERVER, {_, Caps}) ->
-    #diameter_base_STR{'Class' = [Name]}
-        = Req,
-    {reply, request(?A(Name), Req, Caps)}.
+request(#diameter_base_STR{'Class' = [Name]} = Req, Caps) ->
+    request(?A(Name), Req, Caps).
+
+request(send_ok, Req, Caps) ->
+    {reply, #diameter_packet{msg = answer(Req, Caps),
+                             errors = [5002]}};  %% UNKNOWN_SESSION_ID
 
-request(send_ok_override, Req, Caps) ->
-    #diameter_packet{msg = answer(Req, Caps),
-                     errors = [?UNKNOWN_SESSION_ID]};  %% override
+request(send_3xxx, _Req, _Caps) ->
+    {answer_message, 3999};
+
+request(send_5xxx, _Req, _Caps) ->
+    {answer_message, 5999};
 
 request(send_invalid_avp_bits, Req, Caps) ->
     #diameter_base_STR{'Origin-State-Id' = []}
@@ -370,10 +486,16 @@ request(send_invalid_avp_bits, Req, Caps) ->
     %% Default errors field but a non-answer-message and only 3xxx
     %% errors detected means diameter sets neither Result-Code nor
     %% Failed-AVP.
-    #diameter_packet{msg = answer(Req, Caps)};
+    {reply, #diameter_packet{msg = answer(Req, Caps)}};
+
+request(T, Req, Caps)
+  when T == send_double_error;
+       T == send_missing_avp ->
+    {reply, answer(Req, Caps)};
 
-request(send_double_error, Req, Caps) ->
-    answer(Req, Caps).
+request(send_ignore_missing_avp, Req, Caps) ->
+    {reply, #diameter_packet{msg = answer(Req, Caps),
+                             errors = false}}.  %% ignore errors
 
 answer(Req, Caps) ->
     #diameter_base_STR{'Session-Id' = SId}
@@ -384,4 +506,4 @@ answer(Req, Caps) ->
     #diameter_base_STA{'Session-Id' = SId,
                        'Origin-Host' = OH,
                        'Origin-Realm' = OR,
-                       'Result-Code' = ?SUCCESS}.
+                       'Result-Code' = 2001}.  %% SUCCESS