Browse Source

204 and 304 responses must not include a body

When calling cowboy_req:reply/4 with a body a crash will occur
resulting in a 500 response. When calling cowboy_req:stream_reply/2,3
and then attempting to send a body a crash will occur.
Loïc Hoguin 5 years ago
parent
commit
39b2816255
4 changed files with 47 additions and 18 deletions
  1. 2 1
      src/cowboy_http.erl
  2. 17 2
      src/cowboy_req.erl
  3. 14 1
      test/handlers/resp_h.erl
  4. 14 14
      test/rfc7230_SUITE.erl

+ 2 - 1
src/cowboy_http.erl

@@ -1029,8 +1029,9 @@ commands(State0=#state{socket=Socket, transport=Transport, out_state=wait, strea
 	#stream{version=Version} = lists:keyfind(StreamID, #stream.id, Streams),
 	{State1, Headers} = connection(State0, Headers0, StreamID, Version),
 	State = State1#state{out_state=done},
-	%% @todo Ensure content-length is set.
+	%% @todo Ensure content-length is set. 204 must never have content-length set.
 	Response = cow_http:response(StatusCode, 'HTTP/1.1', headers_to_list(Headers)),
+	%% @todo 204 and 304 responses must not include a response body. (RFC7230 3.3.1, RFC7230 3.3.2)
 	case Body of
 		{sendfile, _, _, _} ->
 			Transport:send(Socket, Response),

+ 17 - 2
src/cowboy_req.erl

@@ -806,12 +806,16 @@ reply(Status, Headers, SendFile = {sendfile, _, Len, _}, Req)
 	}, SendFile, Req);
 %% 204 responses must not include content-length. 304 responses may
 %% but only when set explicitly. (RFC7230 3.3.1, RFC7230 3.3.2)
+%% Neither status code must include a response body. (RFC7230 3.3)
 reply(Status, Headers, Body, Req)
 		when Status =:= 204; Status =:= 304 ->
+	0 = iolist_size(Body),
 	do_reply(Status, Headers, Body, Req);
-reply(Status= <<"204",_/bits>>, Headers, Body, Req) ->
+reply(Status = <<"204",_/bits>>, Headers, Body, Req) ->
+	0 = iolist_size(Body),
 	do_reply(Status, Headers, Body, Req);
-reply(Status= <<"304",_/bits>>, Headers, Body, Req) ->
+reply(Status = <<"304",_/bits>>, Headers, Body, Req) ->
+	0 = iolist_size(Body),
 	do_reply(Status, Headers, Body, Req);
 reply(Status, Headers, Body, Req)
 		when is_integer(Status); is_binary(Status) ->
