From 68f16c2ce9f07d9f94937200ad18896419220efb Mon Sep 17 00:00:00 2001 From: Nick Vatamaniuc Date: Fri, 9 Oct 2020 18:10:44 -0400 Subject: Properly combine base and extra headers when making replicator requests 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. --- .../src/couch_replicator_httpc.erl | 28 ++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/couch_replicator/src/couch_replicator_httpc.erl b/src/couch_replicator/src/couch_replicator_httpc.erl index 4dce319dc..466bc5744 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,27 @@ 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() -> + ?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. -- cgit v1.2.1