From 5269bf580bc1bc75bb1afe59d52ef2c4772ff12d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= <essen@ninenines.eu>
Date: Wed, 6 Dec 2017 16:30:30 +0100
Subject: Fix 408 not sending connection: close for HTTP/1.1

Also make sure the header is sent for all types of early_error
that result in the closing of the connection.
---
 src/cowboy_http.erl    | 14 ++++++++++----
 test/rfc7231_SUITE.erl | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl
index 71b7099..bc8fbb9 100644
--- a/src/cowboy_http.erl
+++ b/src/cowboy_http.erl
@@ -1157,16 +1157,22 @@ error_terminate(StatusCode, State=#state{ref=Ref, peer=Peer, in_state=StreamStat
 			end
 		}
 	end,
-	early_error(StatusCode, State, Reason, PartialReq),
+	early_error(StatusCode, State, Reason, PartialReq, #{<<"connection">> => <<"close">>}),
 	terminate(State, Reason).
 
+early_error(StatusCode, State, Reason, PartialReq) ->
+	early_error(StatusCode, State, Reason, PartialReq, #{}).
+
 early_error(StatusCode0, #state{socket=Socket, transport=Transport,
-		opts=Opts, in_streamid=StreamID}, Reason, PartialReq) ->
-	Resp = {response, StatusCode0, RespHeaders0=#{<<"content-length">> => <<"0">>}, <<>>},
+		opts=Opts, in_streamid=StreamID}, Reason, PartialReq, RespHeaders0) ->
+	RespHeaders1 = RespHeaders0#{<<"content-length">> => <<"0">>},
+	Resp = {response, StatusCode0, RespHeaders1, <<>>},
 	try cowboy_stream:early_error(StreamID, Reason, PartialReq, Resp, Opts) of
 		{response, StatusCode, RespHeaders, RespBody} ->
 			Transport:send(Socket, [
 				cow_http:response(StatusCode, 'HTTP/1.1', maps:to_list(RespHeaders)),
+				%% @todo We shouldn't send the body when the method is HEAD.
+				%% @todo Technically we allow the sendfile tuple.
 				RespBody
 			])
 	catch Class:Exception ->
@@ -1176,7 +1182,7 @@ early_error(StatusCode0, #state{socket=Socket, transport=Transport,
 		%% We still need to send an error response, so send what we initially
 		%% wanted to send. It's better than nothing.
 		Transport:send(Socket, cow_http:response(StatusCode0,
-			'HTTP/1.1', maps:to_list(RespHeaders0)))
+			'HTTP/1.1', maps:to_list(RespHeaders1)))
 	end,
 	ok.
 
diff --git a/test/rfc7231_SUITE.erl b/test/rfc7231_SUITE.erl
index 5d5a803..f1b3415 100644
--- a/test/rfc7231_SUITE.erl
+++ b/test/rfc7231_SUITE.erl
@@ -19,6 +19,10 @@
 -import(ct_helper, [config/2]).
 -import(ct_helper, [doc/1]).
 -import(cowboy_test, [gun_open/1]).
+-import(cowboy_test, [raw_open/1]).
+-import(cowboy_test, [raw_send/2]).
+-import(cowboy_test, [raw_recv_head/1]).
+-import(cowboy_test, [raw_recv/3]).
 
 all() ->
 	cowboy_test:common_all().
@@ -139,6 +143,20 @@ method_connect(Config) ->
 	{response, fin, 501, _} = gun:await(ConnPid, Ref),
 	ok.
 
+%   A client sending a CONNECT request MUST send the authority form of
+%   request-target (Section 5.3 of [RFC7230]); i.e., the request-target
+%   consists of only the host name and port number of the tunnel
+%   destination, separated by a colon.
+%
+%   A server MUST NOT send any Transfer-Encoding or Content-Length header
+%   fields in a 2xx (Successful) response to CONNECT.  A client MUST
+%   ignore any Content-Length or Transfer-Encoding header fields received
+%   in a successful response to CONNECT.
+%
+%   A payload within a CONNECT request message has no defined semantics;
+%   sending a payload body on a CONNECT request might cause some existing
+%   implementations to reject the request.
+
 method_options(Config) ->
 	doc("The OPTIONS method is accepted. (RFC7231 4.3.7)"),
 	ConnPid = gun_open(Config),
@@ -405,6 +423,25 @@ status_code_408(Config) ->
 	{response, _, 408, _} = gun:await(ConnPid, Ref),
 	ok.
 
+status_code_408_connection_close(Config) ->
+	case config(protocol, Config) of
+		http ->
+			do_http11_status_code_408_connection_close(Config);
+		http2 ->
+			doc("HTTP/2 connections are not closed on 408 responses.")
+	end.
+
+do_http11_status_code_408_connection_close(Config) ->
+	doc("A 408 response should result in a connection close "
+		"for HTTP/1.1 connections. (RFC7231 6.5.7)"),
+	Client = raw_open(Config),
+	ok = raw_send(Client, "GET / HTTP/1.1\r\n"),
+	{_, 408, _, Rest} = cow_http:parse_status_line(raw_recv_head(Client)),
+	{Headers, <<>>} = cow_http:parse_headers(Rest),
+	{_, <<"close">>} = lists:keyfind(<<"connection">>, 1, Headers),
+	{error, closed} = raw_recv(Client, 0, 1000),
+	ok.
+
 status_code_409(Config) ->
 	doc("The 409 Conflict status code can be sent. (RFC7231 6.5.8)"),
 	ConnPid = gun_open(Config),
-- 
cgit v1.2.3