From 501b27d75cdf9623756848fc09cf16e59fe523ce Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 22 Aug 2017 14:45:39 +0200 Subject: Rename transport_opt() capx_strictness to strict_capx To follow the naming of options like strict_mbit and more. Still accept capx_strictness since this is known to be used. Introduced in commit e4f28f3b. --- lib/diameter/src/base/diameter.erl | 2 +- lib/diameter/src/base/diameter_config.erl | 2 ++ lib/diameter/src/base/diameter_peer_fsm.erl | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) (limited to 'lib/diameter') diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index 2e18a1d903..888a108f8b 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -394,7 +394,7 @@ call(SvcName, App, Message) -> | {capabilities, [capability()]} | {capabilities_cb, evaluable()} | {capx_timeout, 'Unsigned32'()} - | {capx_strictness, boolean()} + | {strict_capx, boolean()} | {disconnect_cb, evaluable()} | {dpr_timeout, 'Unsigned32'()} | {dpa_timeout, 'Unsigned32'()} diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index f1b6e56782..d54dbd9976 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -591,6 +591,8 @@ opt({K, Tmo}) ?IS_UINT32(Tmo); opt({capx_strictness, B}) -> + is_boolean(B) andalso {value, {strict_capx, B}}; +opt({strict_capx, B}) -> is_boolean(B); opt({length_errors, T}) -> diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index 4870b86be5..77ee3d6057 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -239,7 +239,7 @@ i({Ack, WPid, {M, Ref} = T, Opts, {SvcOpts, Nodes, Dict0, Svc}}) -> proplists:get_value(dpa_timeout, Opts, ?DPA_TIMEOUT)}), Tmo = proplists:get_value(capx_timeout, Opts, ?CAPX_TIMEOUT), - Strictness = proplists:get_value(capx_strictness, Opts, true), + Strict = proplists:get_value(strict_capx, Opts, true), LengthErr = proplists:get_value(length_errors, Opts, exit), {TPid, Addrs} = start_transport(T, Rest, Svc), @@ -253,7 +253,7 @@ i({Ack, WPid, {M, Ref} = T, Opts, {SvcOpts, Nodes, Dict0, Svc}}) -> mode = M, service = svc(Svc, Addrs), length_errors = LengthErr, - strict = Strictness, + strict = Strict, incoming_maxlen = Maxlen, codec = maps:with([decode_format, string_decode, -- cgit v1.2.3 From 3c7e35ca4512088f0b4074f809f3110dfa285b8e Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 22 Aug 2017 15:37:04 +0200 Subject: Document transport_opt() strict_capx --- lib/diameter/doc/src/diameter.xml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'lib/diameter') diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml index 3ad24257a5..264dbf9152 100644 --- a/lib/diameter/doc/src/diameter.xml +++ b/lib/diameter/doc/src/diameter.xml @@ -1409,6 +1409,20 @@ Options monitor and link are ignored.

Defaults to the list configured on the service if not specified.

+ +{strict_capx, boolean()]} + +

+Whether or not to enforce the RFC 6733 requirement that any message +before capabilities exchange should close the peer connection. +If false then unexpected messages are discarded.

+ +

+Defaults to true. +Changing this results in non-standard behaviour, but can be useful in +case peers are known to be behave badly.

+ + {transport_config, term()} {transport_config, term(), &dict_Unsigned32; | infinity} -- cgit v1.2.3 From 2d540b95755a6f628b0cfc5a9bab4ff41046fe4b Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 24 Aug 2017 11:51:44 +0200 Subject: Rename type evaluable -> eval Export the old type as a synonym for backwards compatability. The name evaluable is a bit too awkward. --- lib/diameter/doc/src/diameter.xml | 30 ++++++++++++++-------------- lib/diameter/doc/src/diameter_app.xml | 9 +++++---- lib/diameter/doc/src/seealso.ent | 4 ++-- lib/diameter/src/base/diameter.erl | 22 +++++++++++--------- lib/diameter/src/base/diameter_callback.erl | 6 +++--- lib/diameter/src/transport/diameter_sctp.erl | 6 +++--- lib/diameter/src/transport/diameter_tcp.erl | 4 ++-- 7 files changed, 43 insertions(+), 38 deletions(-) (limited to 'lib/diameter') diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml index 264dbf9152..6a4a7b9f7c 100644 --- a/lib/diameter/doc/src/diameter.xml +++ b/lib/diameter/doc/src/diameter.xml @@ -397,10 +397,10 @@ from the peer offers it.

Note that each tuple communicates one or more AVP values. It is an error to specify duplicate tuples.

- + -evaluable() = {M,F,A} | fun() | [evaluable() | A] +eval() = {M,F,A} | fun() | [eval() | A]

An expression that can be evaluated as a function in the following @@ -418,7 +418,7 @@ eval(F) ->

-Applying an &evaluable; +Applying an &eval; E to an argument list A is meant in the sense of eval([E|A]).

@@ -484,11 +484,11 @@ Matches only those peers whose Origin-Realm has the specified value, or all peers if the atom any.

 -{eval, &evaluable;} +{eval, &eval;}

Matches only those peers for which the specified -&evaluable; returns +&eval; returns true when applied to the connection's diameter_caps record. Any other return value or exception is equivalent to false.

@@ -650,7 +650,7 @@ Result = ResultCode | {capabilities_cb, CB, ResultCode|discard} Caps = #diameter_caps{} Pkt = #diameter_packet{} ResultCode = integer() -CB = &evaluable; +CB = &eval;

@@ -843,7 +843,7 @@ Length field in a Diameter Header.

| node | nodes | [node()] - | evaluable()} + | eval()}

The degree to which the service allows multiple transport @@ -854,7 +854,7 @@ at capabilities exchange.

