From 4d8afe10687f11e9e9796ba787a49c85f76082e9 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sat, 16 Feb 2013 22:38:03 +0100 Subject: Comments and minor Result-Code fix In particular, don't put an error tuple in the errors field of a #diameter_packet{} when Result-Code and the E-bit are in conflict, put {integer(), #diameter_avp{}}. --- lib/diameter/src/base/diameter_traffic.erl | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index d6cb5ff0fa..c70fad8f83 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -910,16 +910,20 @@ 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}, _, _) -> ok; +%% Incoming with decode errors. incr(recv = D, #diameter_packet{header = H, errors = [_|_]}, _, TPid) -> incr(TPid, {diameter_codec:msg_id(H), D, error}); +%% Incoming without errors or outgoing. Outgoing with encode errors +%% never gets here since encode fails. incr(Dir, Pkt, Dict, TPid) -> #diameter_packet{header = #diameter_header{is_error = E} = Hdr, @@ -929,9 +933,9 @@ incr(Dir, Pkt, Dict, TPid) -> 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) + %% Check that Result-Code matches E-bit. + (not (E orelse PE)) %% non-3xxx answer without E-bit + orelse (E andalso PE) %% 3xxx answer with E-bit orelse x({invalid_error_bit, RC}, answer, [Dir, Pkt]), irc(TPid, Hdr, Dir, rc_counter(Dict, Rec, RC)). @@ -947,7 +951,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: %% @@ -1303,12 +1307,15 @@ handle_answer(SvcName, answer(Pkt, SvcName, Dict, App, #request{transport = TPid} = Req) -> try - incr(recv, Pkt, Dict, TPid) + incr(recv, Pkt, Dict, TPid) %% count result codes in received answers 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, -- cgit v1.2.3 From 1da8a27f08a890d3ba44ad9707b848ff8afbc1f5 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sat, 16 Feb 2013 01:14:12 +0100 Subject: Answer 5xxx errors with application_opt() request_errors = answer RFC 3588 allowed only 3xxx result codes in an answer-message (that is, an answer that sets the E-bit) while RFC 6733 also allows 5xxx result codes. Setting request_errors = answer tells diameter to answer 5xxx errors itself. Returning {answer_message, integer()} from a handle_request callback allows both 3xxx and 5xxx result codes to be set. {protocol_error, integer()} is retained for 3xxx result codes. --- lib/diameter/doc/src/diameter.xml | 33 ++++++-- lib/diameter/doc/src/diameter_app.xml | 27 ++++-- lib/diameter/include/diameter.hrl | 2 +- lib/diameter/src/base/diameter.erl | 2 +- lib/diameter/src/base/diameter_config.erl | 43 ++++------ lib/diameter/src/base/diameter_traffic.erl | 132 ++++++++++++++++++----------- 6 files changed, 144 insertions(+), 95 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 alias option if unspecified.

Specifies whether or not the &app_pick_peer; -application callback can modify the application state, +application callback can modify the application state. Defaults to false if unspecified.

@@ -226,22 +226,37 @@ question is as if a callback had taken place and returned Defaults to report if unspecified.

 -{request_errors, answer_3xxx|callback} +{request_errors, answer_3xxx|answer|callback}

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

+error other than 3007, DIAMETER_APPLICATION_UNSUPPORTED (which cannot +be associated with an application callback module), is detected.

-If answer_3xxx 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 callback then even 3xxx errors result in an application -&app_handle_request; callback.

+If answer_3xxx then requests are answered without a +&app_handle_request; callback taking place. +If answer 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 callback then a &app_handle_request; callback always takes +place and the return value determines the answer sent to the peer.

Defaults to answer_3xxx if unspecified.

+ + +

