Merge branch 'maint'

3 files changed, 225 insertions, 99 deletions

diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl
index e446a0209c..6c0e73de36 100644
--- a/lib/diameter/src/base/diameter_codec.erl
+++ b/lib/diameter/src/base/diameter_codec.erl
@@ -38,6 +38,10 @@
 
 -define(MASK(N,I), ((I) band (1 bsl (N)))).
 
+-type u32() :: 0..16#FFFFFFFF.
+-type u24() :: 0..16#FFFFFF.
+-type u1()  :: 0..1.
+
 %%     0                   1                   2                   3
 %%     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
 %%    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -55,9 +59,13 @@
 %%    +-+-+-+-+-+-+-+-+-+-+-+-+-
 
 %%% ---------------------------------------------------------------------------
-%%% # encode/[2-4]
+%%% # encode/2
 %%% ---------------------------------------------------------------------------
 
+-spec encode(module(), Msg :: term())
+   -> #diameter_packet{}
+    | no_return().
+
 encode(Mod, #diameter_packet{} = Pkt) ->
     try
         e(Mod, Pkt)
@@ -217,6 +225,9 @@ rec2msg(Mod, Rec) ->
 
 %% Unsuccessfully decoded AVPs will be placed in #diameter_packet.errors.
 
+-spec decode(module(), #diameter_packet{} | bitstring())
+   -> #diameter_packet{}.
+
 decode(Mod, Pkt) ->
     decode(Mod:id(), Mod, Pkt).
 
@@ -225,9 +236,9 @@ decode(Mod, Pkt) ->
 %% question.
 decode(?APP_ID_RELAY, _, #diameter_packet{} = Pkt) ->
     case collect_avps(Pkt) of
-        {Bs, As} ->
+        {E, As} ->
             Pkt#diameter_packet{avps = As,
-                                errors = [Bs]};
+                                errors = [E]};
         As ->
             Pkt#diameter_packet{avps = As}
     end;
@@ -251,12 +262,12 @@ decode(Id, Mod, Bin)
   when is_bitstring(Bin) ->
     decode(Id, Mod, #diameter_packet{header = decode_header(Bin), bin = Bin}).
 
-decode_avps(MsgName, Mod, Pkt, {Bs, Avps}) ->  %% invalid avp bits ...
+decode_avps(MsgName, Mod, Pkt, {E, Avps}) ->
     ?LOG(invalid, Pkt#diameter_packet.bin),
     #diameter_packet{errors = Failed}
         = P
         = decode_avps(MsgName, Mod, Pkt, Avps),
-    P#diameter_packet{errors = [Bs | Failed]};
+    P#diameter_packet{errors = [E | Failed]};
 
 decode_avps('', Mod, Pkt, Avps) ->  %% unknown message ...
     ?LOG(unknown, {Mod, Pkt#diameter_packet.header}),
@@ -275,6 +286,10 @@ decode_avps(MsgName, Mod, Pkt, Avps) ->  %% ... or not
 %%% # decode_header/1
 %%% ---------------------------------------------------------------------------
 
+-spec decode_header(bitstring())
+   -> #diameter_header{}
+    | false.
+
 decode_header(<<Version:8,
                 MsgLength:24,
                 CmdFlags:1/binary,
@@ -324,6 +339,13 @@ decode_header(_) ->
 %% wraparound counter. The 8-bit counter is incremented each time the
 %% system is restarted.
 
+-spec sequence_numbers(#diameter_packet{}
+                       | #diameter_header{}
+                       | binary()
+                       | Seq)
+   -> Seq
+ when Seq :: {HopByHopId :: u32(), EndToEndId :: u32()}.
+
 sequence_numbers({_,_} = T) ->
     T;
 
@@ -345,6 +367,9 @@ sequence_numbers(<<_:12/binary, H:32, E:32, _/binary>>) ->
 %%% # hop_by_hop_id/2
 %%% ---------------------------------------------------------------------------
 
+-spec hop_by_hop_id(u32(), binary())
+   -> binary().
+
 hop_by_hop_id(Id, <<H:12/binary, _:32, T/binary>>) ->
     <<H/binary, Id:32, T/binary>>.
 
@@ -352,6 +377,10 @@ hop_by_hop_id(Id, <<H:12/binary, _:32, T/binary>>) ->
 %%% # msg_name/2
 %%% ---------------------------------------------------------------------------
 
+-spec msg_name(module(), #diameter_header{})
+   -> atom()
+    | {ApplicationId :: u32(), CommandCode :: u24(), Rbit :: u1()}.
+
 msg_name(Dict0, #diameter_header{application_id = ?APP_ID_COMMON,
                                  cmd_code = C,
                                  is_request = R}) ->
@@ -367,6 +396,9 @@ msg_name(_, Hdr) ->
 %%% # msg_id/1
 %%% ---------------------------------------------------------------------------
 
+-spec msg_id(#diameter_packet{} | #diameter_header{})
+   -> {ApplicationId :: u32(), CommandCode :: u24(), Rbit :: u1()}.
+
 msg_id(#diameter_packet{msg = [#diameter_header{} = Hdr | _]}) ->
     msg_id(Hdr);
 
@@ -389,6 +421,12 @@ msg_id(<<_:32, Rbit:1, _:7, CmdCode:24, ApplId:32, _/bitstring>>) ->
 %% order in the binary. Note also that grouped avp's aren't unraveled,
 %% only those at the top level.
 
+-spec collect_avps(#diameter_packet{} | bitstring())
+   -> [Avp]
+    | {Error, [Avp]}
+ when Avp   :: #diameter_avp{},
+      Error :: {5014, #diameter_avp{}}.
+
 collect_avps(#diameter_packet{bin = Bin}) ->
     <<_:20/binary, Avps/bitstring>> = Bin,
     collect_avps(Avps);
@@ -403,8 +441,8 @@ collect_avps(Bin, N, Acc) ->
         {Rest, AVP} ->
             collect_avps(Rest, N+1, [AVP#diameter_avp{index = N} | Acc])
     catch
-        ?FAILURE(_) ->
-            {Bin, Acc}
+        ?FAILURE(Error) ->
+            {Error, Acc}
     end.
 
 %%     0                   1                   2                   3
@@ -422,42 +460,87 @@ collect_avps(Bin, N, Acc) ->
 %% split_avp/1
 
 split_avp(Bin) ->
-    8 =< size(Bin) orelse ?THROW(truncated_header),
+    {Code, V, M, P, Len, HdrLen} = split_head(Bin),
+    {Data, B} = split_data(Bin, HdrLen, Len - HdrLen),
 
-    <<Code:32, Flags:1/binary, Length:24, Rest/bitstring>>
-        = Bin,
+    {B, #diameter_avp{code = Code,
+                      vendor_id = V,
+                      is_mandatory = 1 == M,
+                      need_encryption = 1 == P,
+                      data = Data}}.
 
-    DataSize = Length - 8,        % size(Code+Flags+Length) = 8 octets
-    PadSize = (4 - (DataSize rem 4)) rem 4,
+%% split_head/1
 
-    DataSize + PadSize =< size(Rest)
-        orelse ?THROW(truncated_data),
+split_head(<<Code:32, 1:1, M:1, P:1, _:5, Len:24, V:32, _/bitstring>>) ->
+    {Code, V, M, P, Len, 12};
 
-    <<Data:DataSize/binary, _:PadSize/binary, R/bitstring>>
-        = Rest,
-    <<Vbit:1, Mbit:1, Pbit:1, _Reserved:5>>
-        = Flags,
+split_head(<<Code:32, 0:1, M:1, P:1, _:5, Len:24, _/bitstring>>) ->
+    {Code, undefined, M, P, Len, 8};
 
-    0 == Vbit orelse 4 =< size(Data)
-        orelse ?THROW(truncated_vendor_id),
+split_head(Bin) ->
+    ?THROW({5014, #diameter_avp{data = Bin}}).
+
+%% 3588:
+%%
+%%   DIAMETER_INVALID_AVP_LENGTH        5014
+%%      The request contained an AVP with an invalid length.  A Diameter
+%%      message indicating this error MUST include the offending AVPs
+%%      within a Failed-AVP AVP.
 
-    {Vid, D} = vid(Vbit, Data),
-    {R, #diameter_avp{code = Code,
-                      vendor_id = Vid,
-                      is_mandatory = 1 == Mbit,
-                      need_encryption = 1 == Pbit,
-                      data = D}}.
+%% 6733:
+%%
+%%    DIAMETER_INVALID_AVP_LENGTH 5014
+%%
+%%       The request contained an AVP with an invalid length.  A Diameter
+%%       message indicating this error MUST include the offending AVPs
+%%       within a Failed-AVP AVP.  In cases where the erroneous AVP length
+%%       value exceeds the message length or is less than the minimum AVP
+%%                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+%%       header length, it is sufficient to include the offending AVP
+%%       ^^^^^^^^^^^^^
+%%       header and a zero filled payload of the minimum required length
+%%       for the payloads data type.  If the AVP is a Grouped AVP, the
+%%       Grouped AVP header with an empty payload would be sufficient to
+%%       indicate the offending AVP.  In the case where the offending AVP
+%%       header cannot be fully decoded when the AVP length is less than
+%%       the minimum AVP header length, it is sufficient to include an
+%%       offending AVP header that is formulated by padding the incomplete
+%%       AVP header with zero up to the minimum AVP header length.
+%%
+%% The underlined clause must be in error since (1) a header less than
+%% the minimum value mean we don't know the identity of the AVP and
+%% (2) the last sentence covers this case.
 
-%% The RFC is a little misleading when stating that OctetString is
-%% padded to a 32-bit boundary while other types align naturally. All
-%% other types are already multiples of 32 bits so there's no need to
-%% distinguish between types here. Any invalid lengths will result in
-%% decode error in diameter_types.
+%% split_data/3
 
-vid(1, <<Vid:32, Data/bitstring>>) ->
-    {Vid, Data};
-vid(0, Data) ->
-    {undefined, Data}.
+split_data(Bin, HdrLen, Len)
+  when 0 =< Len ->
+    split_data(Bin, HdrLen, Len, (4 - (Len rem 4)) rem 4);
+
+split_data(_, _, _) ->
+    invalid_avp_length().
+
+%% split_data/4
+
+split_data(Bin, HdrLen, Len, Pad) ->
+    <<_:HdrLen/binary, T/bitstring>> = Bin,
+    case T of
+        <<Data:Len/binary, _:Pad/binary, Rest/bitstring>> ->
+            {Data, Rest};
+        _ ->
+            invalid_avp_length()
+    end.
+
+%% invalid_avp_length/0
+%%
+%% AVP Length doesn't mesh with payload. Induce a decode error by
+%% returning a payload that no valid Diameter type can have. This is
+%% so that a known AVP will result in 5014 error with a zero'd
+%% payload. Here we simply don't know how to construct this payload.
+%% (Yes, this solution is an afterthought.)
+
+invalid_avp_length() ->
+    {<<0:1>>, <<>>}.
 
 %%% ---------------------------------------------------------------------------
 %%% # pack_avp/1
@@ -472,20 +555,35 @@ vid(0, Data) ->
 pack_avp(#diameter_avp{data = [#diameter_avp{} | _] = Avps} = A) ->
     pack_avp(A#diameter_avp{data = encode_avps(Avps)});
 
-%% ... data as a type/value tuple, possibly with header data, ...
+%% ... data as a type/value tuple ...
 pack_avp(#diameter_avp{data = {Type, Value}} = A)
   when is_atom(Type) ->
     pack_avp(A#diameter_avp{data = diameter_types:Type(encode, Value)});
+
+%% ... with a header in various forms ...
 pack_avp(#diameter_avp{data = {{_,_,_} = T, {Type, Value}}}) ->
     pack_avp(T, iolist_to_binary(diameter_types:Type(encode, Value)));
+
 pack_avp(#diameter_avp{data = {{_,_,_} = T, Bin}})
   when is_binary(Bin) ->
     pack_avp(T, Bin);
+
 pack_avp(#diameter_avp{data = {Dict, Name, Value}} = A) ->
     {Code, _Flags, Vid} = Hdr = Dict:avp_header(Name),
     {Name, Type} = Dict:avp_name(Code, Vid),
     pack_avp(A#diameter_avp{data = {Hdr, {Type, Value}}});
 
+pack_avp(#diameter_avp{code = undefined, data = Bin})
+  when is_binary(Bin) ->
+    %% Reset the AVP Length of an AVP Header resulting from a 5014
+    %% error. The RFC doesn't explicitly say to do this but the
+    %% receiver can't correctly extract this and following AVP's
+    %% without a correct length. On the downside, the header doesn't
+    %% reveal if the received header has been padded.
+    Pad = 8*header_length(Bin) - bit_size(Bin),
+    Len = size(<<H:5/binary, _:24, T/binary>> = <<Bin/bitstring, 0:Pad>>),
+    <<H/binary, Len:24, T/binary>>;
+
 %% ... or as an iolist.
 pack_avp(#diameter_avp{code = Code,
                        vendor_id = V,
@@ -497,6 +595,11 @@ pack_avp(#diameter_avp{code = Code,
                                             {P, 2#00100000}]),
     pack_avp({Code, Flags, V}, iolist_to_binary(Data)).
 
+header_length(<<_:32, 1:1, _/bitstring>>) ->
+    12;
+header_length(_) ->
+    8.
+
 flag_avp({true, B}, F) ->
     F bor B;
 flag_avp({false, _}, F) ->
diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl
index 25b902e3f2..820d37535a 100644
--- a/lib/diameter/src/base/diameter_traffic.erl
+++ b/lib/diameter/src/base/diameter_traffic.erl
@@ -226,10 +226,10 @@ recv_R(false = No, _, _, _, _) ->  %% transport has gone down
 
 collect_avps(Pkt) ->
     case diameter_codec:collect_avps(Pkt) of
-        {_Bs, As} ->
-            As;
-        As ->
-            As
+        {_Error, Avps} ->
+            Avps;
+        Avps ->
+            Avps
     end.
 
 %% recv_R/6
@@ -300,7 +300,7 @@ errors(_, #diameter_packet{header = #diameter_header{version = V},
 %%      AVP's definition.
 
 errors(_, #diameter_packet{errors = [Bs | Es]} = Pkt)
-  when is_bitstring(Bs) ->
+  when is_bitstring(Bs) ->  %% from old code
     Pkt#diameter_packet{errors = [3009 | Es]};
 
 %%   DIAMETER_COMMAND_UNSUPPORTED       3001
@@ -595,71 +595,87 @@ reply([Msg], Dict, TPid, Dict0, Fs, ReqPkt)
        is_tuple(Msg) ->
     reply(Msg, Dict, TPid, Dict0, Fs, ReqPkt#diameter_packet{errors = []});
 
-%% No errors or a diameter_header/avp list.
 reply(Msg, Dict, TPid, Dict0, Fs, ReqPkt) ->
-    Pkt = encode(Dict, reset(make_answer_packet(Msg, ReqPkt), Dict), Fs),
+    Pkt = encode(Dict,
+                 reset(make_answer_packet(Msg, ReqPkt), Dict, Dict0),
+                 Fs),
     incr(send, Pkt, Dict, TPid, Dict0),  %% count outgoing result codes
     send(TPid, Pkt).
 
-%% reset/2
+%% reset/3
 
 %% Header/avps list: send as is.
-reset(#diameter_packet{msg = [#diameter_header{} | _]} = Pkt, _) ->
+reset(#diameter_packet{msg = [#diameter_header{} | _]} = Pkt, _, _) ->
     Pkt;
 
 %% No errors to set or errors explicitly ignored.
-reset(#diameter_packet{errors = Es} = Pkt, _)
+reset(#diameter_packet{errors = Es} = Pkt, _, _)
   when Es == [];
        Es == false ->
     Pkt;
 
 %% Otherwise possibly set Result-Code and/or Failed-AVP.
-reset(#diameter_packet{msg = Msg, errors = Es} = Pkt, Dict) ->
-    Pkt#diameter_packet{msg = reset(Msg, Dict, Es)}.
-
-%% reset/3
-
-reset(Msg, Dict, Es)
-  when is_list(Es) ->
-    {E3, E5, Fs} = partition(Es),
-    FailedAVP = failed_avp(Msg, lists:reverse(Fs), Dict),
-    reset(set(Msg, FailedAVP, Dict),
-          Dict,
-          choose(is_answer_message(Msg, Dict), E3, E5));
-
-reset(Msg, Dict, N)
-  when is_integer(N) ->
-    ResultCode = rc(Msg, {'Result-Code', N}, Dict),
-    set(Msg, ResultCode, Dict);
-
-reset(Msg, _, _) ->
-    Msg.
-
-partition(Es) ->
-    lists:foldl(fun pacc/2, {false, false, []}, Es).
+reset(#diameter_packet{msg = Msg, errors = Es} = Pkt, Dict, Dict0) ->
+    {RC, Failed} = select_error(Msg, Es, Dict0),
+    Pkt#diameter_packet{msg = reset(Msg, Dict, RC, Failed)}.
 
-%% Note that the errors list can contain not only integer() and
-%% {integer(), #diameter_avp{}} but also #diameter_avp{}. The latter
-%% isn't something that's returned by decode but can be set in a reply
-%% for encode.
+%% select_error/3
+%%
+%% Extract the first appropriate RC or {RC, #diameter_avp{}}
+%% pair from an errors list, and accumulate all #diameter_avp{}.
+%%
+%% RFC 6733:
+%%
+%%  7.5.  Failed-AVP AVP
+%%
+%%   The Failed-AVP AVP (AVP Code 279) is of type Grouped and provides
+%%   debugging information in cases where a request is rejected or not
+%%   fully processed due to erroneous information in a specific AVP.  The
+%%   value of the Result-Code AVP will provide information on the reason
+%%   for the Failed-AVP AVP.  A Diameter answer message SHOULD contain an
+%%   instance of the Failed-AVP AVP that corresponds to the error
+%%   indicated by the Result-Code AVP.  For practical purposes, this
+%%   Failed-AVP would typically refer to the first AVP processing error
+%%   that a Diameter node encounters.
+
+select_error(Msg, Es, Dict0) ->
+    {RC, Avps} = lists:foldl(fun(T,A) -> select(T, A, Dict0) end,
+                             {is_answer_message(Msg, Dict0), []},
+                             Es),
+    {RC, lists:reverse(Avps)}.
+
+%% Only integer() and {integer(), #diameter_avp{}} are the result of
+%% decode. #diameter_avp{} can only be set in a reply for encode.
+
+select(#diameter_avp{} = A, {RC, As}, _) ->
+    {RC, [A|As]};
+
+select(_, {RC, _} = Acc, _)
+  when is_integer(RC) ->
+    Acc;
 
-pacc({RC, #diameter_avp{} = A}, {E3, E5, Acc})
+select({RC, #diameter_avp{} = A}, {IsAns, As} = Acc, Dict0)
   when is_integer(RC) ->
-    pacc(RC, {E3, E5, [A|Acc]});
+    case is_result(RC, IsAns, Dict0) of
+        true  -> {RC, [A|As]};
+        false -> Acc
+    end;
 
-pacc(#diameter_avp{} = A, {E3, E5, Acc}) ->
-    {E3, E5, [A|Acc]};
+select(RC, {IsAns, As} = Acc, Dict0)
+  when is_boolean(IsAns), is_integer(RC) ->
+    case is_result(RC, IsAns, Dict0) of
+        true  -> {RC, As};
+        false -> Acc
+    end.
 
-pacc(RC, {false, E5, Acc})
-  when 3 == RC div 1000 ->
-    {RC, E5, Acc};
+%% reset/4
 
-pacc(RC, {E3, false, Acc})
-  when 5 == RC div 1000 ->
-    {E3, RC, Acc};
+reset(Msg, Dict, RC, Avps) ->
+    FailedAVP = failed_avp(Msg, Avps, Dict),
+    ResultCode = rc(Msg, RC, Dict),
+    set(set(Msg, FailedAVP, Dict), ResultCode, Dict).
 
-pacc(_, Acc) ->
-    Acc.
+%% eval_packet/2
 
 eval_packet(Pkt, Fs) ->
     lists:foreach(fun(F) -> diameter_lib:eval([F,Pkt]) end, Fs).
@@ -725,29 +741,34 @@ set(Rec, Avps, Dict) ->
 %% the arity is 1 or {0,1}. In other cases (which probably shouldn't
 %% exist in practise) we can't know what's appropriate.
 
-rc([MsgName | _], {'Result-Code' = K, RC} = T, Dict) ->
-    case Dict:avp_arity(MsgName, 'Result-Code') of
-        1     -> [T];
+rc(_, B, _)
+  when is_boolean(B) ->
+    [];
+
+rc([MsgName | _], RC, Dict) ->
+    K = 'Result-Code',
+    case Dict:avp_arity(MsgName, K) of
+        1     -> [{K, RC}];
         {0,1} -> [{K, [RC]}];
         _     -> []
     end;
 
-rc(Rec, T, Dict) ->
-    rc([Dict:rec2msg(element(1, Rec))], T, Dict).
+rc(Rec, RC, Dict) ->
+    rc([Dict:rec2msg(element(1, Rec))], RC, Dict).
 
 %% failed_avp/3
 
 failed_avp(_, [] = No, _) ->
     No;
 
-failed_avp(Rec, Failed, Dict) ->
-    [fa(Rec, [{'AVP', Failed}], Dict)].
+failed_avp(Rec, Avps, Dict) ->
+    [failed(Rec, [{'AVP', Avps}], Dict)].
 
 %% Reply as name and tuple list ...
-fa([MsgName | Values], FailedAvp, Dict) ->
-    R = Dict:msg2rec(MsgName),
+failed([MsgName | Values], FailedAvp, Dict) ->
+    RecName = Dict:msg2rec(MsgName),
     try
-        Dict:'#info-'(R, {index, 'Failed-AVP'}),
+        Dict:'#info-'(RecName, {index, 'Failed-AVP'}),
         {'Failed-AVP', [FailedAvp]}
     catch
         error: _ ->
@@ -758,8 +779,10 @@ fa([MsgName | Values], FailedAvp, Dict) ->
     end;
 
 %% ... or record.
-fa(Rec, FailedAvp, Dict) ->
+failed(Rec, FailedAvp, Dict) ->
     try
+        RecName = element(1, Rec),  
+        Dict:'#info-'(RecName, {index, 'Failed-AVP'}),
         {'Failed-AVP', [FailedAvp]}
     catch
         error: _ ->
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(<<I:32/integer>>)],
+    [{?clause, [?ATOM(decode), ?Atom(AvpName), ?TERM(<<I:32>>)],
       [],
       [?TERM(I)]},
      {?clause, [?ATOM(encode), ?Atom(AvpName), ?INTEGER(I)],
       [],
-      [?TERM(<<I:32/integer>>)]}].
+      [?TERM(<<I:32>>)]}].
 
 %%% ------------------------------------------------------------------------
 %%% 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