Fix a bug where dupe headers were sent in cowboy_http_req:reply/4.

Now the server defines default headers that can be overwritten by the
handler simply by passing them to the reply/4 function. Default headers
include, for now, Connection and Content-Length headers. Note that it isn't
enough to change the Connection header to close a keep-alive connection
server-side.

3 files changed, 34 insertions, 12 deletions

diff --git a/src/cowboy_http_req.erl b/src/cowboy_http_req.erl
index 8ea8efc..4907503 100644
--- a/src/cowboy_http_req.erl
+++ b/src/cowboy_http_req.erl
@@ -174,15 +174,18 @@ body_qs(Req) ->
 
 -spec reply(Code::http_status(), Headers::http_headers(),
 	Body::iolist(), Req::#http_req{}) -> {ok, Req::#http_req{}}.
-%% @todo Don't be naive about the headers!
 reply(Code, Headers, Body, Req=#http_req{socket=Socket,
 		transport=Transport, connection=Connection,
 		resp_state=waiting}) ->
 	StatusLine = ["HTTP/1.1 ", status(Code), "\r\n"],
-	BaseHeaders = ["Connection: ", atom_to_connection(Connection),
-		"\r\nContent-Length: ", integer_to_list(iolist_size(Body)), "\r\n"],
-	Transport:send(Socket,
-		[StatusLine, BaseHeaders, Headers, "\r\n", Body]),
+	DefaultHeaders = [
+		{"Connection", atom_to_connection(Connection)},
+		{"Content-Length", integer_to_list(iolist_size(Body))}
+	],
+	Headers2 = lists:keysort(1, Headers),
+	Headers3 = lists:ukeymerge(1, Headers2, DefaultHeaders),
+	Headers4 = [[Key, ": ", Value, "\r\n"] || {Key, Value} <- Headers3],
+	Transport:send(Socket, [StatusLine, Headers4, "\r\n", Body]),
 	{ok, Req#http_req{resp_state=done}}.
 
 %% Internal.
diff --git a/test/http_SUITE.erl b/test/http_SUITE.erl
index 663878b..30ec709 100644
--- a/test/http_SUITE.erl
+++ b/test/http_SUITE.erl
@@ -18,7 +18,7 @@
 
 -export([all/0, groups/0, init_per_suite/1, end_per_suite/1,
 	init_per_group/2, end_per_group/2]). %% ct.
--export([pipeline/1, raw/1]). %% http.
+-export([headers_dupe/1, pipeline/1, raw/1]). %% http.
 -export([http_200/1, http_404/1]). %% http and https.
 
 %% ct.
@@ -28,7 +28,7 @@ all() ->
 
 groups() ->
 	BaseTests = [http_200, http_404],
-	[{http, [], [pipeline, raw] ++ BaseTests},
+	[{http, [], [headers_dupe, pipeline, raw] ++ BaseTests},
 	{https, [], BaseTests}].
 
 init_per_suite(Config) ->
@@ -76,7 +76,11 @@ end_per_group(https, _Config) ->
 
 init_http_dispatch() ->
 	[
-		{["localhost"], [{[], http_handler, []}]}
+		{["localhost"], [
+			{["headers", "dupe"], http_handler,
+				[{headers, [{"Connection", "close"}]}]},
+			{[], http_handler, []}
+		]}
 	].
 
 init_https_dispatch() ->
@@ -84,6 +88,17 @@ init_https_dispatch() ->
 
 %% http.
 
+headers_dupe(Config) ->
+	{port, Port} = lists:keyfind(port, 1, Config),
+	{ok, Socket} = gen_tcp:connect("localhost", Port,
+		[binary, {active, false}, {packet, raw}]),
+	ok = gen_tcp:send(Socket,
+		"GET /headers/dupe HTTP/1.1\r\nHost: localhost\r\nConnection: keep-alive\r\n\r\n"),
+	{ok, Data} = gen_tcp:recv(Socket, 0, 6000),
+	{_Start, _Length} = binary:match(Data, <<"Connection: close">>),
+	nomatch = binary:match(Data, <<"Connection: keep-alive">>),
+	ok = gen_tcp:close(Socket).
+
 pipeline(Config) ->
 	{port, Port} = lists:keyfind(port, 1, Config),
 	{ok, Socket} = gen_tcp:connect("localhost", Port,
diff --git a/test/http_handler.erl b/test/http_handler.erl
index 3882e17..76a85d4 100644
--- a/test/http_handler.erl
+++ b/test/http_handler.erl
@@ -4,11 +4,15 @@
 -behaviour(cowboy_http_handler).
 -export([init/3, handle/2, terminate/2]).
 
-init({_Transport, http}, Req, _Opts) ->
-	{ok, Req, undefined}.
+-record(state, {headers, body}).
 
-handle(Req, State) ->
-	{ok, Req2} = cowboy_http_req:reply(200, [], "http_handler", Req),
+init({_Transport, http}, Req, Opts) ->
+	Headers = proplists:get_value(headers, Opts, []),
+	Body = proplists:get_value(body, Opts, "http_handler"),
+	{ok, Req, #state{headers=Headers, body=Body}}.
+
+handle(Req, State=#state{headers=Headers, body=Body}) ->
+	{ok, Req2} = cowboy_http_req:reply(200, Headers, Body, Req),
 	{ok, Req2, State}.
 
 terminate(_Req, _State) ->