From 39b2816255503910dc23e2fdf703ee63bbc8953e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Wed, 20 May 2020 13:41:05 +0200 Subject: 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. --- src/cowboy_http.erl | 3 ++- src/cowboy_req.erl | 19 +++++++++++++++++-- test/handlers/resp_h.erl | 15 ++++++++++++++- test/rfc7230_SUITE.erl | 28 ++++++++++++++-------------- 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl index 2833e79..89ba9d8 100644 --- a/src/cowboy_http.erl +++ b/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), diff --git a/src/cowboy_req.erl b/src/cowboy_req.erl index a2eadfc..90c5a3a 100644 --- a/src/cowboy_req.erl +++ b/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). diff --git a/test/handlers/resp_h.erl b/test/handlers/resp_h.erl index d43152e..8031d0e 100644 --- a/test/handlers/resp_h.erl +++ b/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), diff --git a/test/rfc7230_SUITE.erl b/test/rfc7230_SUITE.erl index e110441..9846a0f 100644 --- a/test/rfc7230_SUITE.erl +++ b/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)), -- cgit v1.2.3