From 066544fa0c85e2603f5f0ebfdd8b2f07a8e4a4a7 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 14 May 2013 13:11:32 +0200 Subject: Fix recognition of 5001 on mandatory AVP's An AVP setting the M-bit was not regarded as erroneous if it was defined in the dictionary in question and its container (message or Grouped AVP) had an 'AVP' field. It's now regarded as a 5001 error (AVP_UNSUPPORTED), as in the case that the AVP is not defined. --- lib/diameter/include/diameter_gen.hrl | 34 ++++++++++++++-------------- lib/diameter/test/diameter_traffic_SUITE.erl | 29 ++++++++++++++++++++---- 2 files changed, 42 insertions(+), 21 deletions(-) diff --git a/lib/diameter/include/diameter_gen.hrl b/lib/diameter/include/diameter_gen.hrl index 2126a5623d..55aae3a243 100644 --- a/lib/diameter/include/diameter_gen.hrl +++ b/lib/diameter/include/diameter_gen.hrl @@ -221,7 +221,9 @@ decode(Name, 'AVP', Avp, Acc) -> %% d/3 %% Don't try to decode the value of a Failed-AVP component since it -%% probably won't. +%% probably won't. Note that matching on 'Failed-AVP' assumes that +%% this is the RFC AVP, with code 279. Strictly, this doesn't need to +%% be the case, so we're assuming no one defines another Failed-AVP. d('Failed-AVP' = Name, Avp, Acc) -> decode_AVP(Name, Avp, Acc); @@ -253,17 +255,10 @@ d(Name, Avp, {Avps, Acc}) -> %% decode_AVP/3 %% %% Don't know this AVP: see if it can be packed in an 'AVP' field -%% undecoded, unless it's mandatory. Need to give Failed-AVP special -%% treatment since it'll contain any unrecognized mandatory AVP's. -%% Note that the type field is 'undefined' in this case. - -decode_AVP(Name, #diameter_avp{is_mandatory = M} = Avp, {Avps, Acc}) -> - {[Avp | Avps], if Name == 'Failed-AVP'; - not M -> - pack_AVP(Name, Avp, Acc); - true -> - unknown(Avp, Acc) - end}. +%% undecoded. Note that the type field is 'undefined' in this case. + +decode_AVP(Name, Avp, {Avps, Acc}) -> + {[Avp | Avps], pack_AVP(Name, Avp, Acc)}. %% rc/1 @@ -315,10 +310,18 @@ pack_avp(_, Arity, Avp, Acc) -> %% pack_AVP/3 -pack_AVP(Name, Avp, Acc) -> +%% Give Failed-AVP special treatment since it'll contain any +%% unrecognized mandatory AVP's. +pack_AVP(Name, #diameter_avp{is_mandatory = true} = Avp, Acc) + when Name /= 'Failed-AVP' -> + {Rec, Failed} = Acc, + {Rec, [{5001, Avp} | Failed]}; + +pack_AVP(Name, #diameter_avp{is_mandatory = M} = Avp, Acc) -> case avp_arity(Name, 'AVP') of 0 -> - unknown(Avp, Acc); + {Rec, Failed} = Acc, + {Rec, [{if M -> 5001; true -> 5008 end, Avp} | Failed]}; Arity -> pack(Arity, 'AVP', Avp, Acc) end. @@ -335,9 +338,6 @@ pack_AVP(Name, Avp, Acc) -> %% A message was received with an AVP that MUST NOT be present. The %% Failed-AVP AVP MUST be included and contain a copy of the %% offending AVP. -%% -unknown(#diameter_avp{is_mandatory = B} = Avp, {Rec, Failed}) -> - {Rec, [{if B -> 5001; true -> 5008 end, Avp} | Failed]}. %% pack/4 diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl index 1d5627af5e..42cb4b6844 100644 --- a/lib/diameter/test/diameter_traffic_SUITE.erl +++ b/lib/diameter/test/diameter_traffic_SUITE.erl @@ -54,6 +54,7 @@ send_zero_avp_length/1, send_invalid_avp_length/1, send_invalid_reject/1, + send_unrecognized_mandatory/1, send_long/1, send_nopeer/1, send_noapp/1, @@ -275,6 +276,7 @@ tc() -> send_zero_avp_length, send_invalid_avp_length, send_invalid_reject, + send_unrecognized_mandatory, send_long, send_nopeer, send_noapp, @@ -420,13 +422,14 @@ send_protocol_error(Config) -> ?answer_message(?TOO_BUSY) = call(Config, Req). -%% Send an ASR with an arbitrary AVP and expect success and the same -%% AVP in the reply. +%% Send an ASR with an arbitrary non-mandatory AVP and expect success +%% and the same AVP in the reply. send_arbitrary(Config) -> - Req = ['ASR', {'AVP', [#diameter_avp{name = 'Class', value = "XXX"}]}], + Req = ['ASR', {'AVP', [#diameter_avp{name = 'Product-Name', + value = "XXX"}]}], ['ASA', _SessionId, {'Result-Code', ?SUCCESS} | Avps] = call(Config, Req), - {'AVP', [#diameter_avp{name = 'Class', + {'AVP', [#diameter_avp{name = 'Product-Name', value = "XXX"}]} = lists:last(Avps). @@ -522,6 +525,14 @@ send_invalid_reject(Config) -> ?answer_message(?TOO_BUSY) = call(Config, Req). +%% Send an STR containing a known AVP, but one that's not allowed and +%% sets the M-bit. +send_unrecognized_mandatory(Config) -> + Req = ['STR', {'Termination-Cause', ?LOGOUT}], + + ['STA', _SessionId, {'Result-Code', ?AVP_UNSUPPORTED} | _] + = call(Config, Req). + %% Send something long that will be fragmented by TCP. send_long(Config) -> Req = ['STR', {'Termination-Cause', ?LOGOUT}, @@ -868,6 +879,16 @@ prepare(Pkt, Caps, N, #group{client_dict0 = Dict0} = Group) 0:32/integer, T/binary>>}; +prepare(Pkt, Caps, send_unrecognized_mandatory, #group{client_dict0 = Dict0} + = Group) -> + Req = prepare(Pkt, Caps, Group), + #diameter_packet{bin = <>} + = E + = diameter_codec:encode(Dict0, Pkt#diameter_packet{msg = Req}), + {Code, Flags, undefined} = Dict0:avp_header('Proxy-State'), + Avp = <>, + E#diameter_packet{bin = <>}; + prepare(Pkt, Caps, send_unsupported, #group{client_dict0 = Dict0} = Group) -> Req = prepare(Pkt, Caps, Group), #diameter_packet{bin = <>} -- cgit v1.2.3 From 0b5dbb28332012c22b915364b55ce31843a26bdf Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 16 May 2013 18:40:13 +0200 Subject: Remove redundant integer type specifiers from binaries --- lib/diameter/src/compiler/diameter_codegen.erl | 6 +++--- lib/diameter/test/diameter_codec_test.erl | 10 +++++----- lib/diameter/test/diameter_traffic_SUITE.erl | 19 +++++++------------ 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/lib/diameter/src/compiler/diameter_codegen.erl b/lib/diameter/src/compiler/diameter_codegen.erl index 80036879ea..e687145263 100644 --- a/lib/diameter/src/compiler/diameter_codegen.erl +++ b/lib/diameter/src/compiler/diameter_codegen.erl @@ -574,12 +574,12 @@ cs_enumerated_avp({AvpName, Values}) -> lists:flatmap(fun(V) -> c_enumerated_avp(AvpName, V) end, Values). c_enumerated_avp(AvpName, {_,I}) -> - [{?clause, [?ATOM(decode), ?Atom(AvpName), ?TERM(<>)], + [{?clause, [?ATOM(decode), ?Atom(AvpName), ?TERM(<>)], [], [?TERM(I)]}, {?clause, [?ATOM(encode), ?Atom(AvpName), ?INTEGER(I)], [], - [?TERM(<>)]}]. + [?TERM(<>)]}]. %%% ------------------------------------------------------------------------ %%% msg_header/1 @@ -700,7 +700,7 @@ c_empty_value({Name, _, _, _}) -> c_empty_value({Name, _}) -> {?clause, [?Atom(Name)], [], - [?TERM(<<0:32/integer>>)]}. + [?TERM(<<0:32>>)]}. %%% ------------------------------------------------------------------------ %%% # dict/0 diff --git a/lib/diameter/test/diameter_codec_test.erl b/lib/diameter/test/diameter_codec_test.erl index 0baac59c1a..24d4c7665e 100644 --- a/lib/diameter/test/diameter_codec_test.erl +++ b/lib/diameter/test/diameter_codec_test.erl @@ -265,7 +265,7 @@ arity(M, Name, AvpName, Rec) -> %% enum/3 enum(M, Name, {_,E}) -> - B = <>, + B = <>, B = M:avp(encode, E, Name), E = M:avp(decode, B, Name). @@ -322,15 +322,15 @@ values('Unsigned64') -> values('Float32') -> E = (1 bsl 8) - 2, F = (1 bsl 23) - 1, - <> = <<0:1/integer, E:8/integer, F:23/integer>>, - <> = <<1:1/integer, E:8/integer, F:23/integer>>, + <> = <<0:1, E:8, F:23>>, + <> = <<1:1, E:8, F:23>>, {[0.0, infinity, '-infinity', Mx, Mn], [0]}; values('Float64') -> E = (1 bsl 11) - 2, F = (1 bsl 52) - 1, - <> = <<0:1/integer, E:11/integer, F:52/integer>>, - <> = <<1:1/integer, E:11/integer, F:52/integer>>, + <> = <<0:1, E:11, F:52>>, + <> = <<1:1, E:11, F:52>>, {[0.0, infinity, '-infinity', Mx, Mn], [0]}; values('Address') -> diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl index 42cb4b6844..21a9625fa1 100644 --- a/lib/diameter/test/diameter_traffic_SUITE.erl +++ b/lib/diameter/test/diameter_traffic_SUITE.erl @@ -847,7 +847,7 @@ prepare(Pkt, Caps, N, #group{client_dict0 = Dict0} = Group) = diameter_codec:encode(Dict0, Pkt#diameter_packet{msg = Req}), Offset = L - 24, %% to Auth-Application-Id <> = B, AL = case N of @@ -857,7 +857,7 @@ prepare(Pkt, Caps, N, #group{client_dict0 = Dict0} = Group) end, E#diameter_packet{bin = <>}; + Hdr/binary, AL:24, Data/binary>>}; prepare(Pkt, Caps, N, #group{client_dict0 = Dict0} = Group) when N == send_invalid_avp_length; @@ -870,14 +870,9 @@ prepare(Pkt, Caps, N, #group{client_dict0 = Dict0} = Group) = E = diameter_codec:encode(Dict0, Pkt#diameter_packet{msg = Req}), Offset = L - 7 - 12, %% to AVP Length - <> = B0, - <> = H0, %% assert - E#diameter_packet{bin = <>}; + <> = B0, + <> = H0, %% assert + E#diameter_packet{bin = <>}; prepare(Pkt, Caps, send_unrecognized_mandatory, #group{client_dict0 = Dict0} = Group) -> @@ -894,7 +889,7 @@ prepare(Pkt, Caps, send_unsupported, #group{client_dict0 = Dict0} = Group) -> #diameter_packet{bin = <>} = E = diameter_codec:encode(Dict0, Pkt#diameter_packet{msg = Req}), - E#diameter_packet{bin = <>}; + E#diameter_packet{bin = <>}; prepare(Pkt, Caps, send_unsupported_app, #group{client_dict0 = Dict0} = Group) -> @@ -902,7 +897,7 @@ prepare(Pkt, Caps, send_unsupported_app, #group{client_dict0 = Dict0} #diameter_packet{bin = <>} = E = diameter_codec:encode(Dict0, Pkt#diameter_packet{msg = Req}), - E#diameter_packet{bin = <>}; + E#diameter_packet{bin = <>}; prepare(Pkt, Caps, send_error_bit, Group) -> #diameter_packet{header = Hdr} = Pkt, -- cgit v1.2.3