aboutsummaryrefslogtreecommitdiffstats
path: root/lib/diameter
diff options
context:
space:
mode:
authorAnders Svensson <[email protected]>2011-12-07 11:50:22 +0100
committerAnders Svensson <[email protected]>2011-12-08 00:45:19 +0100
commitb4ecd2efd32df9d14d3314b705cd1114de7141ae (patch)
tree043af124f7367387110c7ceb4ad181026b076be0 /lib/diameter
parente2c8662b4fd5d6100ab301cd6c21f9a8d34b7f71 (diff)
downloadotp-b4ecd2efd32df9d14d3314b705cd1114de7141ae.tar.gz
otp-b4ecd2efd32df9d14d3314b705cd1114de7141ae.tar.bz2
otp-b4ecd2efd32df9d14d3314b705cd1114de7141ae.zip
Fix semantic checks on AVP qualifiers
Didn't quite interpret '*' as RFC 3588 dictates. In particular, the interpretation depends on what's being qualified, a required, optional or fixed AVP.
Diffstat (limited to 'lib/diameter')
-rw-r--r--lib/diameter/src/compiler/diameter_codegen.erl10
-rw-r--r--lib/diameter/src/compiler/diameter_dict_util.erl83
-rw-r--r--lib/diameter/test/diameter_compiler_SUITE.erl156
3 files changed, 211 insertions, 38 deletions
diff --git a/lib/diameter/src/compiler/diameter_codegen.erl b/lib/diameter/src/compiler/diameter_codegen.erl
index 2c011376ad..489f521f70 100644
--- a/lib/diameter/src/compiler/diameter_codegen.erl
+++ b/lib/diameter/src/compiler/diameter_codegen.erl
@@ -835,15 +835,15 @@ avp_info(Entry) -> %% {Name, Arity}
[A] -> {A, {0,1}};
{Q,T} ->
{A,_} = avp_info(T),
- {A, arity(Q)}
+ {A, arity(T,Q)}
end.
%% Normalize arity to 1 or {N,X} where N is an integer. A record field
%% for an AVP is list-valued iff the normalized arity is not 1.
-arity('*' = Inf) -> {0, Inf};
-arity({'*', N}) -> {0, N};
-arity({1,1}) -> 1;
-arity(T) -> T.
+arity({{_}}, '*' = Inf) -> {0, Inf};
+arity([_], '*' = Inf) -> {0, Inf};
+arity({_}, '*' = Inf) -> {1, Inf};
+arity(_, {_,_} = Q) -> Q.
prefix(Spec) ->
case orddict:find(prefix, Spec) of
diff --git a/lib/diameter/src/compiler/diameter_dict_util.erl b/lib/diameter/src/compiler/diameter_dict_util.erl
index 2207925e49..e4cd29ab7f 100644
--- a/lib/diameter/src/compiler/diameter_dict_util.erl
+++ b/lib/diameter/src/compiler/diameter_dict_util.erl
@@ -174,7 +174,13 @@ fmt(message_application_id_mismatch) ->
fmt(invalid_avp_order) ->
"AVP reference ~c~s~c at line ~p breaks fixed/required/optional order";
-fmt(invalid_qualifier) ->
+fmt(required_avp_has_zero_max_arity) ->
+ "Required AVP has maximum arity 0 at line ~p";
+fmt(required_avp_has_zero_min_arity) ->
+ "Required AVP has minimum arity 0 at line ~p";
+fmt(optional_avp_has_nonzero_min_arity) ->
+ "Optional AVP has non-zero minimum arity at line ~p";
+fmt(qualifier_has_min_greater_than_max) ->
"Qualifier ~p*~p at line ~p has Min > Max";
fmt(avp_already_referenced) ->
"AVP ~s at line ~p already referenced at line ~p";
@@ -300,7 +306,7 @@ body(avp_types, {Name, Code, Type, Flags}) ->
body(messages, {"answer-message", _, _, [], Avps}) ->
[?NL, ?NL, ?INDENT,
"answer-message ::= < Diameter Header: code, ERR [PXY] >",
- f_avps(Avps)];
+ f_avps(Avps)];
body(messages, {Name, Code, Flags, ApplId, Avps}) ->
[?NL, ?NL, ?INDENT, word(Name), " ::= ", header(Code, Flags, ApplId),
f_avps(Avps)];
@@ -326,7 +332,8 @@ f_avps(L) ->
[[?NL, ?INDENT, ?INDENT, f_avp(A)] || A <- L].
f_avp({Q, A}) ->
- f_avp(f_qual(Q), f_delim(A));
+ [D | _] = Avp = f_delim(A),
+ f_avp(f_qual(D, Q), Avp);
f_avp(A) ->
f_avp("", f_delim(A)).
@@ -337,17 +344,19 @@ f_delim({A}) ->
f_delim([A]) ->
[$[, word(A), $]].
-f_avp(Q, Avp) ->
+f_avp(Q, [L, Avp, R]) ->
Len = length(lists:flatten([Q])),
- [io_lib:format("~*s", [-1*max(Len+1, 6) , Q]), Avp].
+ [io_lib:format("~*s", [-1*max(Len+1, 6) , Q]), L, " ", Avp, " ", R].
-f_qual('*') ->
+f_qual(${, '*') ->
+ "1*"; %% Equivalent to "*" but the more common/obvious rendition
+f_qual(_, '*') ->
"*";
-f_qual({'*', N}) ->
+f_qual(_, {'*', N}) ->
[$*, ?I(N)];
-f_qual({N, '*'}) ->
+f_qual(_, {N, '*'}) ->
[?I(N), $*];
-f_qual({M,N}) ->
+f_qual(_, {M,N}) ->
[?I(M), $*, ?I(N)].
section(Key) ->
@@ -500,7 +509,17 @@ make_body(Avps) ->
avp([false, D, Avp]) ->
avp(D, Avp);
avp([Q, D, Avp]) ->
- {qual(Q), avp(D, Avp)}.
+ case {qual(D, Q), avp(D, Avp)} of
+ {{0,1}, A} when D == $[ ->
+ A;
+ {{1,1}, A} ->
+ A;
+ T ->
+ T
+ end.
+%% Could just store the qualifier as a pair in all cases but the more
+%% compact form is easier to parse visually so live with a bit of
+%% mapping. Ditto the use of '*'.
avp(D, {'AVP', _}) ->
delim(D, "AVP");
@@ -514,14 +533,40 @@ delim(${, N) ->
delim($[, N) ->
[N].
-qual({true, {_,_,N}}) ->
- {'*', N};
-qual({{_,_,N}, true}) ->
+%% There's a difference between max = 0 and not specifying an AVP:
+%% reception of an AVP with max = 0 will always be an error, otherwise
+%% it depends on the existence of 'AVP' and the M flag.
+
+qual(${, {{_,L,0}, _}) ->
+ ?RETURN(required_avp_has_zero_min_arity, [L]);
+qual(${, {_, {_,L,0}}) ->
+ ?RETURN(required_avp_has_zero_max_arity, [L]);
+
+qual($[, {{_,L,N}, _})
+ when 0 < N ->
+ ?RETURN(optional_avp_has_nonzero_min_arity, [L]);
+
+qual(_, {{_,L,Min}, {_,_,Max}})
+ when Min > Max ->
+ ?RETURN(qualifier_has_min_greater_than_max, [Min, Max, L]);
+
+qual(_, true) ->
+ '*';
+
+qual(${, {true, {_,_,N}}) ->
+ {1, N};
+qual(_, {true, {_,_,N}}) ->
+ {0, N};
+
+qual(D, {{_,_,N}, true})
+ when D == ${, N == 1;
+ D /= ${, N == 0 ->
+ '*';
+qual(_, {{_,_,N}, true}) ->
{N, '*'};
-qual({{_,_,N},{_,_,M}}) ->
- {N, M};
-qual(true) ->
- '*'.
+
+qual(_, {{_,_,Min}, {_,_,Max}}) ->
+ {Min, Max}.
%% Optional reports when running verbosely.
report(What, [F | A])
@@ -851,10 +896,6 @@ xa(Ds, [[Qual, D, {'AVP', Line}] | Avps], Dict, Key, Name) ->
xa([], [[_Qual, D, {_, Line, Name}] | _], _, _, _) ->
?RETURN(invalid_avp_order, [D, Name, close(D), Line]);
-xa([D|_], [[{{_, Line, Min}, {_, _, Max}}, D, _] | _], _, _, _)
- when Min > Max ->
- ?RETURN(invalid_qualifier, [Min, Max, Line]);
-
xa([D|_] = Ds, [[Qual, D, {_, Line, AvpName}] | Avps], Dict, Key, Name) ->
xa(Ds,
Avps,
diff --git a/lib/diameter/test/diameter_compiler_SUITE.erl b/lib/diameter/test/diameter_compiler_SUITE.erl
index cc4b1ddac5..8f563ff2ab 100644
--- a/lib/diameter/test/diameter_compiler_SUITE.erl
+++ b/lib/diameter/test/diameter_compiler_SUITE.erl
@@ -22,7 +22,6 @@
%%
-module(diameter_compiler_SUITE).
--compile({no_auto_import, [error/2]}).
-export([suite/0,
all/0,
@@ -30,7 +29,7 @@
end_per_suite/1]).
%% testcases
--export([format/1,
+-export([format/1, format/2,
replace/1, replace/2]).
-export([dict/0]). %% fake dictionary module
@@ -46,7 +45,10 @@
%% returned in the first element of the error tuple returned by
%% diameter_dict_util:parse/2.
-define(REPLACE,
- [{scan,
+ [{ok,
+ "",
+ ""},
+ {scan,
"@id 0",
"@id \\&"},
{scan,
@@ -154,9 +156,126 @@
{invalid_avp_order,
"CEA ::=",
"{Result-Code} &"},
- {invalid_qualifier,
- "CEA ::=.*",
- "& 3*2"},
+ {ok,
+ "{ Product-Name",
+ "* &"},
+ {required_avp_has_zero_max_arity,
+ "{ Product-Name",
+ "*0 &"},
+ {required_avp_has_zero_min_arity,
+ "{ Product-Name",
+ "0* &"},
+ {required_avp_has_zero_min_arity,
+ "{ Product-Name",
+ "0*0 &"},
+ {ok,
+ "{ Product-Name",
+ "*1 &"},
+ {ok,
+ "{ Product-Name",
+ "1* &"},
+ {ok,
+ "{ Product-Name",
+ "1*1 &"},
+ {ok,
+ "{ Product-Name",
+ "2* &"},
+ {ok,
+ "{ Product-Name",
+ "*2 &"},
+ {ok,
+ "{ Product-Name",
+ "2*2 &"},
+ {ok,
+ "{ Product-Name",
+ "2*3 &"},
+ {qualifier_has_min_greater_than_max,
+ "{ Product-Name",
+ "3*2 &"},
+ {ok,
+ "\\[ Origin-State-Id",
+ "* &"},
+ {ok,
+ "\\[ Origin-State-Id",
+ "0* &"},
+ {ok,
+ "\\[ Origin-State-Id",
+ "*0 &"},
+ {ok,
+ "\\[ Origin-State-Id",
+ "0*0 &"},
+ {ok,
+ "\\[ Origin-State-Id",
+ "0*1 &"},
+ {ok,
+ "\\[ Origin-State-Id",
+ "0*2 &"},
+ {ok,
+ "\\[ Origin-State-Id",
+ "*1 &"},
+ {optional_avp_has_nonzero_min_arity,
+ "\\[ Origin-State-Id",
+ "1* &"},
+ {optional_avp_has_nonzero_min_arity,
+ "\\[ Origin-State-Id",
+ "1*1 &"},
+ {ok,
+ "\\[ Origin-State-Id",
+ "*2 &"},
+ {optional_avp_has_nonzero_min_arity,
+ "\\[ Origin-State-Id",
+ "2* &"},
+ {optional_avp_has_nonzero_min_arity,
+ "\\[ Origin-State-Id",
+ "2*2 &"},
+ {optional_avp_has_nonzero_min_arity,
+ "\\[ Origin-State-Id",
+ "2*3 &"},
+ {optional_avp_has_nonzero_min_arity,
+ "\\[ Origin-State-Id",
+ "3*2 &"},
+ {ok,
+ "^ *< Session-Id",
+ "* &"},
+ {ok,
+ "^ *< Session-Id",
+ "*0 &"},
+ {ok,
+ "^ *< Session-Id",
+ "0* &"},
+ {ok,
+ "^ *< Session-Id",
+ "0*0 &"},
+ {ok,
+ "^ *< Session-Id",
+ "0*1 &"},
+ {ok,
+ "^ *< Session-Id",
+ "0*2 &"},
+ {ok,
+ "^ *< Session-Id",
+ "*1 &"},
+ {ok,
+ "^ *< Session-Id",
+ "1* &"},
+ {ok,
+ "^ *< Session-Id",
+ "1*1 &"},
+ {ok,
+ "^ *< Session-Id",
+ "*2 &"},
+ {ok,
+ "^ *< Session-Id",
+ "2* &"},
+ {ok,
+ "^ *< Session-Id",
+ "2*2 &"},
+ {ok,
+ "^ *< Session-Id",
+ "2*3 &"},
+ {qualifier_has_min_greater_than_max,
+ "^ *< Session-Id",
+ "3*2 &"},
{avp_already_referenced,
"CER ::=.*",
"& {Origin-Host}"},
@@ -232,7 +351,13 @@ end_per_suite(_Config) ->
%% Ensure that parse o format is the identity map.
format(Config) ->
Bin = proplists:get_value(base, Config),
- {ok, Dict} = diameter_dict_util:parse(Bin, []),
+ [] = ?util:run([{?MODULE, [format, M, Bin]}
+ || E <- ?REPLACE,
+ {ok, M} <- [norm(E)]]).
+
+format(Mods, Bin) ->
+ B = modify(Bin, Mods),
+ {ok, Dict} = diameter_dict_util:parse(B, []),
{ok, D} = diameter_dict_util:parse(diameter_dict_util:format(Dict), []),
{Dict, Dict} = {Dict, D}.
@@ -240,13 +365,12 @@ format(Config) ->
replace(Config) ->
Bin = proplists:get_value(base, Config),
- [] = ?util:run([{?MODULE, [replace, E, Bin]} || E <- ?REPLACE]).
-
-replace({E, RE, Repl}, Bin) ->
- replace({E, [{RE, Repl}]}, Bin);
+ [] = ?util:run([{?MODULE, [replace, N, Bin]}
+ || E <- ?REPLACE,
+ N <- [norm(E)]]).
replace({E, Mods}, Bin) ->
- B = iolist_to_binary(lists:foldl(fun re/2, Bin, Mods)),
+ B = modify(Bin, Mods),
case {E, diameter_dict_util:parse(B, [{include, here()}]), Mods} of
{ok, {ok, Dict}, _} ->
Dict;
@@ -262,6 +386,14 @@ re({RE, Repl}, Bin) ->
%% ===========================================================================
+modify(Bin, Mods) ->
+ lists:foldl(fun re/2, Bin, Mods).
+
+norm({E, RE, Repl}) ->
+ {E, [{RE, Repl}]};
+norm({_,_} = T) ->
+ T.
+
nochar(Char, Str, Err) ->
Err == parse orelse not lists:member(Char, Str) orelse Str.