merge default

5 files changed, 157 insertions, 10 deletions

diff --git a/include/rabbit.hrl b/include/rabbit.hrl
index 3db2b68a..dc99bf78 100644
--- a/include/rabbit.hrl
+++ b/include/rabbit.hrl
@@ -101,5 +101,6 @@
 -define(DESIRED_HIBERNATE,         10000).
 -define(CREDIT_DISC_BOUND,   {2000, 500}).
 
+-define(INVALID_HEADERS_KEY, <<"x-invalid-headers">>).
 -define(ROUTING_HEADERS, [<<"CC">>, <<"BCC">>]).
 -define(DELETED_HEADER, <<"BCC">>).
diff --git a/src/rabbit_amqqueue_process.erl b/src/rabbit_amqqueue_process.erl
index 8d05a78c..ec5ef3f8 100644
--- a/src/rabbit_amqqueue_process.erl
+++ b/src/rabbit_amqqueue_process.erl
@@ -853,8 +853,8 @@ make_dead_letter_msg(Reason,
                         {<<"time">>,         timestamp, TimeSec},
                         {<<"exchange">>,     longstr,   Exchange#resource.name},
                         {<<"routing-keys">>, array,     RKs1}],
-                HeadersFun1(rabbit_basic:append_table_header(<<"x-death">>,
-                                                             Info, Headers))
+                HeadersFun1(rabbit_basic:prepend_table_header(<<"x-death">>,
+                                                              Info, Headers))
         end,
     Content1 = rabbit_basic:map_headers(HeadersFun2, Content),
     Msg#basic_message{exchange_name = DLX, id = rabbit_guid:gen(),
diff --git a/src/rabbit_basic.erl b/src/rabbit_basic.erl
index db2b7e95..c3250fb1 100644
--- a/src/rabbit_basic.erl
+++ b/src/rabbit_basic.erl
@@ -19,7 +19,7 @@
 -include("rabbit_framing.hrl").
 
 -export([publish/4, publish/5, publish/1,
-         message/3, message/4, properties/1, append_table_header/3,
+         message/3, message/4, properties/1, prepend_table_header/3,
          extract_headers/1, map_headers/2, delivery/3, header_routes/1]).
 -export([build_content/2, from_content/1]).
 
@@ -177,15 +177,50 @@ properties(P) when is_list(P) ->
                         end
                 end, #'P_basic'{}, P).
 
-append_table_header(Name, Info, undefined) ->
-    append_table_header(Name, Info, []);
-append_table_header(Name, Info, Headers) ->
-    Prior = case rabbit_misc:table_lookup(Headers, Name) of
-                undefined          -> [];
-                {array, Existing}  -> Existing
-            end,
+prepend_table_header(Name, Info, undefined) ->
+    prepend_table_header(Name, Info, []);
+prepend_table_header(Name, Info, Headers) ->
+    case rabbit_misc:table_lookup(Headers, Name) of
+        {array, Existing} ->
+            prepend_table(Headers, Name, Info, Existing);
+        undefined ->
+            prepend_table(Headers, Name, Info, []);
+        Other ->
+            Headers2 = prepend_table(Headers, Name, Info, []),
+            set_invalid_header(Name, Other, Headers2)
+    end.
+
+prepend_table(Headers, Name, Info, Prior) ->
     rabbit_misc:set_table_value(Headers, Name, array, [{table, Info} | Prior]).
 
+set_invalid_header(Name, {_, _}=Value, Headers) when is_list(Headers) ->
+    case rabbit_misc:table_lookup(Headers, ?INVALID_HEADERS_KEY) of
+        undefined ->
+            Invalid = [{Name, array, [Value]}],
+            rabbit_misc:set_table_value(Headers, ?INVALID_HEADERS_KEY,
+                                        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) ->
     #content{properties = #'P_basic'{headers = Headers}} =
         rabbit_binary_parser:ensure_content_decoded(Content),
diff --git a/src/rabbit_basic_tests.erl b/src/rabbit_basic_tests.erl
new file mode 100644
index 00000000..19c901c2
--- /dev/null
+++ b/src/rabbit_basic_tests.erl
@@ -0,0 +1,110 @@
+%% The contents of this file are subject to the Mozilla Public License
+%% Version 1.1 (the "License"); you may not use this file except in
+%% compliance with the License. You may obtain a copy of the License
+%% at http://www.mozilla.org/MPL/
+%%
+%% Software distributed under the License is distributed on an "AS IS"
+%% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See
+%% the License for the specific language governing rights and
+%% limitations under the License.
+%%
+%% The Original Code is RabbitMQ.
+%%
+%% The Initial Developer of the Original Code is VMware, Inc.
+%% Copyright (c) 2007-2012 VMware, Inc.  All rights reserved.
+%%
+-module(rabbit_basic_tests).
+
+-include("rabbit.hrl").
+-include_lib("eunit/include/eunit.hrl").
+
+-compile(export_all).
+
+-define(XDEATH_TABLE,
+        [{<<"reason">>,       longstr,   <<"blah">>},
+         {<<"queue">>,        longstr,   <<"foo.bar.baz">>},
+         {<<"exchange">>,     longstr,   <<"my-exchange">>},
+         {<<"routing-keys">>, array,     []}]).
+
+-define(ROUTE_TABLE, [{<<"redelivered">>, bool, <<"true">>}]).
+
+write_table_with_invalid_existing_type_test_() ->
+    [{"existing entries with invalid types are moved to a table "
+      "stored as <<\"x-invalid-headers header\">>",
+      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:prepend_table_header(
+                      <<"x-received-from">>, ?ROUTE_TABLE, BadHeaders),
+          ?_assertEqual({array, [{table, ?ROUTE_TABLE}]},
+                        rabbit_misc:table_lookup(Headers, <<"x-received-from">>))
+      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:prepend_table_header(HeaderKey, HeaderTable,
+                                               [{HeaderKey, TBin, VBin}]),
+    InvalidHeaders = rabbit_misc:table_lookup(Headers, ?INVALID_HEADERS_KEY),
+    ?assertMatch({table, _}, InvalidHeaders),
+    {_, Invalid} = InvalidHeaders,
+    InvalidArrayForKey = rabbit_misc:table_lookup(Invalid, HeaderKey),
+    ?assertMatch({array, _}, InvalidArrayForKey),
+    {_, Array} = InvalidArrayForKey,
+    ?assertMatch([InvalidEntry], Array),
+    Headers.
diff --git a/src/rabbit_tests.erl b/src/rabbit_tests.erl
index 962bb648..bea94c15 100644
--- a/src/rabbit_tests.erl
+++ b/src/rabbit_tests.erl
@@ -34,6 +34,7 @@
 all_tests() ->
     ok = setup_cluster(),
     ok = supervisor2_tests:test_all(),
+    ok = rabbit_basic_tests:test(),
     passed = gm_tests:all_tests(),
     passed = mirrored_supervisor_tests:all_tests(),
     application:set_env(rabbit, file_handles_high_watermark, 10, infinity),