From c2d5c74d231b62cc511f3927201ac6578844b578 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 13 Jun 2013 13:18:39 +0200 Subject: Fix list-valued Vendor-Specific-Application-Id config Commit f0a36c79 broke the handling of such configuration, resulting in a function clause error in diameter_capx if the list was not of length 3, and faulty extraction of application id's otherwise. Only record-valued config was properly interpreted. --- lib/diameter/src/base/diameter_capx.erl | 9 ++-- lib/diameter/test/diameter_capx_SUITE.erl | 82 +++++++++++++++++++++++++++---- 2 files changed, 77 insertions(+), 14 deletions(-) (limited to 'lib') diff --git a/lib/diameter/src/base/diameter_capx.erl b/lib/diameter/src/base/diameter_capx.erl index 4b821f5139..1a931a9854 100644 --- a/lib/diameter/src/base/diameter_capx.erl +++ b/lib/diameter/src/base/diameter_capx.erl @@ -419,12 +419,13 @@ app_union(#diameter_caps{auth_application_id = U, vendor_specific_application_id = V}) -> set_list(U ++ C ++ lists:flatmap(fun vsa_apps/1, V)). -vsa_apps([_ | [_,_] = Ids]) -> - lists:append(Ids); +vsa_apps(Vals) + when is_list(Vals) -> + lists:flatmap(fun({'Vendor-Id', _}) -> []; ({_, Ids}) -> Ids end, Vals); vsa_apps(Rec) when is_tuple(Rec) -> - [_|T] = tuple_to_list(Rec), - vsa_apps(T). + [_Name, _VendorId | Idss] = tuple_to_list(Rec), + lists:append(Idss). %% It's a configuration error for a locally advertised application not %% to be represented in Apps. Don't just match on lists:keyfind/3 in diff --git a/lib/diameter/test/diameter_capx_SUITE.erl b/lib/diameter/test/diameter_capx_SUITE.erl index 8c9bb67e61..deabdd720b 100644 --- a/lib/diameter/test/diameter_capx_SUITE.erl +++ b/lib/diameter/test/diameter_capx_SUITE.erl @@ -27,6 +27,8 @@ -export([suite/0, all/0, groups/0, + init_per_suite/1, + end_per_suite/1, init_per_group/2, end_per_group/2, init_per_testcase/2, @@ -56,6 +58,11 @@ peer_down/4]). -include("diameter.hrl"). +-include("diameter_gen_base_rfc3588.hrl"). +%% Use only the Vendor-Specific-Application-Id record from the base +%% include, to test the independence of capabilities configuration +%% from the different definitions of Vendor-Id in RFC's 3588 and RFC +%% 6733. %% =========================================================================== @@ -69,6 +76,11 @@ -define(REALM, "erlang.org"). -define(HOST(Name), Name ++ "." ++ ?REALM). +%% Application id's that are never agreed upon at capabilities +%% exchange. Testcase no_common_application references them in order +%% to exercise Vendor-Specific-Application-Id handling. +-define(NOAPPS, [1111, 2222, 3333, 4444]). + %% Config for diameter:start_service/2. -define(SERVICE, [{'Origin-Realm', ?REALM}, @@ -83,7 +95,10 @@ || {A,D} <- [{base3588, diameter_gen_base_rfc3588}, {acct3588, diameter_gen_base_accounting}, {base6733, diameter_gen_base_rfc6733}, - {acct6733, diameter_gen_acct_rfc6733}]]]). + {acct6733, diameter_gen_acct_rfc6733}]]] + ++ [{application, [{dictionary, dict(N)}, + {module, not_really}]} + || N <- ?NOAPPS]). -define(A, list_to_atom). -define(L, atom_to_list). @@ -116,6 +131,16 @@ groups() -> Tc = lists:flatmap(fun tc/1, tc()), [{D, [], Tc} || D <- ?DICTS]. +init_per_suite(Config) -> + lists:foreach(fun load_dict/1, ?NOAPPS), + Config. + +end_per_suite(_Config) -> + [] = [Mod || N <- ?NOAPPS, + Mod <- [dict(N)], + false <- [code:delete(Mod)]], + ok. + %% Generate a unique hostname for each testcase so that watchdogs %% don't prevent a connection from being brought up immediately. init_per_testcase(Name, Config) -> @@ -160,7 +185,7 @@ start(_Config) -> ok = diameter:start(). %% Ensure that both integer and list-valued vendor id's can be -%% configured in a 'Vendor-Specific-Application-Id, the arity having +%% configured in a Vendor-Specific-Application-Id, the arity having %% changed between RFC 3588 and RFC 6733. vendor_id(_Config) -> [] = ?util:run([[fun vid/1, V] || V <- [1, [1], [1,2], x]]). @@ -188,13 +213,13 @@ add_listeners(Config) -> Acct = [listen(?SERVER, [{capabilities, [{'Origin-Host', ?HOST(H)}, {'Auth-Application-Id', []}]}, - {applications, [A]}, + {applications, [A | noapps()]}, {capabilities_cb, [fun server_capx/3, acct]}]) || {A,H} <- [{acct3588, "acct3588-srv"}, {acct6733, "acct6733-srv"}]], Base = [listen(?SERVER, [{capabilities, [{'Origin-Host', ?HOST(H)}]}, - {applications, A}, + {applications, A ++ noapps()}, {capabilities_cb, [fun server_capx/3, base]}]) || {A,H} <- [{[base3588, acct3588], "base3588-srv"}, {[base6733, acct6733], "base6733-srv"}]], @@ -224,15 +249,33 @@ stop(_Config) -> %% DIAMETER_NO_COMMON_APPLICATION = 5010. s_no_common_application(Config) -> - server_closed(Config, fun no_common_application/1, 5010). + Vs = [[{'Vendor-Id', 111}, + {'Auth-Application-Id', [1111]}], + #'diameter_base_Vendor-Specific-Application-Id' + {'Vendor-Id' = [222], + 'Acct-Application-Id' = [2222]}], + server_closed(Config, + fun(C) -> no_common_application(C,Vs) end, + 5010). c_no_common_application(Config) -> - client_closed(Config, "acct-srv", fun no_common_application/1, 5010). - -no_common_application(Config) -> + Vs = [#'diameter_base_Vendor-Specific-Application-Id' + {'Vendor-Id' = 333, + 'Auth-Application-Id' = [3333]}, + [{'Vendor-Id', [444]}, + {'Acct-Application-Id', [4444]}]], + client_closed(Config, + "acct-srv", + fun(C) -> no_common_application(C,Vs) end, + 5010). + +no_common_application(Config, Vs) -> [Common, _Acct] = apps(Config), - connect(Config, acct, [{capabilities, [{'Acct-Application-Id', []}]}, - {applications, [Common]}]). + connect(Config, + acct, + [{capabilities, [{'Acct-Application-Id', []}, + {'Vendor-Specific-Application-Id', Vs}]}, + {applications, [Common | noapps()]}]). %% ==================== %% Ask the base server to speak accounting with an unknown security @@ -324,6 +367,25 @@ client_reject(Config) -> %% =========================================================================== +noapps() -> + lists:map(fun dict/1, ?NOAPPS). + +dict(N) -> + ?A(?L(?MODULE) ++ "_" ++ integer_to_list(N)). + +%% Compile and load minimal dictionary modules. These actually have to +%% exists since diameter will call their id/0 to extract application +%% id's, failing with app_not_configured if it can't. +load_dict(N) -> + Mod = dict(N), + Forms = [{attribute, 1, module, Mod}, + {attribute, 2, compile, [export_all]}, + {function, 3, id, 0, + [{clause, 4, [], [], [{integer, 4, N}]}]}], + {ok, Mod, Bin, []} = compile:forms(Forms, [return]), + {module, Mod} = code:load_binary(Mod, Mod, Bin), + N = Mod:id(). + %% server_closed/3 server_closed(Config, F, RC) -> -- cgit v1.2.3