aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnders Svensson <anders@erlang.org>2015-03-23 07:57:26 +0100
committerAnders Svensson <anders@erlang.org>2015-03-23 08:13:22 +0100
commit9321b4b3aa332f140268ef8f3251f6314a4984fe (patch)
tree2d601c94df415788dadf8c5b71c3a1d9ebdf6097
parentaf87b1c3d4897840d8247589a88d3611106ecedc (diff)
downloadotp-9321b4b3aa332f140268ef8f3251f6314a4984fe.tar.gz
otp-9321b4b3aa332f140268ef8f3251f6314a4984fe.tar.bz2
otp-9321b4b3aa332f140268ef8f3251f6314a4984fe.zip
Fix ordering of AVPs in relayed messages
6.1.9 of RFC 6733 states this: A relay or proxy agent MUST append a Route-Record AVP to all requests forwarded. The AVP was inserted as the head of the AVP list, not appended, since the entire AVP list was reversed relative to the received order. Thanks to Andrzej TrawiƄski.
-rw-r--r--lib/diameter/src/base/diameter_codec.erl40
-rw-r--r--lib/diameter/src/base/diameter_traffic.erl2
2 files changed, 33 insertions, 9 deletions
diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl
index a2b04bfd63..07ad5f97d7 100644
--- a/lib/diameter/src/base/diameter_codec.erl
+++ b/lib/diameter/src/base/diameter_codec.erl
@@ -90,7 +90,7 @@ encode(Mod, Msg) ->
msg = Msg}).
e(_, #diameter_packet{msg = [#diameter_header{} = Hdr | As]} = Pkt) ->
- try encode_avps(As) of
+ try encode_avps(reorder(As)) of
Avps ->
Length = size(Avps) + 20,
@@ -183,26 +183,50 @@ values(Avps) ->
%% Message as a list of #diameter_avp{} ...
encode_avps(_, _, [#diameter_avp{} | _] = Avps) ->
- encode_avps(reorder(Avps, [], Avps));
+ encode_avps(reorder(Avps));
%% ... or as a tuple list or record.
encode_avps(Mod, MsgName, Values) ->
Mod:encode_avps(MsgName, Values).
%% reorder/1
+%%
+%% Reorder AVPs for the relay case using the index field of
+%% diameter_avp records. Decode populates this field in collect_avps
+%% and presents AVPs in reverse order. A relay then sends the reversed
+%% list with a Route-Record AVP prepended. The goal here is just to do
+%% lists:reverse/1 in Grouped AVPs and the outer list, but only in the
+%% case there are indexed AVPs at all, so as not to reverse lists that
+%% have been explicilty sent (unindexed, in the desired order) as a
+%% diameter_avp list. The effect is the same as lists:keysort/2, but
+%% only on the cases we expect, not a general sort.
+
+reorder(Avps) ->
+ case reorder(Avps, []) of
+ false ->
+ Avps;
+ Sorted ->
+ Sorted
+ end.
-reorder([#diameter_avp{index = 0} | _] = Avps, Acc, _) ->
+%% reorder/3
+
+%% In case someone has reversed the list already. (Not likely.)
+reorder([#diameter_avp{index = 0} | _] = Avps, Acc) ->
Avps ++ Acc;
-reorder([#diameter_avp{index = N} = A | Avps], Acc, _)
+%% Assume indexed AVPs are in reverse order.
+reorder([#diameter_avp{index = N} = A | Avps], Acc)
when is_integer(N) ->
lists:reverse(Avps, [A | Acc]);
-reorder([H | T], Acc, Avps) ->
- reorder(T, [H | Acc], Avps);
+%% An unindexed AVP.
+reorder([H | T], Acc) ->
+ reorder(T, [H | Acc]);
-reorder([], Acc, _) ->
- Acc.
+%% No indexed members.
+reorder([], _) ->
+ false.
%% encode_avps/1
diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl
index 3b62afca47..3717e43e4a 100644
--- a/lib/diameter/src/base/diameter_traffic.erl
+++ b/lib/diameter/src/base/diameter_traffic.erl
@@ -592,7 +592,7 @@ resend(false,
Route = #diameter_avp{data = {Dict0, 'Route-Record', OH}},
Seq = diameter_session:sequence(Mask),
Hdr = Hdr0#diameter_header{hop_by_hop_id = Seq},
- Msg = [Hdr, Route | Avps],
+ Msg = [Hdr, Route | Avps], %% reordered at encode
resend(send_request(SvcName, App, Msg, Opts), Caps, Dict0, Pkt).
%% The incoming request is relayed with the addition of a
%% Route-Record. Note the requirement on the return from call/4 below,