Fix best effort decode of Failed-AVP

Commit c2c00fdd didn't get it quite right: it only decoded failed AVPs
in the common dictionary since it's this dictionary an answer-message is
decoded in. An extra dictionary isn't something that's easily passed
through the decode without rewriting dictionary compilation however, and
that's no small job, so continue with the use/abuse of the process
dictionary by storing the dictionary module for the decode to retrieve.
This is one step worse than previous uses since the dictionary is put in
one module (diameter_codec) and got in another (the dictionary module),
but it's the lesser of two evils.

3 files changed, 59 insertions, 13 deletions

diff --git a/lib/diameter/include/diameter_gen.hrl b/lib/diameter/include/diameter_gen.hrl
index 97cf0c7e29..233eb8dd15 100644
--- a/lib/diameter/include/diameter_gen.hrl
+++ b/lib/diameter/include/diameter_gen.hrl
@@ -311,7 +311,9 @@ d(Name, Avp, Acc) ->
 
     Failed = relax(Name),  %% Not AvpName or else a failed Failed-AVP
                            %% decode is packed into 'AVP'.
-    try avp(decode, Data, AvpName) of
+    Mod = dict(Failed),    %% Dictionary to decode in.
+
+    try Mod:avp(decode, Data, AvpName) of
         V ->
             {Avps, T} = Acc,
             {H, A} = ungroup(V, Avp),
@@ -324,6 +326,25 @@ d(Name, Avp, Acc) ->
         reset(?FAILED_KEY, Failed)
     end.
 
+%% dict/1
+%%
+%% Retrieve the dictionary for the best-effort decode of Failed-AVP,
+%% as put by diameter_codec:decode/2. See that function for the
+%% explanation.
+
+dict(true) ->
+    case get({diameter_codec, dictionary}) of
+        undefined ->
+            ?MODULE;
+        Mod ->
+            Mod
+    end;
+
+dict(_) ->
+    ?MODULE.
+
+%% d/5
+
 %% Ignore a decode error within Failed-AVP ...
 d(true, _, Name, Avp, Acc) ->
     decode_AVP(Name, Avp, Acc);
@@ -341,6 +362,8 @@ d(false, Reason, Name, Avp, {Avps, Acc}) ->
     {Rec, Failed} = Acc,
     {[Avp|Avps], {Rec, [rc(Reason, Avp) | Failed]}}.
 
+%% relax/2
+
 %% Set false in the process dictionary as soon as we see a Grouped AVP
 %% that doesn't set the M-bit, so that is_strict() can say whether or
 %% not to ignore the M-bit on an encapsulated AVP.
@@ -357,22 +380,23 @@ relax(_, _) ->
 is_strict() ->
     false /= getr(?STRICT_KEY).
 
+%% relax/1
+%%
 %% Set true in the process dictionary as soon as we see Failed-AVP.
 %% Matching on 'Failed-AVP' assumes that this is the RFC AVP.
 %% Strictly, this doesn't need to be the case.
+
 relax('Failed-AVP') ->
-    case getr(?FAILED_KEY) of
-        undefined ->
-            putr(?FAILED_KEY, true);
-        true = Yes ->
-            Yes
-    end;
+    is_failed() orelse putr(?FAILED_KEY, true);
+
 relax(_) ->
     is_failed().
     
 is_failed() ->
     true == getr(?FAILED_KEY).
 
+%% reset/2
+
 reset(Key, undefined) ->
     eraser(Key);
 reset(_, _) ->
diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl
index 06a4f5de64..a2b04bfd63 100644
--- a/lib/diameter/src/base/diameter_codec.erl
+++ b/lib/diameter/src/base/diameter_codec.erl
@@ -237,15 +237,35 @@ rec2msg(Mod, Rec) ->
 
 %% Unsuccessfully decoded AVPs will be placed in #diameter_packet.errors.
 
--spec decode(module(), #diameter_packet{} | binary())
+-spec decode(module() | {module(), module()}, #diameter_packet{} | binary())
    -> #diameter_packet{}.
 
+%% An Answer setting the E-bit. The application dictionary is needed
+%% for the best-effort decode of Failed-AVP, and the best way to make
+%% this available to the AVP decode in diameter_gen.hrl, without
+%% having to rewrite the entire codec generation, is to place it in
+%% the process dictionary. It's the code in diameter_gen.hrl (that's
+%% included by every generated codec module) that looks for the entry.
+%% Not ideal, but it solves the problem relatively simply.
+decode({Mod, Mod}, Pkt) ->
+    decode(Mod, Pkt);
+decode({Mod, AppMod}, Pkt) ->
+    Key = {?MODULE, dictionary},
+    put(Key, AppMod),
+    try
+        decode(Mod, Pkt)
+    after
+        erase(Key)
+    end;
+
+%% Or not: a request, or an answer not setting the E-bit.
 decode(Mod, Pkt) ->
     decode(Mod:id(), Mod, Pkt).
 
-%% If we're a relay application then just extract the avp's without
-%% any decoding of their data since we don't know the application in
-%% question.
+%% decode/3
+
+%% Relay application: just extract the avp's without any decoding of
+%% their data since we don't know the application in question.
 decode(?APP_ID_RELAY, _, #diameter_packet{} = Pkt) ->
     case collect_avps(Pkt) of
         {E, As} ->
@@ -274,6 +294,8 @@ decode(Id, Mod, Bin)
   when is_binary(Bin) ->
     decode(Id, Mod, #diameter_packet{header = decode_header(Bin), bin = Bin}).
 
+%% decode_avps/4
+
 decode_avps(MsgName, Mod, Pkt, {E, Avps}) ->
     ?LOG(invalid_avp_length, Pkt#diameter_packet.header),
     #diameter_packet{errors = Failed}
diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl
index c4875c5975..280d09d7e8 100644
--- a/lib/diameter/src/base/diameter_traffic.erl
+++ b/lib/diameter/src/base/diameter_traffic.erl
@@ -476,7 +476,7 @@ send_A({Caps, Pkt}, TPid, Dict0, _RecvData) ->  %% unsupported application
     #diameter_packet{errors = [RC|_]} = Pkt,
     send_A(answer_message(RC, Caps, Dict0, Pkt),
            TPid,
-           Dict0,
+           {Dict0, Dict0},
            Pkt,
            [],
            []);
@@ -1457,7 +1457,7 @@ handle_answer(SvcName,
               = App,
               {answer, Req, Dict0, Pkt}) ->
     Dict = dict(AppDict, Dict0, Pkt),
-    handle_A(errors(Id, diameter_codec:decode(Dict, Pkt)),
+    handle_A(errors(Id, diameter_codec:decode({Dict, AppDict}, Pkt)),
              SvcName,
              Dict,
              Dict0,