+Answers sent by diameter set the E-bit in the Diameter Header. +Since RFC 3588 allowed only 3xxx result codes in an +answer-message, answer has the same semantics as +answer_3xxx if the peer connection in question has configured +the RFC 3588 common dictionary, diameter_gen_base_rfc3588. +RFC 6733 allows both 3xxx and 5xxx result codes in an +answer-message so a connection configured with the RFC 6733 +common dictionary, diameter_gen_base_rfc6733, does +distinguish between answer_3xxx and answer.

+ 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.

| discard | {eval|eval_packet, Action, PostF} Reply = {reply, &packet; | &message;} + | {answer_message, 3000..3999|5000..5999} | {protocol_error, 3000..3999} Opt = &mod_call_opt; PostF = &mod_evaluable; @@ -545,22 +546,25 @@ preserved in the outgoing answer, appropriate values otherwise being set by diameter.

-{protocol_error, 3000..3999} +{answer_message, 3000..3999|5000..5999}

Send an answer message to the peer containing the specified -protocol error. +Result-Code. Equivalent to

 {reply, ['answer-message' | Avps]

where Avps sets the Origin-Host, Origin-Realm, the specified -Result-Code and (if the request sent one) Session-Id AVP's.

+Result-Code and (if the request contained one) Session-Id AVP's.

-Returning a non-3xxx value in a protocol_error tuple -will cause the request process in question to fail.

+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 diameter_gen_base_rfc3588. +(Since RFC 3588 only allows 3xxx values in an answer-message.)

 {relay, Opts} @@ -613,11 +617,20 @@ containing the encoded binary. The return value is ignored.

+{protocol_error, 3000..3999} + +

+Equivalent to {answer_message, 3000..3999}.

+ + +

-Note that protocol errors detected by diameter will result in an -answer message without handle_request/3 being invoked.

+Requests containing errors may be answered by diameter, without a +callback taking place, depending on the value of the +&mod_application_opt; request_errors.

+ 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 c70fad8f83..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 @@ -915,31 +937,43 @@ find(Pred, [H|T]) -> %% Outgoing message as binary: don't count. (Sending binaries is only %% partially supported.) -incr(_, #diameter_packet{msg = undefined}, _, _) -> +incr(_, #diameter_packet{msg = undefined}, _, _, _) -> ok; %% Incoming with decode errors. -incr(recv = D, #diameter_packet{header = H, errors = [_|_]}, _, TPid) -> +incr(recv = D, #diameter_packet{header = H, errors = [_|_]}, _, TPid, _) -> incr(TPid, {diameter_codec:msg_id(H), D, error}); %% Incoming without errors or outgoing. Outgoing with encode errors %% never gets here since encode fails. -incr(Dir, Pkt, Dict, TPid) -> +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 Result-Code matches E-bit. - (not (E orelse PE)) %% non-3xxx answer without E-bit - orelse (E andalso PE) %% 3xxx answer with E-bit + %% 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; @@ -991,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. @@ -1143,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 %% @@ -1295,19 +1326,20 @@ 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) %% count result codes in received answers + incr(recv, Pkt, Dict, TPid, Dict0) %% count incoming result codes of _ -> answer(Pkt, SvcName, App, Req) catch @@ -1505,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}}, -- cgit v1.2.3 From 1f7db75c2b67009f90118877c3f6ce6cabab183f Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sun, 17 Feb 2013 00:23:25 +0100 Subject: Add testcases for application_opt() request_errors = answer --- lib/diameter/test/diameter_3xxx_SUITE.erl | 368 ++++++++++++++++++++---------- 1 file changed, 245 insertions(+), 123 deletions(-) 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 = <>} + = Pkt + = diameter_codec:encode(?DICT, Pkt0#diameter_packet{msg = Req}), + + Pkt#diameter_packet{bin = <>}; + 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 = <>} = Pkt = diameter_codec:encode(?DICT, Pkt0#diameter_packet{msg = Req}), Pkt#diameter_packet{bin = <>}; -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, <> = Bin, Pkt#diameter_packet{bin = <>}; 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) -> <> = 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 -- cgit v1.2.3