If [node()] then a connection is rejected if another already exists on any of the specified nodes. Types false, node, nodes and -&evaluable; are equivalent to +&eval; are equivalent to [], [node()], [node()|nodes()] and the evaluated value respectively, evaluation of each expression taking place whenever a new connection is to be established. @@ -869,7 +869,7 @@ by their own peer and watchdog state machines.

Defaults to nodes.

 -{sequence, {H,N} | &evaluable;} +{sequence, {H,N} | &eval;}

A constant value H for the topmost 32-N bits of @@ -904,7 +904,7 @@ outgoing requests.

 -{share_peers, boolean() | [node()] | evaluable()} +{share_peers, boolean() | [node()] | eval()}

Nodes to which peer connections established on the local @@ -917,7 +917,7 @@ configured to use them: see use_shared_peers below.

If false then peers are not shared. If [node()] then peers are shared with the specified list of nodes. -If evaluable() then peers are shared with the nodes returned +If eval() then peers are shared with the nodes returned by the specified function, evaluated whenever a peer connection becomes available or a remote service requests information about local connections. @@ -1073,7 +1073,7 @@ omitted counters are not returned by &service_info;.

 -{use_shared_peers, boolean() | [node()] | evaluable()} +{use_shared_peers, boolean() | [node()] | eval()}

Nodes from which communicated peers are made available in @@ -1083,7 +1083,7 @@ the remote candidates list of &app_pick_peer; callbacks.

If false then remote peers are not used. If [node()] then only peers from the specified list of nodes are used. -If evaluable() then only peers returned by the specified +If eval() then only peers returned by the specified function are used, evaluated whenever a remote service communicates information about an available peer connection. The value true is equivalent to fun &nodes;. @@ -1155,7 +1155,7 @@ TLS is desired over TCP as implemented by &man_tcp;.

 -{capabilities_cb, &evaluable;} +{capabilities_cb, &eval;}

Callback invoked upon reception of CER/CEA during capabilities @@ -1249,7 +1249,7 @@ transport.

 -{disconnect_cb, &evaluable;} +{disconnect_cb, &eval;}

Callback invoked prior to terminating the transport process of a diff --git a/lib/diameter/doc/src/diameter_app.xml b/lib/diameter/doc/src/diameter_app.xml index dfcd00975b..aa334beb21 100644 --- a/lib/diameter/doc/src/diameter_app.xml +++ b/lib/diameter/doc/src/diameter_app.xml @@ -13,7 +13,8 @@

-20112016 +2011 +2017 Ericsson AB. All Rights Reserved. @@ -319,7 +320,7 @@ or &peer_down; callback.

Action = Send | Discard | {eval_packet, Action, PostF} Send = {send, &packet; | &message;} Discard = {discard, Reason} | discard -PostF = &mod_evaluable;} +PostF = &mod_eval;}

@@ -371,7 +372,7 @@ discarded}.

Action = Send | Discard | {eval_packet, Action, PostF} Send = {send, &packet; | &message;} Discard = {discard, Reason} | discard -PostF = &mod_evaluable;} +PostF = &mod_eval;}

@@ -478,7 +479,7 @@ not selected.

| {answer_message, 3000..3999|5000..5999} | {protocol_error, 3000..3999} Opt = &mod_call_opt; -PostF = &mod_evaluable; +PostF = &mod_eval;

