accumulate invalid headers carefully, considering even misconfigured x-invalid-headers

2 files changed, 76 insertions, 9 deletions

diff --git a/src/rabbit_basic.erl b/src/rabbit_basic.erl
index 95aaf1cd..c3250fb1 100644
--- a/src/rabbit_basic.erl
+++ b/src/rabbit_basic.erl
@@ -198,7 +198,27 @@ set_invalid_header(Name, {_, _}=Value, Headers) when is_list(Headers) ->
         undefined ->
             Invalid = [{Name, array, [Value]}],
             rabbit_misc:set_table_value(Headers, ?INVALID_HEADERS_KEY,
-                                        table, Invalid)
+                                        table, Invalid);
+        {table, InvalidEntries} ->
+            case rabbit_misc:table_lookup(InvalidEntries, Name) of
+                undefined ->
+                    rabbit_misc:set_table_value(
+                      Headers, ?INVALID_HEADERS_KEY, table,
+                      rabbit_misc:set_table_value(InvalidEntries,
+                                                  Name, array, [Value]));
+                {array, Prior} ->
+                    rabbit_misc:set_table_value(
+                      Headers, ?INVALID_HEADERS_KEY, table,
+                      rabbit_misc:set_table_value(InvalidEntries,
+                                                  Name, array, [Value | Prior]))
+            end;
+        Other ->
+            %% somehow the x-invalid-headers header is corrupt
+            set_invalid_header(
+              Name, Value,
+              rabbit_misc:set_table_value(
+                Headers, ?INVALID_HEADERS_KEY,
+                table, [{?INVALID_HEADERS_KEY, array, [Other]}]))
     end.
 
 extract_headers(Content) ->
diff --git a/src/rabbit_basic_tests.erl b/src/rabbit_basic_tests.erl
index 2beb4c18..19c901c2 100644
--- a/src/rabbit_basic_tests.erl
+++ b/src/rabbit_basic_tests.erl
@@ -31,26 +31,74 @@
 write_table_with_invalid_existing_type_test_() ->
     [{"existing entries with invalid types are moved to a table "
       "stored as <<\"x-invalid-headers header\">>",
-      assert_invalid(<<"x-death">>,
-                    {longstr, <<"this should be a table!!!">>},
-                    ?XDEATH_TABLE)},
+      fun() ->
+              check_invalid(<<"x-death">>,
+                            {longstr, <<"this should be a table!!!">>},
+                            ?XDEATH_TABLE)
+      end},
      {"if invalid existing headers are moved, newly added "
       "ones are still stored correctly",
       begin
           BadHeaders = [{<<"x-received-from">>,
                          longstr, <<"this should be a table!!!">>}],
-          Headers = rabbit_basic:append_table_header(
+          Headers = rabbit_basic:prepend_table_header(
                       <<"x-received-from">>, ?ROUTE_TABLE, BadHeaders),
           ?_assertEqual({array, [{table, ?ROUTE_TABLE}]},
                         rabbit_misc:table_lookup(Headers, <<"x-received-from">>))
-      end}
-    ].
+      end},
+     {"disparate invalid header entries are accumulated separately",
+      begin
+          BadHeaders = [{<<"x-received-from">>,
+                         longstr, <<"this should be a table!!!">>}],
+          Headers = rabbit_basic:prepend_table_header(
+                      <<"x-received-from">>, ?ROUTE_TABLE, BadHeaders),
+          BadHeaders2 = rabbit_basic:prepend_table_header(
+                          <<"x-death">>, ?XDEATH_TABLE,
+                          [{<<"x-death">>,
+                            longstr, <<"and so should this!!!">>}|Headers]),
+          ?_assertMatch(
+             {table,
+              [{<<"x-death">>, array, [{longstr, <<"and so should this!!!">>}]},
+               {<<"x-received-from">>,
+                array, [{longstr, <<"this should be a table!!!">>}]}]},
+             rabbit_misc:table_lookup(BadHeaders2, ?INVALID_HEADERS_KEY))
+      end},
+     {"corrupt or invalid x-invalid-headers entries are overwritten!",
+      begin
+          Headers0 = [{<<"x-death">>, longstr, <<"this should be a table">>},
+                        {?INVALID_HEADERS_KEY, longstr, <<"what the!?">>}],
+          Headers1 = rabbit_basic:prepend_table_header(
+                      <<"x-death">>, ?XDEATH_TABLE, Headers0),
+          ?_assertMatch(
+             {table,
+              [{<<"x-death">>, array,
+                [{longstr, <<"this should be a table">>}]},
+               {?INVALID_HEADERS_KEY, array,
+                [{longstr, <<"what the!?">>}]}]},
+             rabbit_misc:table_lookup(Headers1, ?INVALID_HEADERS_KEY))
+      end}].
+
+invalid_same_header_entry_accumulation_test() ->
+    Key = <<"x-received-from">>,
+    BadHeader1 = {longstr, <<"this should be a table!!!">>},
+    Headers = check_invalid(Key, BadHeader1, ?ROUTE_TABLE),
+    Headers2 = rabbit_basic:prepend_table_header(
+                 Key,
+                 ?ROUTE_TABLE,
+                 [{Key, longstr,
+                   <<"this should also be a table!!!">>}|Headers]),
+    Invalid = rabbit_misc:table_lookup(Headers2, ?INVALID_HEADERS_KEY),
+    ?assertMatch({table, _}, Invalid),
+    {table, InvalidHeaders} = Invalid,
+    ?assertMatch({array,
+                  [{longstr, <<"this should also be a table!!!">>},BadHeader1]},
+                 rabbit_misc:table_lookup(InvalidHeaders, Key)).
 
 assert_invalid(HeaderKey, Entry, Table) ->
     fun() -> check_invalid(HeaderKey, Entry, Table) end.
 
 check_invalid(HeaderKey, {TBin, VBin}=InvalidEntry, HeaderTable) ->
-    Headers = rabbit_basic:append_table_header(HeaderKey, HeaderTable,
+    Headers = rabbit_basic:prepend_table_header(HeaderKey, HeaderTable,
                                                [{HeaderKey, TBin, VBin}]),
     InvalidHeaders = rabbit_misc:table_lookup(Headers, ?INVALID_HEADERS_KEY),
     ?assertMatch({table, _}, InvalidHeaders),
@@ -60,4 +108,3 @@ check_invalid(HeaderKey, {TBin, VBin}=InvalidEntry, HeaderTable) ->
     {_, Array} = InvalidArrayForKey,
     ?assertMatch([InvalidEntry], Array),
     Headers.
-