Reject responses with explicit set-cookie header

LH: The tests received a lot of fixes and tweaking.
    I also reworded the error message to be more concise.

3 files changed, 83 insertions, 0 deletions

diff --git a/src/cowboy_req.erl b/src/cowboy_req.erl
index 840d349..8edf4ff 100644
--- a/src/cowboy_req.erl
+++ b/src/cowboy_req.erl
@@ -718,6 +718,9 @@ set_resp_cookie(Name, Value, Req, Opts) ->
 
 -spec set_resp_header(binary(), iodata(), Req)
 	-> Req when Req::req().
+set_resp_header(<<"set-cookie">>, _, _) ->
+	exit({response_error, invalid_header,
+		'Response cookies must be set using cowboy_req:set_resp_cookie/3,4.'});
 set_resp_header(Name, Value, Req=#{resp_headers := RespHeaders}) ->
 	Req#{resp_headers => RespHeaders#{Name => Value}};
 set_resp_header(Name,Value, Req) ->
@@ -725,6 +728,9 @@ set_resp_header(Name,Value, Req) ->
 
 -spec set_resp_headers(cowboy:http_headers(), Req)
 	-> Req when Req::req().
+set_resp_headers(#{<<"set-cookie">> := _}, _) ->
+	exit({response_error, invalid_header,
+		'Response cookies must be set using cowboy_req:set_resp_cookie/3,4.'});
 set_resp_headers(Headers, Req=#{resp_headers := RespHeaders}) ->
 	Req#{resp_headers => maps:merge(RespHeaders, Headers)};
 set_resp_headers(Headers, Req) ->
@@ -781,6 +787,9 @@ inform(Status, Req) ->
 inform(_, _, #{has_sent_resp := _}) ->
 	exit({response_error, response_already_sent,
 		'The final response has already been sent.'});
+inform(_, #{<<"set-cookie">> := _}, _) ->
+	exit({response_error, invalid_header,
+		'Response cookies must be set using cowboy_req:set_resp_cookie/3,4.'});
 inform(Status, Headers, Req) when is_integer(Status); is_binary(Status) ->
 	cast({inform, Status, Headers}, Req).
 
@@ -800,6 +809,9 @@ reply(Status, Headers, Req) ->
 reply(_, _, _, #{has_sent_resp := _}) ->
 	exit({response_error, response_already_sent,
 		'The final response has already been sent.'});
+reply(_, #{<<"set-cookie">> := _}, _, _) ->
+	exit({response_error, invalid_header,
+		'Response cookies must be set using cowboy_req:set_resp_cookie/3,4.'});
 reply(Status, Headers, {sendfile, _, 0, _}, Req)
 		when is_integer(Status); is_binary(Status) ->
 	do_reply(Status, Headers#{
@@ -857,6 +869,9 @@ stream_reply(Status, Req) ->
 stream_reply(_, _, #{has_sent_resp := _}) ->
 	exit({response_error, response_already_sent,
 		'The final response has already been sent.'});
+stream_reply(_, #{<<"set-cookie">> := _}, _) ->
+	exit({response_error, invalid_header,
+		'Response cookies must be set using cowboy_req:set_resp_cookie/3,4.'});
 %% 204 and 304 responses must NOT send a body. We therefore
 %% transform the call to a full response and expect the user
 %% to NOT call stream_body/3 afterwards. (RFC7230 3.3)
@@ -908,6 +923,9 @@ stream_events(Events, IsFin, Req=#{has_sent_resp := headers}) ->
 	stream_body({data, self(), IsFin, cow_sse:events(Events)}, Req).
 
 -spec stream_trailers(cowboy:http_headers(), req()) -> ok.
+stream_trailers(#{<<"set-cookie">> := _}, _) ->
+	exit({response_error, invalid_header,
+		'Response cookies must be set using cowboy_req:set_resp_cookie/3,4.'});
 stream_trailers(Trailers, Req=#{has_sent_resp := headers}) ->
 	cast({trailers, Trailers}, Req).
 
diff --git a/test/handlers/resp_h.erl b/test/handlers/resp_h.erl
index db3844b..aae9eb9 100644
--- a/test/handlers/resp_h.erl
+++ b/test/handlers/resp_h.erl
@@ -30,6 +30,10 @@ do(<<"set_resp_cookie4">>, Req0, Opts) ->
 do(<<"set_resp_header">>, Req0, Opts) ->
 	Req = cowboy_req:set_resp_header(<<"content-type">>, <<"text/plain">>, Req0),
 	{ok, cowboy_req:reply(200, #{}, "OK", Req), Opts};
+do(<<"set_resp_header_cookie">>, Req0, Opts) ->
+	ct_helper:ignore(cowboy_req, set_resp_header, 3),
+	Req = cowboy_req:set_resp_header(<<"set-cookie">>, <<"name=value">>, Req0),
+	{ok, cowboy_req:reply(200, #{}, "OK", Req), Opts};
 do(<<"set_resp_header_server">>, Req0, Opts) ->
 	Req = cowboy_req:set_resp_header(<<"server">>, <<"nginx">>, Req0),
 	{ok, cowboy_req:reply(200, #{}, "OK", Req), Opts};
@@ -39,6 +43,12 @@ do(<<"set_resp_headers">>, Req0, Opts) ->
 		<<"content-encoding">> => <<"compress">>
 	}, Req0),
 	{ok, cowboy_req:reply(200, #{}, "OK", Req), Opts};
+do(<<"set_resp_headers_cookie">>, Req0, Opts) ->
+	ct_helper:ignore(cowboy_req, set_resp_headers, 2),
+	Req = cowboy_req:set_resp_headers(#{
+		<<"set-cookie">> => <<"name=value">>
+	}, Req0),
+	{ok, cowboy_req:reply(200, #{}, "OK", Req), Opts};
 do(<<"set_resp_headers_http11">>, Req0, Opts) ->
 	Req = cowboy_req:set_resp_headers(#{
 		<<"connection">> => <<"custom-header, close">>,
@@ -147,6 +157,9 @@ do(<<"inform3">>, Req0, Opts) ->
 		<<"error">> ->
 			ct_helper:ignore(cowboy_req, inform, 3),
 			cowboy_req:inform(ok, Headers, Req0);
+		<<"set_cookie">> ->
+			ct_helper:ignore(cowboy_req, inform, 3),
+			cowboy_req:inform(102, #{<<"set-cookie">> => <<"name=value">>}, Req0);
 		<<"twice">> ->
 			cowboy_req:inform(102, Headers, Req0),
 			cowboy_req:inform(102, Headers, Req0);
@@ -179,6 +192,9 @@ do(<<"reply3">>, Req0, Opts) ->
 		<<"error">> ->
 			ct_helper:ignore(cowboy_req, reply, 4),
 			cowboy_req:reply(200, ok, Req0);
+		<<"set_cookie">> ->
+			ct_helper:ignore(cowboy_req, reply, 4),
+			cowboy_req:reply(200, #{<<"set-cookie">> => <<"name=value">>}, Req0);
 		Status ->
 			cowboy_req:reply(binary_to_integer(Status),
 				#{<<"content-type">> => <<"text/plain">>}, Req0)
@@ -189,6 +205,9 @@ do(<<"reply4">>, Req0, Opts) ->
 		<<"error">> ->
 			ct_helper:ignore(erlang, iolist_size, 1),
 			cowboy_req:reply(200, #{}, ok, Req0);
+		<<"set_cookie">> ->
+			ct_helper:ignore(cowboy_req, reply, 4),
+			cowboy_req:reply(200, #{<<"set-cookie">> => <<"name=value">>}, <<"OK">>, Req0);
 		<<"204body">> ->
 			ct_helper:ignore(cowboy_req, do_reply_ensure_no_body, 4),
 			cowboy_req:reply(204, #{}, <<"OK">>, Req0);
@@ -240,6 +259,9 @@ do(<<"stream_reply3">>, Req0, Opts) ->
 		<<"error">> ->
 			ct_helper:ignore(cowboy_req, stream_reply, 3),
 			cowboy_req:stream_reply(200, ok, Req0);
+		<<"set_cookie">> ->
+			ct_helper:ignore(cowboy_req, stream_reply, 3),
+			cowboy_req:stream_reply(200, #{<<"set-cookie">> => <<"name=value">>}, Req0);
 		Status ->
 			cowboy_req:stream_reply(binary_to_integer(Status),
 				#{<<"content-type">> => <<"text/plain">>}, Req0)
@@ -395,6 +417,16 @@ do(<<"stream_trailers">>, Req0, Opts) ->
 				<<"grpc-status">> => <<"0">>
 			}, Req),
 			{ok, Req, Opts};
+		<<"set_cookie">> ->
+			ct_helper:ignore(cowboy_req, stream_trailers, 2),
+			Req = cowboy_req:stream_reply(200, #{
+				<<"trailer">> => <<"set-cookie">>
+			}, Req0),
+			cowboy_req:stream_body(<<"Hello world!">>, nofin, Req),
+			cowboy_req:stream_trailers(#{
+				<<"set-cookie">> => <<"name=value">>
+			}, Req),
+			{ok, Req, Opts};
 		_ ->
 			Req = cowboy_req:stream_reply(200, #{
 				<<"trailer">> => <<"grpc-status">>
diff --git a/test/req_SUITE.erl b/test/req_SUITE.erl
index 4bf2739..f407853 100644
--- a/test/req_SUITE.erl
+++ b/test/req_SUITE.erl
@@ -809,6 +809,8 @@ set_resp_header(Config) ->
 	doc("Response using set_resp_header."),
 	{200, Headers, <<"OK">>} = do_get("/resp/set_resp_header", Config),
 	true = lists:keymember(<<"content-type">>, 1, Headers),
+	%% The set-cookie header is special. set_resp_cookie must be used.
+	{500, _, _} = do_get("/resp/set_resp_header_cookie", Config),
 	ok.
 
 set_resp_headers(Config) ->
@@ -816,6 +818,8 @@ set_resp_headers(Config) ->
 	{200, Headers, <<"OK">>} = do_get("/resp/set_resp_headers", Config),
 	true = lists:keymember(<<"content-type">>, 1, Headers),
 	true = lists:keymember(<<"content-encoding">>, 1, Headers),
+	%% The set-cookie header is special. set_resp_cookie must be used.
+	{500, _, _} = do_get("/resp/set_resp_headers_cookie", Config),
 	ok.
 
 resp_header(Config) ->
@@ -893,6 +897,8 @@ inform3(Config) ->
 	{102, Headers, 200, _, _} = do_get_inform("/resp/inform3/102", Config),
 	{102, Headers, 200, _, _} = do_get_inform("/resp/inform3/binary", Config),
 	{500, _} = do_get_inform("/resp/inform3/error", Config),
+	%% The set-cookie header is special. set_resp_cookie must be used.
+	{500, _} = do_get_inform("/resp/inform3/set_cookie", Config),
 	{102, Headers, 200, _, _} = do_get_inform("/resp/inform3/twice", Config),
 	%% @todo How to test this properly? This isn't enough.
 	{200, _} = do_get_inform("/resp/inform3/after_reply", Config),
@@ -918,6 +924,8 @@ reply3(Config) ->
 	{404, Headers3, _} = do_get("/resp/reply3/404", Config),
 	true = lists:keymember(<<"content-type">>, 1, Headers3),
 	{500, _, _} = do_get("/resp/reply3/error", Config),
+	%% The set-cookie header is special. set_resp_cookie must be used.
+	{500, _, _} = do_get("/resp/reply3/set_cookie", Config),
 	ok.
 
 reply4(Config) ->
@@ -926,6 +934,8 @@ reply4(Config) ->
 	{201, _, <<"OK">>} = do_get("/resp/reply4/201", Config),
 	{404, _, <<"OK">>} = do_get("/resp/reply4/404", Config),
 	{500, _, _} = do_get("/resp/reply4/error", Config),
+	%% The set-cookie header is special. set_resp_cookie must be used.
+	{500, _, _} = do_get("/resp/reply4/set_cookie", Config),
 	ok.
 
 stream_reply2(Config) ->
@@ -975,6 +985,8 @@ stream_reply3(Config) ->
 	{404, Headers3, Body} = do_get("/resp/stream_reply3/404", Config),
 	true = lists:keymember(<<"content-type">>, 1, Headers3),
 	{500, _, _} = do_get("/resp/stream_reply3/error", Config),
+	%% The set-cookie header is special. set_resp_cookie must be used.
+	{500, _, _} = do_get("/resp/stream_reply3/set_cookie", Config),
 	ok.
 
 stream_body_fin0(Config) ->
@@ -1154,6 +1166,27 @@ stream_trailers_no_te(Config) ->
 	<<"Hello world!">> = do_decode(RespHeaders, RespBody),
 	gun:close(ConnPid).
 
+stream_trailers_set_cookie(Config) ->
+	doc("Trying to send set-cookie in trailers should result in a crash."),
+	ConnPid = gun_open(Config),
+	Ref = gun:get(ConnPid, "/resp/stream_trailers/set_cookie", [
+		{<<"accept-encoding">>, <<"gzip">>},
+		{<<"te">>, <<"trailers">>}
+	]),
+	{response, nofin, 200, _} = gun:await(ConnPid, Ref, infinity),
+	case config(protocol, Config) of
+		http ->
+			%% Trailers are not sent because of the stream error.
+			{ok, _Body} = gun:await_body(ConnPid, Ref, infinity),
+			{error, timeout} = gun:await_body(ConnPid, Ref, 1000),
+			ok;
+		http2 ->
+			{error, {stream_error, {stream_error, internal_error, _}}}
+				= gun:await_body(ConnPid, Ref, infinity),
+			ok
+	end,
+	gun:close(ConnPid).
+
 do_trailers(Path, Config) ->
 	ConnPid = gun_open(Config),
 	Ref = gun:get(ConnPid, Path, [