aboutsummaryrefslogtreecommitdiffstats
path: root/lib/diameter
diff options
context:
space:
mode:
authorAnders Svensson <[email protected]>2015-05-02 10:14:58 +0200
committerAnders Svensson <[email protected]>2015-05-05 12:42:54 +0200
commita1df50b38826370b84ec557de401bc1b09d56de7 (patch)
tree154970a12d14f0424329eb60d03f0595abd2e136 /lib/diameter
parent545890576542e4be630df8772654b99bd0306f62 (diff)
downloadotp-a1df50b38826370b84ec557de401bc1b09d56de7.tar.gz
otp-a1df50b38826370b84ec557de401bc1b09d56de7.tar.bz2
otp-a1df50b38826370b84ec557de401bc1b09d56de7.zip
Don't confuse Result-Code and Experimental-Result
Decode of an answer message not setting the E-bit, and containing Experiment-Result but not Result-Code, identified Result-Code as the erroneous when Erroneous-Result-Code was 3xxx. Here's an example (from trace) of a the errors field after decode: [{5004, {diameter_avp,undefined,undefined,false,false,undefined,'Result-Code', 3001,undefined,undefined}}], The diameter_avp was just constructed from the AVP name and decoded result, without regard for which result code AVP contained the value. Fix by extracting the AVP from the incoming message.
Diffstat (limited to 'lib/diameter')
-rw-r--r--lib/diameter/src/base/diameter_traffic.erl158
-rw-r--r--lib/diameter/test/diameter_traffic_SUITE.erl60
2 files changed, 147 insertions, 71 deletions
diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl
index 538ebeeeba..9a9f7a3197 100644
--- a/lib/diameter/src/base/diameter_traffic.erl
+++ b/lib/diameter/src/base/diameter_traffic.erl
@@ -980,8 +980,8 @@ answer_message(OH, OR, RC, Dict0, #diameter_packet{avps = Avps,
session_id(Code, Vid, Dict0, Avps)
when is_list(Avps) ->
try
- {value, #diameter_avp{data = D}} = find_avp(Code, Vid, Avps),
- [{'Session-Id', [Dict0:avp(decode, D, 'Session-Id')]}]
+ #diameter_avp{data = Bin} = find_avp(Code, Vid, Avps),
+ [{'Session-Id', [Dict0:avp(decode, Bin, 'Session-Id')]}]
catch
error: _ ->
[]
@@ -998,26 +998,17 @@ failed_avp(_, [] = No) ->
%% find_avp/3
-find_avp(Code, Vid, Avps)
- when is_integer(Code), (undefined == Vid orelse is_integer(Vid)) ->
- find(fun(A) -> is_avp(Code, Vid, A) end, Avps).
+%% Grouped ...
+find_avp(Code, VId, [[#diameter_avp{code = Code, vendor_id = VId} | _] = As
+ | _]) ->
+ As;
-%% The final argument here could be a list of AVP's, depending on the case,
-%% but we're only searching at the top level.
-is_avp(Code, Vid, #diameter_avp{code = Code, vendor_id = Vid}) ->
- true;
-is_avp(_, _, _) ->
- false.
+%% ... or not.
+find_avp(Code, VId, [#diameter_avp{code = Code, vendor_id = VId} = A | _]) ->
+ A;
-find(_, []) ->
- false;
-find(Pred, [H|T]) ->
- case Pred(H) of
- true ->
- {value, H};
- false ->
- find(Pred, T)
- end.
+find_avp(Code, VId, [_ | Avps]) ->
+ find_avp(Code, VId, Avps).
%% 7. Error Handling
%%
@@ -1086,7 +1077,6 @@ incr_result(_, #diameter_packet{msg = undefined = No}, _, _) ->
incr_result(Dir, Pkt, TPid, {Dict, AppDict, Dict0}) ->
#diameter_packet{header = #diameter_header{is_error = E}
= Hdr,
- msg = Msg,
errors = Es}
= Pkt,
@@ -1096,13 +1086,13 @@ incr_result(Dir, Pkt, TPid, {Dict, AppDict, Dict0}) ->
recv /= Dir orelse [] == Es orelse incr_error(Dir, Id, TPid, AppDict),
%% Exit on a missing result code.
- T = rc_counter(Dict, Msg),
+ T = rc_counter(Dict, Dir, Pkt),
T == false andalso ?LOGX(no_result_code, {Dict, Dir, Hdr}),
- {Ctr, RC} = T,
+ {Ctr, RC, Avp} = T,
%% Or on an inappropriate value.
is_result(RC, E, Dict0)
- orelse ?LOGX(invalid_error_bit, {Dict, Dir, Hdr, RC}),
+ orelse ?LOGX(invalid_error_bit, {Dict, Dir, Hdr, Avp}),
incr(TPid, {Id, Dir, Ctr}),
Ctr.
@@ -1148,7 +1138,7 @@ is_result(RC, true, _) ->
incr(TPid, Counter) ->
diameter_stats:incr(Counter, TPid, 1).
-%% rc_counter/2
+%% rc_counter/3
%% RFC 3588, 7.6:
%%
@@ -1156,39 +1146,45 @@ incr(TPid, Counter) ->
%% applications MUST include either one Result-Code AVP or one
%% Experimental-Result AVP.
-rc_counter(Dict, Msg) ->
- rcc(Dict, Msg, int(get_avp_value(Dict, 'Result-Code', Msg))).
+rc_counter(Dict, recv, #diameter_packet{header = H, avps = As}) ->
+ rc_counter(Dict, [H|As]);
-rcc(Dict, Msg, undefined) ->
- rcc(get_avp_value(Dict, 'Experimental-Result', Msg));
+rc_counter(Dict, _, #diameter_packet{msg = Msg}) ->
+ rc_counter(Dict, Msg).
+
+rc_counter(Dict, Msg) ->
+ rcc(get_result(Dict, Msg)).
-rcc(_, _, N)
+rcc(#diameter_avp{name = 'Result-Code' = Name, value = N} = A)
when is_integer(N) ->
- {{'Result-Code', N}, N}.
+ {{Name, N}, N, A};
-%% Outgoing answers may be in any of the forms messages can be sent
-%% in. Incoming messages will be records. We're assuming here that the
-%% arity of the result code AVP's is 0 or 1.
+rcc(#diameter_avp{name = 'Result-Code' = Name, value = [N|_]} = A)
+ when is_integer(N) ->
+ {{Name, N}, N, A};
-rcc([{_,_,N} = T | _])
+rcc(#diameter_avp{name = 'Experimental-Result', value = {_,_,N} = T} = A)
when is_integer(N) ->
- {T,N};
-rcc({_,_,N} = T)
+ {T, N, A};
+
+rcc(#diameter_avp{name = 'Experimental-Result', value = [{_,_,N} = T|_]} = A)
when is_integer(N) ->
- {T,N};
+ {T, N, A};
+
rcc(_) ->
false.
-%% Extract the first good looking integer. There's no guarantee
-%% that what we're looking for has arity 1.
-int([N|_])
- when is_integer(N) ->
- N;
-int(N)
- when is_integer(N) ->
- N;
-int(_) ->
- undefined.
+%% get_result/2
+
+get_result(Dict, Msg) ->
+ try
+ [throw(A) || N <- ['Result-Code', 'Experimental-Result'],
+ #diameter_avp{} = A <- [get_avp(Dict, N, Msg)]]
+ of
+ [] -> false
+ catch
+ #diameter_avp{} = A -> A
+ end.
x(T) ->
exit(T).
@@ -1528,10 +1524,10 @@ handle_A(Pkt, SvcName, Dict, Dict0, App, #request{transport = TPid} = Req) ->
%% a missing AVP. If both are optional in the dictionary
%% then this isn't a decode error: just continue on.
answer(Pkt, SvcName, App, Req);
- exit: {invalid_error_bit, {_, _, _, RC}} ->
+ exit: {invalid_error_bit, {_, _, _, Avp}} ->
#diameter_packet{errors = Es}
= Pkt,
- E = {5004, #diameter_avp{name = 'Result-Code', value = RC}},
+ E = {5004, Avp},
answer(Pkt#diameter_packet{errors = [E|Es]}, SvcName, App, Req)
end.
@@ -1868,7 +1864,7 @@ str([]) ->
str(T) ->
T.
-%% get_avp_value/3
+%% get_avp/3
%%
%% Find an AVP in a message of one of three forms:
%%
@@ -1885,47 +1881,71 @@ str(T) ->
%% look for are in the common dictionary. This is required since the
%% relay dictionary doesn't inherit the common dictionary (which maybe
%% it should).
-get_avp_value(?RELAY, Name, Msg) ->
- get_avp_value(?BASE, Name, Msg);
+get_avp(?RELAY, Name, Msg) ->
+ get_avp(?BASE, Name, Msg);
-%% Message sent as a header/avps list, probably a relay case but not
-%% necessarily.
-get_avp_value(Dict, Name, [#diameter_header{} | Avps]) ->
+%% Message as a header/avps list.
+get_avp(Dict, Name, [#diameter_header{} | Avps]) ->
try
{Code, _, VId} = Dict:avp_header(Name),
- [A|_] = lists:dropwhile(fun(#diameter_avp{code = C, vendor_id = V}) ->
- C /= Code orelse V /= VId
- end,
- Avps),
- avp_decode(Dict, Name, A)
+ find_avp(Code, VId, Avps)
+ of
+ A ->
+ avp_decode(Dict, Name, ungroup(A))
catch
error: _ ->
undefined
end;
%% Outgoing message as a name/values list.
-get_avp_value(_, Name, [_MsgName | Avps]) ->
+get_avp(_, Name, [_MsgName | Avps]) ->
case lists:keyfind(Name, 1, Avps) of
{_, V} ->
- V;
+ #diameter_avp{name = Name, value = V};
_ ->
undefined
end;
%% Message is typically a record but not necessarily.
-get_avp_value(Dict, Name, Rec) ->
+get_avp(Dict, Name, Rec) ->
try
- Dict:'#get-'(Name, Rec)
+ #diameter_avp{name = Name, value = Dict:'#get-'(Name, Rec)}
catch
error:_ ->
undefined
end.
+%% get_avp_value/3
+
+get_avp_value(Dict, Name, Msg) ->
+ case get_avp(Dict, Name, Msg) of
+ #diameter_avp{value = V} ->
+ V;
+ undefined = No ->
+ No
+ end.
+
+%% ungroup/1
+
+ungroup([Avp|_]) ->
+ Avp;
+ungroup(Avp) ->
+ Avp.
+
+%% avp_decode/3
+
avp_decode(Dict, Name, #diameter_avp{value = undefined,
- data = Bin}) ->
- Dict:avp(decode, Bin, Name);
-avp_decode(_, _, #diameter_avp{value = V}) ->
- V.
+ data = Bin}
+ = Avp) ->
+ try Dict:avp(decode, Bin, Name) of
+ V ->
+ Avp#diameter_avp{value = V}
+ catch
+ error:_ ->
+ Avp
+ end;
+avp_decode(_, _, #diameter_avp{} = Avp) ->
+ Avp.
cb(#diameter_app{module = [_|_] = M}, F, A) ->
eval(M, F, A);
diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl
index 7dd9f39f85..9e1d5e1435 100644
--- a/lib/diameter/test/diameter_traffic_SUITE.erl
+++ b/lib/diameter/test/diameter_traffic_SUITE.erl
@@ -41,6 +41,7 @@
send_eval/1,
send_bad_answer/1,
send_protocol_error/1,
+ send_experimental_result/1,
send_arbitrary/1,
send_unknown/1,
send_unknown_short/1,
@@ -301,6 +302,7 @@ tc() ->
send_eval,
send_bad_answer,
send_protocol_error,
+ send_experimental_result,
send_arbitrary,
send_unknown,
send_unknown_short,
@@ -480,6 +482,14 @@ send_protocol_error(Config) ->
?answer_message(?TOO_BUSY)
= call(Config, Req).
+%% Send a 3xxx Experimental-Result in an answer not setting the E-bit
+%% and missing a Result-Code.
+send_experimental_result(Config) ->
+ Req = ['ACR', {'Accounting-Record-Type', ?EVENT_RECORD},
+ {'Accounting-Record-Number', 5}],
+ ['ACA', _SessionId | _]
+ = call(Config, Req).
+
%% Send an ASR with an arbitrary non-mandatory AVP and expect success
%% and the same AVP in the reply.
send_arbitrary(Config) ->
@@ -1144,6 +1154,13 @@ answer(Pkt, Req, _Peer, Name, #group{client_dict0 = Dict0}) ->
[R | Vs] = Dict:'#get-'(answer(Ans, Es, Name)),
[Dict:rec2msg(R) | Vs].
+%% Missing Result-Codec and inapproriate Experimental-Result-Code.
+answer(Rec, Es, send_experimental_result) ->
+ [{5004, #diameter_avp{name = 'Experimental-Result'}},
+ {5005, #diameter_avp{name = 'Result-Code'}}]
+ = Es,
+ Rec;
+
%% An inappropriate E-bit results in a decode error ...
answer(Rec, Es, send_bad_answer) ->
[{5004, #diameter_avp{name = 'Result-Code'}} | _] = Es,
@@ -1175,7 +1192,9 @@ handle_error(Reason, _Req, [$C|_], _Peer, _, _Time) ->
%% Note that diameter will set Result-Code and Failed-AVPs if
%% #diameter_packet.errors is non-null.
-handle_request(#diameter_packet{header = H, msg = M}, _, {_Ref, Caps}) ->
+handle_request(#diameter_packet{header = H, msg = M, avps = As},
+ _,
+ {_Ref, Caps}) ->
#diameter_header{end_to_end_id = EI,
hop_by_hop_id = HI}
= H,
@@ -1183,10 +1202,12 @@ handle_request(#diameter_packet{header = H, msg = M}, _, {_Ref, Caps}) ->
V = EI bsr B, %% assert
V = HI bsr B, %%
#diameter_caps{origin_state_id = {_,[Id]}} = Caps,
- answer(origin(Id), request(M, Caps)).
+ answer(origin(Id), request(M, [H|As], Caps)).
answer(T, {Tag, Action, Post}) ->
{Tag, answer(T, Action), Post};
+answer(_, {reply, [#diameter_header{} | _]} = T) ->
+ T;
answer({A,C}, {reply, Ans}) ->
answer(C, {reply, msg(Ans, A, diameter_gen_base_rfc3588)});
answer(pkt, {reply, Ans})
@@ -1195,6 +1216,41 @@ answer(pkt, {reply, Ans})
answer(_, T) ->
T.
+%% request/3
+
+%% send_experimental_result
+request(#diameter_base_accounting_ACR{'Accounting-Record-Number' = 5},
+ [Hdr | Avps],
+ #diameter_caps{origin_host = {OH, _},
+ origin_realm = {OR, _}}) ->
+ [H,R|T] = [A || N <- ['Origin-Host',
+ 'Origin-Realm',
+ 'Session-Id',
+ 'Accounting-Record-Type',
+ 'Accounting-Record-Number'],
+ #diameter_avp{} = A
+ <- [lists:keyfind(N, #diameter_avp.name, Avps)]],
+ Ans = [Hdr#diameter_header{is_request = false},
+ H#diameter_avp{data = OH},
+ R#diameter_avp{data = OR},
+ #diameter_avp{name = 'Experimental-Result',
+ code = 297,
+ need_encryption = false,
+ data = [#diameter_avp{data = {?DIAMETER_DICT_COMMON,
+ 'Vendor-Id',
+ 123}},
+ #diameter_avp{data
+ = {?DIAMETER_DICT_COMMON,
+ 'Experimental-Result-Code',
+ 3987}}]}
+ | T],
+ {reply, Ans};
+
+request(Msg, _Avps, Caps) ->
+ request(Msg, Caps).
+
+%% request/2
+
%% send_nok
request(#diameter_base_accounting_ACR{'Accounting-Record-Number' = 0},
_) ->