Use 'Connection' header only when necessary

Fixes #839 when 'Connection: Keep-Alive' wasn't sent in a HTTP/1.0
response. Now the usage of 'Connection' header is consistent with
current protocol version: when this header is not specified explicitly
in the response, HTTP/1.0 implies 'Connection: close' and HTTP/1.1
implies 'Connection: Keep-Alive'. So if current 'Connection' value
matches the default value of current protocol, we won't state obvious
fact in the response; and vice versa.

Amended to fix and improve tests, and revert the variable name
change from HTTP11Headers to StdHeaders. I think it's still good
to leave it as is because it's not really a standard header for
HTTP/1.0, and it's gone from HTTP/2 entirely.

2 files changed, 24 insertions, 9 deletions

diff --git a/src/cowboy_req.erl b/src/cowboy_req.erl
index b13da50..8f0a04b 100644
--- a/src/cowboy_req.erl
+++ b/src/cowboy_req.erl
@@ -686,7 +686,9 @@ reply(Status, Headers, Body, Req=#http_req{
 		resp_state=RespState, resp_headers=RespHeaders})
 		when RespState =:= waiting; RespState =:= waiting_stream ->
 	HTTP11Headers = if
-		Transport =/= cowboy_spdy, Version =:= 'HTTP/1.1' ->
+		Transport =/= cowboy_spdy, Version =:= 'HTTP/1.0', Connection =:= keepalive ->
+			[{<<"connection">>, atom_to_connection(Connection)}];
+		Transport =/= cowboy_spdy, Version =:= 'HTTP/1.1', Connection =:= close ->
 			[{<<"connection">>, atom_to_connection(Connection)}];
 		true ->
 			[]
@@ -988,13 +990,20 @@ chunked_response(Status, Headers, Req=#http_req{
 		when RespState =:= waiting; RespState =:= waiting_stream ->
 	RespConn = response_connection(Headers, Connection),
 	HTTP11Headers = if
+		Version =:= 'HTTP/1.0', Connection =:= keepalive ->
+			[{<<"connection">>, atom_to_connection(Connection)}];
 		Version =:= 'HTTP/1.0' -> [];
 		true ->
 			MaybeTE = if
 				RespState =:= waiting_stream -> [];
 				true -> [{<<"transfer-encoding">>, <<"chunked">>}]
 			end,
-			[{<<"connection">>, atom_to_connection(Connection)}|MaybeTE]
+			if
+				Connection =:= close ->
+					[{<<"connection">>, atom_to_connection(Connection)}|MaybeTE];
+				true ->
+					MaybeTE
+			end
 	end,
 	RespState2 = if
 		Version =:= 'HTTP/1.1', RespState =:= 'waiting' -> chunks;
diff --git a/test/http_SUITE.erl b/test/http_SUITE.erl
index 314508b..d5ec909 100644
--- a/test/http_SUITE.erl
+++ b/test/http_SUITE.erl
@@ -379,7 +379,10 @@ http10_keepalive_default(Config) ->
 	ok = raw_send(Client, Normal),
 	case catch raw_recv_head(Client) of
 		{'EXIT', _} -> error(closed);
-		_ -> ok
+		Data ->
+			{'HTTP/1.0', 200, _, Rest} = cow_http:parse_status_line(Data),
+			{Headers, _} = cow_http:parse_headers(Rest),
+			false = lists:keymember(<<"connection">>, 1, Headers)
 	end,
 	ok = raw_send(Client, Normal),
 	case catch raw_recv_head(Client) of
@@ -393,7 +396,10 @@ http10_keepalive_forced(Config) ->
 	ok = raw_send(Client, Keepalive),
 	case catch raw_recv_head(Client) of
 		{'EXIT', _} -> error(closed);
-		_ -> ok
+		Data ->
+			{'HTTP/1.0', 200, _, Rest} = cow_http:parse_status_line(Data),
+			{Headers, _} = cow_http:parse_headers(Rest),
+			{_, <<"keep-alive">>} = lists:keyfind(<<"connection">>, 1, Headers)
 	end,
 	ok = raw_send(Client, Keepalive),
 	case catch raw_recv_head(Client) of
@@ -408,7 +414,7 @@ keepalive_max(Config) ->
 	CloseRef = gun:get(ConnPid, "/", [{<<"connection">>, <<"keep-alive">>}]),
 	_ = [begin
 		{response, nofin, 200, Headers} = gun:await(ConnPid, Ref),
-		{_, <<"keep-alive">>} = lists:keyfind(<<"connection">>, 1, Headers)
+		false = lists:keymember(<<"connection">>, 1, Headers)
 	end || Ref <- Refs],
 	{response, nofin, 200, Headers} = gun:await(ConnPid, CloseRef),
 	{_, <<"close">>} = lists:keyfind(<<"connection">>, 1, Headers),
@@ -423,7 +429,7 @@ keepalive_nl(Config) ->
 	end || _ <- lists:seq(1, 10)],
 	_ = [begin
 		{response, nofin, 200, Headers} = gun:await(ConnPid, Ref),
-		{_, <<"keep-alive">>} = lists:keyfind(<<"connection">>, 1, Headers)
+		false = lists:keymember(<<"connection">>, 1, Headers)
 	end || Ref <- Refs],
 	ok.
 
@@ -648,7 +654,7 @@ rest_keepalive(Config) ->
 	Refs = [gun:get(ConnPid, "/simple") || _ <- lists:seq(1, 10)],
 	_ = [begin
 		{response, nofin, 200, Headers} =  gun:await(ConnPid, Ref),
-		{_, <<"keep-alive">>} = lists:keyfind(<<"connection">>, 1, Headers)
+		false = lists:keymember(<<"connection">>, 1, Headers)
 	end || Ref <- Refs],
 	ok.
 
@@ -663,9 +669,9 @@ rest_keepalive_post(Config) ->
 	end || _ <- lists:seq(1, 5)],
 	_ = [begin
 		{response, fin, 403, Headers1} = gun:await(ConnPid, Ref1),
-		{_, <<"keep-alive">>} = lists:keyfind(<<"connection">>, 1, Headers1),
+		false = lists:keymember(<<"connection">>, 1, Headers1),
 		{response, fin, 303, Headers2} = gun:await(ConnPid, Ref2),
-		{_, <<"keep-alive">>} = lists:keyfind(<<"connection">>, 1, Headers2)
+		false = lists:keymember(<<"connection">>, 1, Headers2)
 	end || {Ref1, Ref2} <- Refs],
 	ok.