@@ -840,6 +844,17 @@ stream_reply(Status, Req) ->
 	-> Req when Req::req().
 stream_reply(_, _, #{has_sent_resp := _}) ->
 	error(function_clause);
+%% 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)
+stream_reply(Status = 204, Headers=#{}, Req) ->
+	reply(Status, Headers, <<>>, Req);
+stream_reply(Status = <<"204",_/bits>>, Headers=#{}, Req) ->
+	reply(Status, Headers, <<>>, Req);
+stream_reply(Status = 304, Headers=#{}, Req) ->
+	reply(Status, Headers, <<>>, Req);
+stream_reply(Status = <<"304",_/bits>>, Headers=#{}, Req) ->
+	reply(Status, Headers, <<>>, Req);
 stream_reply(Status, Headers=#{}, Req) when is_integer(Status); is_binary(Status) ->
 	cast({headers, Status, response_headers(Headers, Req)}, Req),
 	done_replying(Req, headers).

+ 14 - 1
test/handlers/resp_h.erl

@@ -181,6 +181,12 @@ do(<<"reply4">>, Req0, Opts) ->
 		<<"error">> ->
 			ct_helper:ignore(erlang, iolist_size, 1),
 			cowboy_req:reply(200, #{}, ok, Req0);
+		<<"204body">> ->
+			ct_helper:ignore(cowboy_req, reply, 4),
+			cowboy_req:reply(204, #{}, <<"OK">>, Req0);
+		<<"304body">> ->
+			ct_helper:ignore(cowboy_req, reply, 4),
+			cowboy_req:reply(304, #{}, <<"OK">>, Req0);
 		Status ->
 			cowboy_req:reply(binary_to_integer(Status), #{}, <<"OK">>, Req0)
 	end,
@@ -199,8 +205,15 @@ do(<<"stream_reply2">>, Req0, Opts) ->
 		<<"204">> ->
 			Req = cowboy_req:stream_reply(204, Req0),
 			{ok, Req, Opts};
-		<<"304">> ->
+		<<"204body">> ->
+			ct_helper:ignore(cowboy_req, stream_body, 3),
+			Req = cowboy_req:stream_reply(204, Req0),
+			stream_body(Req),
+			{ok, Req, Opts};
+		<<"304body">> ->
+			ct_helper:ignore(cowboy_req, stream_body, 3),
 			Req = cowboy_req:stream_reply(304, Req0),
+			stream_body(Req),
 			{ok, Req, Opts};
 		Status ->
 			Req = cowboy_req:stream_reply(binary_to_integer(Status), Req0),

+ 14 - 14
test/rfc7230_SUITE.erl

@@ -1886,22 +1886,22 @@ no_body_in_head_response(Config) ->
 %1xx responses never include a message body. (RFC7230 3.3)
 
 no_body_in_204_response(Config) ->
-	doc("204 responses never include a message body. (RFC7230 3.3)"),
+	doc("204 responses never include a message body. Cowboy produces "
+		"a 500 error response when attempting to do so. (RFC7230 3.3)"),
 	Client = raw_open(Config),
 	ok = raw_send(Client, [
-		"GET /resp/reply2/204 HTTP/1.1\r\n"
+		"GET /resp/reply4/204body HTTP/1.1\r\n"
 		"Host: localhost\r\n"
 		"\r\n"]),
-	{_, 204, _, Rest} = cow_http:parse_status_line(raw_recv_head(Client)),
-	{_, <<>>} = cow_http:parse_headers(Rest),
-	{error, timeout} = raw_recv(Client, 1, 1000),
+	{_, 500, _, _} = cow_http:parse_status_line(raw_recv_head(Client)),
 	ok.
 
 no_body_in_204_response_stream(Config) ->
-	doc("204 responses never include a message body. (RFC7230 3.3)"),
+	doc("204 responses never include a message body. Attempting to "
+		"stream the body produces a crash on the server-side. (RFC7230 3.3)"),
 	Client = raw_open(Config),
 	ok = raw_send(Client, [
-		"GET /resp/stream_reply2/204 HTTP/1.1\r\n"
+		"GET /resp/stream_reply2/204body HTTP/1.1\r\n"
 		"Host: localhost\r\n"
 		"\r\n"]),
 	{_, 204, _, Rest} = cow_http:parse_status_line(raw_recv_head(Client)),
@@ -1910,22 +1910,22 @@ no_body_in_204_response_stream(Config) ->
 	ok.
 
 no_body_in_304_response(Config) ->
-	doc("304 responses never include a message body. (RFC7230 3.3)"),
+	doc("304 responses never include a message body. Cowboy produces "
+		"a 500 error response when attempting to do so. (RFC7230 3.3)"),
 	Client = raw_open(Config),
 	ok = raw_send(Client, [
-		"GET /resp/reply2/304 HTTP/1.1\r\n"
+		"GET /resp/reply4/304body HTTP/1.1\r\n"
 		"Host: localhost\r\n"
 		"\r\n"]),
-	{_, 304, _, Rest} = cow_http:parse_status_line(raw_recv_head(Client)),
-	{_, <<>>} = cow_http:parse_headers(Rest),
-	{error, timeout} = raw_recv(Client, 1, 1000),
+	{_, 500, _, _} = cow_http:parse_status_line(raw_recv_head(Client)),
 	ok.
 
 no_body_in_304_response_stream(Config) ->
-	doc("304 responses never include a message body. (RFC7230 3.3)"),
+	doc("304 responses never include a message body. Attempting to "
+		"stream the body produces a crash on the server-side. (RFC7230 3.3)"),
 	Client = raw_open(Config),
 	ok = raw_send(Client, [
-		"GET /resp/stream_reply2/304 HTTP/1.1\r\n"
+		"GET /resp/stream_reply2/304body HTTP/1.1\r\n"
 		"Host: localhost\r\n"
 		"\r\n"]),
 	{_, 304, _, Rest} = cow_http:parse_status_line(raw_recv_head(Client)),