diff --git a/lib/diameter/doc/src/seealso.ent b/lib/diameter/doc/src/seealso.ent index e5c284c6e8..ef6af1a3d0 100644 --- a/lib/diameter/doc/src/seealso.ent +++ b/lib/diameter/doc/src/seealso.ent @@ -4,7 +4,7 @@ %CopyrightBegin% -Copyright Ericsson AB 2012-2015. All Rights Reserved. +Copyright Ericsson AB 2012-2017. All Rights Reserved. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -53,7 +53,7 @@ significant. diameter:application_opt()'> diameter:call_opt()'> diameter:capability()'> -diameter:evaluable()'> +diameter:eval()'> diameter:peer_filter()'> diameter:service_event()'> diameter:service_event_info()'> diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index 888a108f8b..ad6aaab440 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -46,7 +46,8 @@ -export([start/0, stop/0]). --export_type([evaluable/0, +-export_type([eval/0, + evaluable/0, %% deprecated decode_format/0, strict_arities/0, restriction/0, @@ -301,7 +302,7 @@ call(SvcName, App, Message) -> | realm | {host, any|'DiameterIdentity'()} | {realm, any|'DiameterIdentity'()} - | {eval, evaluable()} + | {eval, eval()} | {neg, peer_filter()} | {all, [peer_filter()]} | {any, [peer_filter()]}. @@ -309,10 +310,13 @@ call(SvcName, App, Message) -> -opaque peer_ref() :: pid(). --type evaluable() +-type eval() :: {module(), atom(), list()} | fun() - | maybe_improper_list(evaluable(), list()). + | maybe_improper_list(eval(), list()). + +-type evaluable() + :: eval(). -type sequence() :: {'Unsigned32'(), 0..32}. @@ -322,12 +326,12 @@ call(SvcName, App, Message) -> | node | nodes | [node()] - | evaluable(). + | eval(). -type remotes() :: boolean() | [node()] - | evaluable(). + | eval(). -type message_length() :: 0..16#FFFFFF. @@ -350,7 +354,7 @@ call(SvcName, App, Message) -> :: capability() | {application, [application_opt()]} | {restrict_connections, restriction()} - | {sequence, sequence() | evaluable()} + | {sequence, sequence() | eval()} | {share_peers, remotes()} | {decode_format, decode_format()} | {traffic_counters, boolean()} @@ -392,10 +396,10 @@ call(SvcName, App, Message) -> | {pool_size, pos_integer()} | {applications, [app_alias()]} | {capabilities, [capability()]} - | {capabilities_cb, evaluable()} + | {capabilities_cb, eval()} | {capx_timeout, 'Unsigned32'()} | {strict_capx, boolean()} - | {disconnect_cb, evaluable()} + | {disconnect_cb, eval()} | {dpr_timeout, 'Unsigned32'()} | {dpa_timeout, 'Unsigned32'()} | {length_errors, exit | handle | discard} diff --git a/lib/diameter/src/base/diameter_callback.erl b/lib/diameter/src/base/diameter_callback.erl index f9cdc66c70..d04a416bef 100644 --- a/lib/diameter/src/base/diameter_callback.erl +++ b/lib/diameter/src/base/diameter_callback.erl @@ -26,16 +26,16 @@ %% as the Diameter application callback in question. The record has %% one field for each callback function as well as 'default' and %% 'extra' fields. A function-specific field can be set to a -%% diameter:evaluable() in order to redirect the callback +%% diameter:eval() in order to redirect the callback %% corresponding to that field, or to 'false' to request the default %% callback implemented in this module. If neither of these fields are %% set then the 'default' field determines the form of the callback: a %% module name results in the usual callback as if the module had been -%% configured directly as the callback module, a diameter_evaluable() +%% configured directly as the callback module, a diameter_eval() %% in a callback applied to the atom-valued callback name and argument %% list. For all callbacks not to this module, the 'extra' field is a %% list of additional arguments, following arguments supplied by -%% diameter but preceding those of the diameter:evaluable() being +%% diameter but preceding those of the diameter:eval() being %% applied. %% %% For example, the following config to diameter:start_service/2, in diff --git a/lib/diameter/src/transport/diameter_sctp.erl b/lib/diameter/src/transport/diameter_sctp.erl index 6a9f1f940b..4f56475529 100644 --- a/lib/diameter/src/transport/diameter_sctp.erl +++ b/lib/diameter/src/transport/diameter_sctp.erl @@ -79,7 +79,7 @@ -type option() :: {sender, boolean()} | sender | {packet, boolean() | raw} - | {message_cb, false | diameter:evaluable()}. + | {message_cb, false | diameter:eval()}. -type uint() :: non_neg_integer(). @@ -104,7 +104,7 @@ os = 0 :: uint(), %% next output stream packet = true :: boolean() %% legacy transport_data? | raw, - message_cb = false :: false | diameter:evaluable(), + message_cb = false :: false | diameter:eval(), send = false :: pid() | boolean()}). %% sending process %% Monitor process state. @@ -120,7 +120,7 @@ socket :: gen_sctp:sctp_socket(), service :: pid(), %% service process pending = {0, queue:new()}, - opts :: [[match()] | boolean() | diameter:evaluable()]}). + opts :: [[match()] | boolean() | diameter:eval()]}). %% Field pending implements two queues: the first of transport-to-be %% processes to which an association has been assigned but for which %% diameter hasn't yet spawned a transport process, a short-lived diff --git a/lib/diameter/src/transport/diameter_tcp.erl b/lib/diameter/src/transport/diameter_tcp.erl index 6252dbddfb..ac55d722fa 100644 --- a/lib/diameter/src/transport/diameter_tcp.erl +++ b/lib/diameter/src/transport/diameter_tcp.erl @@ -110,7 +110,7 @@ -type option() :: {port, non_neg_integer()} | {sender, boolean()} | sender - | {message_cb, false | diameter:evaluable()} + | {message_cb, false | diameter:eval()} | {fragment_timer, 0..16#FFFFFFFF}. %% Accepting/connecting transport process state. @@ -125,7 +125,7 @@ timeout :: infinity | 0..16#FFFFFFFF, %% fragment timeout tref = false :: false | reference(), %% fragment timer reference flush = false :: boolean(), %% flush fragment at timeout? - message_cb :: false | diameter:evaluable(), + message_cb :: false | diameter:eval(), send :: pid() | false}). %% sending process %% The usual transport using gen_tcp can be replaced by anything -- cgit v1.2.3 From 5f3becaddef9b18c81a1ec8bd7bf955384c1a225 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Wed, 23 Aug 2017 12:21:14 +0200 Subject: Let a service configure default transport options Only a default spawn_opt has been possible to configure, but there's no reason why most others should need to be configured per transport. Those options that still only make sense on a transport are transport_module/config (because of the semantics of multiple values), applications/capabilities (since these override service options), and private (since it's only to allow user-specific options in a backwards compatible way). --- lib/diameter/doc/src/diameter.xml | 23 +-- lib/diameter/src/base/diameter.erl | 31 +-- lib/diameter/src/base/diameter_config.erl | 297 +++++++++++++--------------- lib/diameter/src/base/diameter_service.erl | 64 ++++-- lib/diameter/src/base/diameter_watchdog.erl | 35 ++-- 5 files changed, 233 insertions(+), 217 deletions(-) (limited to 'lib/diameter') diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml index 6a4a7b9f7c..8d39b9efce 100644 --- a/lib/diameter/doc/src/diameter.xml +++ b/lib/diameter/doc/src/diameter.xml @@ -943,18 +943,6 @@ of a single Diameter node across multiple Erlang nodes.

-{spawn_opt, [term()]} - -

-Options list passed to &spawn_opt; when spawning a process for an -incoming Diameter request, unless the transport in question -specifies another value. -Options monitor and link are ignored.

- -

-Defaults to the empty list.

- - {strict_arities, boolean() | encode @@ -1108,6 +1096,15 @@ each node from which requests are sent.

+&transport_opt; + +

+Any transport option except applications or +capabilities. +Used as defaults for transport configuration, values passed to +&add_transport; overriding values configured on the service.

+ + @@ -1406,7 +1403,7 @@ incoming Diameter request. Options monitor and link are ignored.

-Defaults to the list configured on the service if not specified.

+Defaults to the empty list.

diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index ad6aaab440..0f919f6c25 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -348,6 +348,22 @@ call(SvcName, App, Message) -> | encode | decode. +%% Options common to both start_service/2 and add_transport/2. + +-type common_opt() + :: {pool_size, pos_integer()} + | {capabilities_cb, eval()} + | {capx_timeout, 'Unsigned32'()} + | {strict_capx, boolean()} + | {disconnect_cb, eval()} + | {dpr_timeout, 'Unsigned32'()} + | {dpa_timeout, 'Unsigned32'()} + | {length_errors, exit | handle | discard} + | {connect_timer, 'Unsigned32'()} + | {watchdog_timer, 'Unsigned32'() | {module(), atom(), list()}} + | {watchdog_config, [{okay|suspect, non_neg_integer()}]} + | {spawn_opt, list()}. + %% Options passed to start_service/2 -type service_opt() @@ -363,7 +379,7 @@ call(SvcName, App, Message) -> | {strict_mbit, boolean()} | {incoming_maxlen, message_length()} | {use_shared_peers, remotes()} - | {spawn_opt, list()}. + | common_opt(). -type application_opt() :: {alias, app_alias()} @@ -393,20 +409,9 @@ call(SvcName, App, Message) -> :: {transport_module, atom()} | {transport_config, any()} | {transport_config, any(), 'Unsigned32'() | infinity} - | {pool_size, pos_integer()} | {applications, [app_alias()]} | {capabilities, [capability()]} - | {capabilities_cb, eval()} - | {capx_timeout, 'Unsigned32'()} - | {strict_capx, boolean()} - | {disconnect_cb, eval()} - | {dpr_timeout, 'Unsigned32'()} - | {dpa_timeout, 'Unsigned32'()} - | {length_errors, exit | handle | discard} - | {connect_timer, 'Unsigned32'()} - | {watchdog_timer, 'Unsigned32'() | {module(), atom(), list()}} - | {watchdog_config, [{okay|suspect, non_neg_integer()}]} - | {spawn_opt, list()} + | common_opt() | {private, any()}. %% Predicate passed to remove_transport/2 diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index d54dbd9976..a4496f9725 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -102,9 +102,6 @@ -record(monitor, {mref = make_ref() :: reference(), service}). %% name -%% The default sequence mask. --define(NOMASK, {0,32}). - %% Time to lay low before restarting a dead service. -define(RESTART_SLEEP, 2000). @@ -560,89 +557,184 @@ add(SvcName, Type, Opts0) -> end. transport_opts(Opts) -> - lists:map(fun topt/1, Opts). + [setopt(transport, T) || T <- Opts]. + +%% setopt/2 -topt(T) -> - case opt(T) of +setopt(K, T) -> + case opt(K, T) of {value, X} -> X; true -> T; false -> - ?THROW({invalid, T}) + ?THROW({invalid, T}); + {error, Reason} -> + ?THROW({invalid, T, Reason}) end. -opt({transport_module, M}) -> +%% opt/2 + +opt(service, {incoming_maxlen, N}) + when 0 =< N, N < 1 bsl 24 -> + true; + +opt(service, {K, B}) + when K == strict_mbit; + K == string_decode; + K == traffic_counters -> + is_boolean(B); + +opt(service, {K, false}) + when K == share_peers; + K == use_shared_peers; + K == monitor; + K == restrict_connections; + K == strict_arities; + K == decode_format -> + true; + +opt(service, {K, true}) + when K == share_peers; + K == use_shared_peers; + K == strict_arities -> + true; + +opt(service, {decode_format, T}) + when T == record; + T == list; + T == map; + T == record_from_map -> + true; + +opt(service, {strict_arities, T}) + when T == encode; + T == decode -> + true; + +opt(service, {restrict_connections, T}) + when T == node; + T == nodes -> + true; + +opt(service, {K, T}) + when (K == share_peers + orelse K == use_shared_peers + orelse K == restrict_connections), ([] == T + orelse is_atom(hd(T))) -> + true; + +opt(service, {monitor, P}) -> + is_pid(P); + +opt(service, {K, F}) + when K == restrict_connections; + K == share_peers; + K == use_shared_peers -> + try diameter_lib:eval(F) of %% but no guarantee that it won't fail later + Nodes -> + is_list(Nodes) orelse {error, Nodes} + catch + E:R -> + {error, {E, R, ?STACK}} + end; + +opt(service, {sequence, {H,N}}) -> + 0 =< N andalso N =< 32 + andalso is_integer(H) + andalso 0 =< H + andalso 0 == H bsr (32-N); + +opt(service = S, {sequence = K, F}) -> + try diameter_lib:eval(F) of + {_,_} = T -> + KT = {K,T}, + opt(S, KT) andalso {value, KT}; + V -> + {error, V} + catch + E:R -> + {error, {E, R, ?STACK}} + end; + +opt(transport, {transport_module, M}) -> is_atom(M); -opt({transport_config, _, Tmo}) -> +opt(transport, {transport_config, _, Tmo}) -> ?IS_UINT32(Tmo) orelse Tmo == infinity; -opt({applications, As}) -> +opt(transport, {applications, As}) -> is_list(As); -opt({capabilities, Os}) -> - is_list(Os) andalso ok == encode_CER(Os); +opt(transport, {capabilities, Os}) -> + is_list(Os) andalso case encode_CER(Os) of + ok -> true; + No -> {error, No} + end; -opt({K, Tmo}) +opt(_, {K, Tmo}) when K == capx_timeout; K == dpr_timeout; K == dpa_timeout -> ?IS_UINT32(Tmo); -opt({capx_strictness, B}) -> +opt(_, {capx_strictness, B}) -> is_boolean(B) andalso {value, {strict_capx, B}}; -opt({strict_capx, B}) -> +opt(_, {strict_capx, B}) -> is_boolean(B); -opt({length_errors, T}) -> +opt(_, {length_errors, T}) -> lists:member(T, [exit, handle, discard]); -opt({K, Tmo}) - when K == reconnect_timer; %% deprecated - K == connect_timer -> +opt(transport, {reconnect_timer, Tmo}) -> %% deprecated + ?IS_UINT32(Tmo) andalso {value, {connect_timer, Tmo}}; +opt(_, {connect_timer, Tmo}) -> ?IS_UINT32(Tmo); -opt({watchdog_timer, {M,F,A}}) +opt(_, {watchdog_timer, {M,F,A}}) when is_atom(M), is_atom(F), is_list(A) -> true; -opt({watchdog_timer, Tmo}) -> +opt(_, {watchdog_timer, Tmo}) -> ?IS_UINT32(Tmo); -opt({watchdog_config, L}) -> - is_list(L) andalso lists:all(fun wdopt/1, L); +opt(_, {watchdog_config, L}) -> + is_list(L) andalso lists:all(fun wd/1, L); -opt({spawn_opt, {M,F,A}}) +opt(_, {spawn_opt, {M,F,A}}) when is_atom(M), is_atom(F), is_list(A) -> true; -opt({spawn_opt = K, Opts}) -> +opt(_, {spawn_opt = K, Opts}) -> if is_list(Opts) -> {value, {K, spawn_opts(Opts)}}; true -> false end; -opt({pool_size, N}) -> +opt(_, {pool_size, N}) -> is_integer(N) andalso 0 < N; -%% Options that we can't validate. -opt({K, _}) +%% Options we can't validate. +opt(_, {K, _}) + when K == disconnect_cb; + K == capabilities_cb -> + true; +opt(transport, {K, _}) when K == transport_config; - K == capabilities_cb; - K == disconnect_cb; K == private -> true; -%% Anything else, which is ignored by us. This makes options sensitive -%% to spelling mistakes but arbitrary options are passed by some users -%% as a way to identify transports. (That is, can't just do away with -%% it.) -opt(_) -> - true. +%% Anything else, which is ignored in transport config. This makes +%% options sensitive to spelling mistakes, but arbitrary options are +%% passed by some users as a way to identify transports so can't just +%% do away with it. +opt(K, _) -> + K == transport. + +%% wd/1 -wdopt({K,N}) -> +wd({K,N}) -> (K == okay orelse K == suspect) andalso is_integer(N) andalso 0 =< N; -wdopt(_) -> +wd(_) -> false. %% start_transport/2 @@ -707,19 +799,7 @@ make_config(SvcName, Opts) -> ok = encode_CER(CapOpts), - SvcOpts = make_opts((Opts -- AppOpts) -- CapOpts, - [{false, share_peers}, - {false, use_shared_peers}, - {false, monitor}, - {?NOMASK, sequence}, - {nodes, restrict_connections}, - {16#FFFFFF, incoming_maxlen}, - {true, strict_arities}, - {true, strict_mbit}, - {record, decode_format}, - {true, traffic_counters}, - {true, string_decode}, - {[], spawn_opt}]), + SvcOpts = service_opts((Opts -- AppOpts) -- CapOpts), D = proplists:get_value(string_decode, SvcOpts, true), @@ -733,115 +813,22 @@ binary_caps(Caps, true) -> binary_caps(Caps, false) -> diameter_capx:binary_caps(Caps). -%% make_opts/2 - -make_opts(Opts, Defs) -> - Known = [{K, get_opt(K, Opts, D)} || {D,K} <- Defs], - Unknown = Opts -- Known, +%% service_opts/1 - [] == Unknown orelse ?THROW({invalid, hd(Unknown)}), - - [{K, opt(K,V)} || {K,V} <- Known]. - -opt(incoming_maxlen, N) - when 0 =< N, N < 1 bsl 24 -> - N; - -opt(spawn_opt, {M,F,A} = T) - when is_atom(M), is_atom(F), is_list(A) -> - T; - -opt(spawn_opt, L) - when is_list(L) -> - spawn_opts(L); - -opt(K, false = B) - when K == share_peers; - K == use_shared_peers; - K == monitor; - K == restrict_connections; - K == strict_arities; - K == strict_mbit; - K == decode_format; - K == traffic_counters; - K == string_decode -> - B; - -opt(K, true = B) - when K == share_peers; - K == use_shared_peers; - K == strict_arities; - K == strict_mbit; - K == traffic_counters; - K == string_decode -> - B; - -opt(decode_format, T) - when T == record; - T == list; - T == map; - T == record_from_map -> - T; - -opt(strict_arities, T) - when T == encode; - T == decode -> - T; - -opt(restrict_connections, T) - when T == node; - T == nodes -> - T; - -opt(K, T) - when (K == share_peers - orelse K == use_shared_peers - orelse K == restrict_connections), ([] == T - orelse is_atom(hd(T))) -> - T; - -opt(monitor, P) - when is_pid(P) -> - P; - -opt(K, F) - when K == restrict_connections; - K == share_peers; - K == use_shared_peers -> - try diameter_lib:eval(F) of %% but no guarantee that it won't fail later - Nodes when is_list(Nodes) -> - F; - V -> - ?THROW({value, {K,V}}) - catch - E:R -> - ?THROW({value, {K, E, R, ?STACK}}) - end; - -opt(sequence, {_,_} = T) -> - sequence(T); - -opt(sequence = K, F) -> - try diameter_lib:eval(F) of - T -> sequence(T) - catch - E:R -> - ?THROW({value, {K, E, R, ?STACK}}) - end; - -opt(K, _) -> - ?THROW({value, K}). +service_opts(Opts) -> + Res = [setopt(service, T) || T <- Opts], + Keys = sets:to_list(sets:from_list([K || {K,_} <- Res])), %% unique + Dups = lists:foldl(fun(K,A) -> lists:keydelete(K, 1, A) end, Res, Keys), + [] == Dups orelse ?THROW({duplicate, Dups}), + Res. +%% Reject duplicates on a service, but not on a transport. There's no +%% particular reason for the inconsistency, but the historic behaviour +%% ignores all but the first of a transport_opt(), and there's no real +%% reason to change it. spawn_opts(L) -> [T || T <- L, T /= link, T /= monitor]. -sequence({H,N} = T) - when 0 =< N, N =< 32, 0 =< H, 0 == H bsr (32-N) -> - T; - -sequence(_) -> - ?THROW({value, sequence}). - make_caps(Caps, Opts) -> case diameter_capx:make_caps(Caps, Opts) of {ok, T} -> diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl index 07f1fd3a4a..c7b0e706a5 100644 --- a/lib/diameter/src/base/diameter_service.erl +++ b/lib/diameter/src/base/diameter_service.erl @@ -117,6 +117,18 @@ decode_format := diameter:decode_format(), traffic_counters := boolean(), string_decode := boolean(), + capabilities_cb => diameter:evaluable(), + pool_size => pos_integer(), + capx_timeout => diameter:'Unsigned32'(), + strict_capx => boolean(), + disconnect_cb => diameter:evaluable(), + dpr_timeout => diameter:'Unsigned32'(), + dpa_timeout => diameter:'Unsigned32'(), + length_errors => exit | handle | discard, + connect_timer => diameter:'Unsigned32'(), + watchdog_timer => diameter:'Unsigned32'() + | {module(), atom(), list()}, + watchdog_config => [{okay|suspect, non_neg_integer()}], spawn_opt := list() | {module(), atom(), list()}}}). %% Record representing an RFC 3539 watchdog process implemented by @@ -682,12 +694,15 @@ i(SvcName) -> cfg_acc({SvcName, #diameter_service{applications = Apps} = Rec, Opts}, {false, Acc}) -> lists:foreach(fun init_mod/1, Apps), + #{monitor := M} + = SvcOpts + = service_opts(Opts), S = #state{service_name = SvcName, service = Rec#diameter_service{pid = self()}, local = init_peers(), remote = init_peers(), - monitor = mref(get_value(monitor, Opts)), - options = service_options(lists:keydelete(monitor, 1, Opts))}, + monitor = mref(M), + options = maps:remove(monitor, SvcOpts)}, {S, Acc}; cfg_acc({_Ref, Type, _Opts} = T, {S, Acc}) @@ -702,8 +717,23 @@ init_peers() -> %% Alias, %% TPid} -service_options(Opts) -> - maps:from_list(lists:delete({strict_arities, true}, Opts)). +service_opts(Opts) -> + T = {strict_arities, true}, + maps:merge(maps:from_list([{monitor, false} | def_opts() -- [T]]), + maps:from_list(Opts -- [T])). + +def_opts() -> %% defaults on the service map + [{share_peers, false}, + {use_shared_peers, false}, + {sequence, {0,32}}, + {restrict_connections, nodes}, + {incoming_maxlen, 16#FFFFFF}, + {strict_arities, true}, + {strict_mbit, true}, + {decode_format, record}, + {traffic_counters, true}, + {string_decode, true}, + {spawn_opt, []}]. mref(false = No) -> No; @@ -721,10 +751,6 @@ init_mod(#diameter_app{alias = Alias, start_fsm({Ref, Type, Opts}, S) -> start(Ref, {Type, Opts}, S). -get_value(Key, Vs) -> - {_, V} = lists:keyfind(Key, 1, Vs), - V. - notify(Share, SvcName, T) -> Nodes = remotes(Share), [] /= Nodes andalso diameter_peer:notify(Nodes, SvcName, T). @@ -809,7 +835,7 @@ start(Ref, Type, Opts, State) -> start(Ref, Type, Opts, N, #state{watchdogT = WatchdogT, local = {PeerT, _, _}, options = #{string_decode := SD} - = SvcOpts0, + = SvcOpts, service_name = SvcName, service = Svc0}) when Type == connect; @@ -818,12 +844,12 @@ start(Ref, Type, Opts, N, #state{watchdogT = WatchdogT, = Svc1 = merge_service(Opts, Svc0), Svc = binary_caps(Svc1, SD), - SvcOpts = merge_options(Opts, SvcOpts0), - RecvData = diameter_traffic:make_recvdata([SvcName, PeerT, Apps, SvcOpts]), - T = {Opts, SvcOpts, RecvData, Svc}, + {SOpts, TOpts} = merge_opts(SvcOpts, Opts), + RecvData = diameter_traffic:make_recvdata([SvcName, PeerT, Apps, SOpts]), + T = {TOpts, SOpts, RecvData, Svc}, Rec = #watchdog{type = Type, ref = Ref, - options = Opts}, + options = TOpts}, diameter_lib:fold_n(fun(_,A) -> [wd(Type, Ref, T, WatchdogT, Rec) | A] @@ -831,10 +857,14 @@ start(Ref, Type, Opts, N, #state{watchdogT = WatchdogT, [], N). -merge_options(Opts, SvcOpts) -> - Keys = maps:keys(SvcOpts), - Map = maps:from_list([KV || {K,_} = KV <- Opts, lists:member(K, Keys)]), - maps:merge(SvcOpts, Map). +merge_opts(SvcOpts, Opts) -> + Keys = [K || {K,_} <- def_opts()], + SO = [T || {K,_} = T <- Opts, lists:member(K, Keys)], + TO = Opts -- SO, + {maps:merge(maps:with(Keys, SvcOpts), maps:from_list(SO)), + TO ++ [T || {K,_} = T <- maps:to_list(SvcOpts), + not lists:member(K, Keys), + not lists:keymember(K, 1, Opts)]}. binary_caps(Svc, true) -> Svc; diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl index b2172356ee..bb671e9860 100644 --- a/lib/diameter/src/base/diameter_watchdog.erl +++ b/lib/diameter/src/base/diameter_watchdog.erl @@ -76,10 +76,8 @@ string_decode := false, strict_arities => diameter:strict_arities(), strict_mbit := boolean(), - failed_avp := false, rfc := 3588 | 6733, - ordered_encode := false, - incoming_maxlen := diameter:message_length()}, + ordered_encode := false}, shutdown = false :: boolean()}). %% --------------------------------------------------------------------------- @@ -137,15 +135,6 @@ i({Ack, T, Pid, {Opts, putr(restart, {T, Opts, Svc, SvcOpts}), %% save seeing it in trace putr(dwr, dwr(Caps)), %% Nodes = restrict_nodes(Restrict), - CodecKeys = [decode_format, - string_decode, - strict_arities, - strict_mbit, - incoming_maxlen, - spawn_opt, - rfc, - ordered_encode], - #watchdog{parent = Pid, transport = start(T, Opts, SvcOpts, Nodes, Dict0, Svc), tw = proplists:get_value(watchdog_timer, @@ -153,13 +142,21 @@ i({Ack, T, Pid, {Opts, ?DEFAULT_TW_INIT), receive_data = RecvData, dictionary = Dict0, - config = - maps:without([traffic_counters | CodecKeys], - config(SvcOpts#{restrict => restrict(Nodes), - suspect => 1, - okay => 3}, - Opts)), - codec = maps:with(CodecKeys -- [strict_arities], + config = maps:with([sequence, + restrict_connections, + restrict, + suspect, + okay], + config(SvcOpts#{restrict => restrict(Nodes), + suspect => 1, + okay => 3}, + Opts)), + codec = maps:with([decode_format, + strict_arities, + strict_mbit, + string_decode, + rfc, + ordered_encode], SvcOpts#{decode_format := false, string_decode := false, ordered_encode => false})}. -- cgit v1.2.3 From b0582c6963f6dc203f05ed810c9446cf3fa0f0ae Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 24 Aug 2017 13:21:28 +0200 Subject: Let strict_mbit and incoming_maxlen be configured per transport Since these can make sense per peer. The remaining service-only options either belong there or make little sense being configured per transport. --- lib/diameter/doc/src/diameter.xml | 112 +++++++++++++++--------------- lib/diameter/src/base/diameter.erl | 4 +- lib/diameter/src/base/diameter_config.erl | 12 ++-- 3 files changed, 64 insertions(+), 64 deletions(-) (limited to 'lib/diameter') diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml index 8d39b9efce..30a26ed845 100644 --- a/lib/diameter/doc/src/diameter.xml +++ b/lib/diameter/doc/src/diameter.xml @@ -826,19 +826,6 @@ field of diameter_packet records independently of - -{incoming_maxlen, 0..16777215} - -

-Bound on the expected size of incoming Diameter messages. -Messages larger than the specified number of bytes are discarded.

- -

-Defaults to 16777215, the maximum value of the 24-bit Message -Length field in a Diameter Header.

- - - {restrict_connections, false | node | nodes @@ -974,49 +961,6 @@ of arity 1 as bare values, not wrapped in a list.

- -{strict_mbit, boolean()} - -

-Whether or not to regard an AVP setting the M-bit as erroneous when -the command grammar in question does not explicitly allow the AVP. -If true then such AVPs are regarded as 5001 errors, -DIAMETER_AVP_UNSUPPORTED. -If false then the M-bit is ignored and policing -it becomes the receiver's responsibility.

- -

-Defaults to true.

- - -

-RFC 6733 is unclear about the semantics of the M-bit. -One the one hand, the CCF specification in section 3.2 documents AVP -in a command grammar as meaning any arbitrary AVP; on the -other hand, 1.3.4 states that AVPs setting the M-bit cannot be added -to an existing command: the modified command must instead be -placed in a new Diameter application.

-

-The reason for the latter is presumably interoperability: -allowing arbitrary AVPs setting the M-bit in a command makes its -interpretation implementation-dependent, since there's no -guarantee that all implementations will understand the same set of -arbitrary AVPs in the context of a given command. -However, interpreting AVP in a command grammar as any -AVP, regardless of M-bit, renders 1.3.4 meaningless, since the receiver -can simply ignore any AVP it thinks isn't relevant, regardless of the -sender's intent.

-

-Beware of confusing mandatory in the sense of the M-bit with mandatory -in the sense of the command grammar. -The former is a semantic requirement: that the receiver understand the -semantics of the AVP in the context in question. -The latter is a syntactic requirement: whether or not the AVP must -occur in the message in question.

- - - - {string_decode, boolean()} @@ -1345,6 +1289,19 @@ connection.

Defaults to 5000.

 + +{incoming_maxlen, 0..16777215} + +

+Bound on the expected size of incoming Diameter messages. +Messages larger than the specified number of bytes are discarded.

+ +

+Defaults to 16777215, the maximum value of the 24-bit Message +Length field in a Diameter Header.

+ + + {length_errors, exit|handle|discard} @@ -1420,6 +1377,49 @@ Changing this results in non-standard behaviour, but can be useful in case peers are known to be behave badly.

 + +{strict_mbit, boolean()} + +

+Whether or not to regard an AVP setting the M-bit as erroneous when +the command grammar in question does not explicitly allow the AVP. +If true then such AVPs are regarded as 5001 errors, +DIAMETER_AVP_UNSUPPORTED. +If false then the M-bit is ignored and policing +it becomes the receiver's responsibility.

+ +

+Defaults to true.

+ + +

+RFC 6733 is unclear about the semantics of the M-bit. +One the one hand, the CCF specification in section 3.2 documents AVP +in a command grammar as meaning any arbitrary AVP; on the +other hand, 1.3.4 states that AVPs setting the M-bit cannot be added +to an existing command: the modified command must instead be +placed in a new Diameter application.

+

+The reason for the latter is presumably interoperability: +allowing arbitrary AVPs setting the M-bit in a command makes its +interpretation implementation-dependent, since there's no +guarantee that all implementations will understand the same set of +arbitrary AVPs in the context of a given command. +However, interpreting AVP in a command grammar as any +AVP, regardless of M-bit, renders 1.3.4 meaningless, since the receiver +can simply ignore any AVP it thinks isn't relevant, regardless of the +sender's intent.

+

+Beware of confusing mandatory in the sense of the M-bit with mandatory +in the sense of the command grammar. +The former is a semantic requirement: that the receiver understand the +semantics of the AVP in the context in question. +The latter is a syntactic requirement: whether or not the AVP must +occur in the message in question.

+ + + + {transport_config, term()} {transport_config, term(), &dict_Unsigned32; | infinity} diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index 0f919f6c25..3b41feac0d 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -355,9 +355,11 @@ call(SvcName, App, Message) -> | {capabilities_cb, eval()} | {capx_timeout, 'Unsigned32'()} | {strict_capx, boolean()} + | {strict_mbit, boolean()} | {disconnect_cb, eval()} | {dpr_timeout, 'Unsigned32'()} | {dpa_timeout, 'Unsigned32'()} + | {incoming_maxlen, message_length()} | {length_errors, exit | handle | discard} | {connect_timer, 'Unsigned32'()} | {watchdog_timer, 'Unsigned32'() | {module(), atom(), list()}} @@ -376,8 +378,6 @@ call(SvcName, App, Message) -> | {traffic_counters, boolean()} | {string_decode, boolean()} | {strict_arities, true | strict_arities()} - | {strict_mbit, boolean()} - | {incoming_maxlen, message_length()} | {use_shared_peers, remotes()} | common_opt(). diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index a4496f9725..4721be1ca0 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -575,13 +575,11 @@ setopt(K, T) -> %% opt/2 -opt(service, {incoming_maxlen, N}) - when 0 =< N, N < 1 bsl 24 -> - true; +opt(_, {incoming_maxlen, N}) -> + is_integer(N) andalso 0 =< N andalso N < 1 bsl 24; opt(service, {K, B}) - when K == strict_mbit; - K == string_decode; + when K == string_decode; K == traffic_counters -> is_boolean(B); @@ -680,7 +678,9 @@ opt(_, {K, Tmo}) opt(_, {capx_strictness, B}) -> is_boolean(B) andalso {value, {strict_capx, B}}; -opt(_, {strict_capx, B}) -> +opt(_, {K, B}) + when K == strict_capx; + K == strict_mbit -> is_boolean(B); opt(_, {length_errors, T}) -> -- cgit v1.2.3 From 3ec3cc239999cb59d1260b6c7790eb112d9da47c Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 31 Aug 2017 16:09:11 +0200 Subject: Fix minor error-handling blunder Leading to this admonition from dialyzer: diameter_config.erl:670: The variable No can never match since previous clauses completely covered the type 'ok' The throw was caught, but resulted in an error return without the intended information. --- lib/diameter/src/base/diameter_config.erl | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'lib/diameter') diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index 4721be1ca0..6fc4277ac8 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -665,9 +665,8 @@ opt(transport, {applications, As}) -> is_list(As); opt(transport, {capabilities, Os}) -> - is_list(Os) andalso case encode_CER(Os) of - ok -> true; - No -> {error, No} + is_list(Os) andalso try ok = encode_CER(Os), true + catch ?FAILURE(No) -> {error, No} end; opt(_, {K, Tmo}) -- cgit v1.2.3 From ae6966491183c62f6513b175aaede785871240cf Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Fri, 1 Sep 2017 16:58:50 +0200 Subject: Fix strict_arities blunder Remove value from the merged map, not from the maps being merged. Bundled in commit 5f3becad. --- lib/diameter/src/base/diameter_service.erl | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'lib/diameter') diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl index c7b0e706a5..1e104f9e65 100644 --- a/lib/diameter/src/base/diameter_service.erl +++ b/lib/diameter/src/base/diameter_service.erl @@ -718,9 +718,13 @@ init_peers() -> %% TPid} service_opts(Opts) -> - T = {strict_arities, true}, - maps:merge(maps:from_list([{monitor, false} | def_opts() -- [T]]), - maps:from_list(Opts -- [T])). + remove([{strict_arities, true}], + maps:merge(maps:from_list([{monitor, false} | def_opts()]), + maps:from_list(Opts))). + +remove(List, Map) -> + maps:filter(fun(K,V) -> not lists:member({K,V}, List) end, + Map). def_opts() -> %% defaults on the service map [{share_peers, false}, -- cgit v1.2.3