Properly combine base and extra headers when making replicator requestssort-headers-before-merging

Previously we subtly relied on one set of headers being sorted, then sorted the
other set of headers, and ran `lists:ukeymerge/3`. That function, however,
needs both arguments to be sorted in order for it to work as expected. If one
argument wasn't sorted we could get duplicate headers easily, which is what was
observed in testing.

A better fix than just sorting both sets of keys, is to use an actual header
processing library to combine them so we can account for case insensitivity as
well.

1 files changed, 27 insertions, 2 deletions

diff --git a/src/couch_replicator/src/couch_replicator_httpc.erl b/src/couch_replicator/src/couch_replicator_httpc.erl
index f11d1895d..53158f9a2 100644
--- a/src/couch_replicator/src/couch_replicator_httpc.erl
+++ b/src/couch_replicator/src/couch_replicator_httpc.erl
@@ -97,8 +97,8 @@ send_req(HttpDb, Params1, Callback) ->
 
 send_ibrowse_req(#httpdb{headers = BaseHeaders} = HttpDb0, Params) ->
     Method = get_value(method, Params, get),
-    UserHeaders = lists:keysort(1, get_value(headers, Params, [])),
-    Headers1 = lists:ukeymerge(1, UserHeaders, BaseHeaders),
+    UserHeaders = get_value(headers, Params, []),
+    Headers1 = merge_headers(BaseHeaders, UserHeaders),
     {Headers2, HttpDb} = couch_replicator_auth:update_headers(HttpDb0, Headers1),
     Url = full_url(HttpDb, Params),
     Body = get_value(body, Params, []),
@@ -493,3 +493,28 @@ backoff_before_request(Worker, HttpDb, Params) ->
         Sleep when Sleep == 0 ->
             ok
     end.
+
+
+merge_headers(Headers1, Headers2) when is_list(Headers1), is_list(Headers2) ->
+    Empty = mochiweb_headers:empty(),
+    Merged = mochiweb_headers:enter_from_list(Headers1 ++ Headers2, Empty),
+    mochiweb_headers:to_list(Merged).
+
+
+-ifdef(TEST).
+
+-include_lib("couch/include/couch_eunit.hrl").
+
+
+merge_headers_test() ->
+    Headers1 = [{"a", "x"}, {"a", "y"}, {"A", "z"}],
+    ?assertEqual([], merge_headers([], [])),
+    ?assertEqual([{"a", "x"}], merge_headers([], [{"a", "x"}])),
+    ?assertEqual([{"a", "x"}], merge_headers([{"a", "x"}], [])),
+    ?assertEqual([{"a", "y"}], merge_headers([{"A", "x"}], [{"a", "y"}])),
+    ?assertEqual([{"a", "y"}, {"B", "x"}], merge_headers([{"B", "x"}],
+        [{"a", "y"}])),
+    ?assertEqual([{"a", "y"}], merge_headers([{"A", "z"}, {"a", "y"}], [])),
+    ?assertEqual([{"a", "y"}], merge_headers([], [{"A", "z"}, {"a", "y"}])).
+
+-endif.