From 21e778b998b895034453251d83c3e6aaa72fe395 Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Fri, 31 May 2013 14:46:59 +0200
Subject: Fix setting of Failed-AVP on {answer_message, 5xxx} from
handle_request
RFC 6733 says that certain 5xxx result codes must be accompanied by
Failed-AVP, and decode populates #diameter_packet.errors with
Result-Code/AVP pairs for errors it detects. However, Failed-AVP was not
set in the outgoing answer if the handle_request callback returned
{answer_message, 5xxx}. It is now set with the AVP from the first pair
with the specified Result-Code, if found.
Note that {answer_message, 5xxx} doesn't handle all cases in which a
5xxx answer is required, only that in which the setting above is
appropriate. If it isn't then handle_request should construct its answer
and return {reply, Ans}.
---
lib/diameter/doc/src/diameter_app.xml | 14 +++++++++---
lib/diameter/src/base/diameter_traffic.erl | 22 +++++++++++++-----
lib/diameter/test/diameter_3xxx_SUITE.erl | 36 ++++++++++++++++++++++++++++--
3 files changed, 61 insertions(+), 11 deletions(-)
(limited to 'lib/diameter')
diff --git a/lib/diameter/doc/src/diameter_app.xml b/lib/diameter/doc/src/diameter_app.xml
index d4fb792787..e6c9cc9a90 100644
--- a/lib/diameter/doc/src/diameter_app.xml
+++ b/lib/diameter/doc/src/diameter_app.xml
@@ -565,7 +565,8 @@ Equivalent to</p>
</pre>
<p>
where <c>Avps</c> sets the Origin-Host, Origin-Realm, the specified
-Result-Code and (if the request contained one) Session-Id AVP's.</p>
+Result-Code and (if the request contained one) Session-Id AVP's, and
+possibly Failed-AVP as described below.</p>
<p>
Returning a value other than 3xxx or 5xxx will cause the request
@@ -573,6 +574,14 @@ 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>
+
+<p>
+When returning 5xxx, Failed-AVP will be populated with the AVP of the
+first matching Result-Code/AVP pair in the <c>errors</c> field of the
+argument &packet;, if found.
+If this is not appropriate then an answer-message should be
+constructed explicitly and returned in a <c>reply</c> tuple
+instead.</p>
</item>
<tag><c>{relay, Opts}</c></tag>
@@ -592,8 +601,7 @@ header of the relayed request.</p>
The returned <c>Opts</c> should not specify <c>detach</c>.
A subsequent &handle_answer;
callback for the relayed request must return its first
-argument, the <c>#diameter_packet{}</c> record containing the answer
-message.
+argument, the &packet; containing the answer message.
Note that the <c>extra</c> option can be specified to supply arguments
that can distinguish the relay case from others if so desired.
Any other return value (for example, from a
diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl
index 820d37535a..0b15e68ec7 100644
--- a/lib/diameter/src/base/diameter_traffic.erl
+++ b/lib/diameter/src/base/diameter_traffic.erl
@@ -479,10 +479,9 @@ answer_message(RC,
#diameter_caps{origin_host = {OH,_},
origin_realm = {OR,_}},
Dict0,
- #diameter_packet{avps = Avps}
- = Pkt) ->
+ Pkt) ->
?LOG({error, RC}, Pkt),
- {Dict0, answer_message(OH, OR, RC, Dict0, Avps)}.
+ {Dict0, answer_message(OH, OR, RC, Dict0, Pkt)}.
%% resend/7
@@ -861,12 +860,14 @@ failed(Rec, FailedAvp, Dict) ->
%% answer_message/5
-answer_message(OH, OR, RC, Dict0, Avps) ->
+answer_message(OH, OR, RC, Dict0, #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, Dict0, Avps)].
+ {'Result-Code', RC}]
+ ++ session_id(Code, Vid, Dict0, Avps)
+ ++ failed_avp(RC, Es).
session_id(Code, Vid, Dict0, Avps)
when is_list(Avps) ->
@@ -878,6 +879,15 @@ session_id(Code, Vid, Dict0, Avps)
[]
end.
+%% Note that this should only match 5xxx result codes currently but
+%% don't bother distinguishing this case.
+failed_avp(RC, [{RC, Avp} | _]) ->
+ [{'Failed-AVP', [{'AVP', [Avp]}]}];
+failed_avp(RC, [_ | Es]) ->
+ failed_avp(RC, Es);
+failed_avp(_, [] = No) ->
+ No.
+
%% find_avp/3
find_avp(Code, Vid, Avps)
diff --git a/lib/diameter/test/diameter_3xxx_SUITE.erl b/lib/diameter/test/diameter_3xxx_SUITE.erl
index 0ec0d5020f..071b1a1177 100644
--- a/lib/diameter/test/diameter_3xxx_SUITE.erl
+++ b/lib/diameter/test/diameter_3xxx_SUITE.erl
@@ -43,6 +43,7 @@
send_invalid_hdr_bits/1,
send_missing_avp/1,
send_ignore_missing_avp/1,
+ send_5xxx_missing_avp/1,
send_double_error/1,
send_3xxx/1,
send_5xxx/1,
@@ -139,6 +140,7 @@ tc() ->
send_invalid_hdr_bits,
send_missing_avp,
send_ignore_missing_avp,
+ send_5xxx_missing_avp,
send_double_error,
send_3xxx,
send_5xxx].
@@ -279,6 +281,32 @@ send_ignore_missing_avp([_,_]) ->
send_ignore_missing_avp(Config) ->
send_ignore_missing_avp(?group(Config)).
+%% send_5xxx_missing_avp/1
+%%
+%% Send a request with a missing AVP that a callback answers
+%% with {answer_message, 5005}.
+
+%% RFC 6733 allows 5xxx in an answer-message.
+send_5xxx_missing_avp([_, rfc6733]) ->
+ #'diameter_base_answer-message'{'Result-Code' = 5005, %% MISSING_AVP
+ 'Failed-AVP' = [_],
+ 'AVP' = []}
+ = call();
+
+%% RFC 3588 doesn't: sending answer fails.
+send_5xxx_missing_avp([_, rfc3588]) ->
+ {error, timeout} = call();
+
+%% Callback answers, ignores the error
+send_5xxx_missing_avp([_,_]) ->
+ #diameter_base_STA{'Result-Code' = 2001, %% SUCCESS
+ 'Failed-AVP' = [],
+ 'AVP' = []}
+ = call();
+
+send_5xxx_missing_avp(Config) ->
+ send_5xxx_missing_avp(?group(Config)).
+
%% send_double_error/1
%%
%% Send a request with both an invalid E-bit and a missing AVP.
@@ -403,7 +431,8 @@ prepare(Pkt0, Caps, send_double_error) ->
prepare(Pkt, Caps, T)
when T == send_missing_avp;
- T == send_ignore_missing_avp ->
+ T == send_ignore_missing_avp;
+ T == send_5xxx_missing_avp ->
Req = sta(Pkt, Caps),
dehost(diameter_codec:encode(?DICT, Pkt#diameter_packet{msg = Req})).
@@ -487,7 +516,10 @@ request(T, Req, Caps)
request(send_ignore_missing_avp, Req, Caps) ->
{reply, #diameter_packet{msg = answer(Req, Caps),
- errors = false}}. %% ignore errors
+ errors = false}}; %% ignore errors
+
+request(send_5xxx_missing_avp, _Req, _Caps) ->
+ {answer_message, 5005}. %% MISSING_AVP
answer(Req, Caps) ->
#diameter_base_STR{'Session-Id' = SId}
--
cgit v1.2.3