Fix two edge cases for cowboy_req:stream_body

Sending data of size 0 with the fin flag set resulted in nothing
being sent to the client and still considering the response to
be finished for HTTP/1.1.

For both HTTP/1.1 and HTTP/2, the final chunk of body that is
sent automatically by Cowboy at the end of a response that the
user did not properly terminate was not passing through stream
handlers. This resulted in issues like compression being incorrect.

Some tests still fail under 20.1.3. They are due to recent zlib
changes and should be fixed in a future patch release. Unfortunately
it does not seem to be any 20.1 version that is safe to use for
Cowboy, although some will work better than others.

4 files changed, 41 insertions, 14 deletions

diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl
index 281e815..c681154 100644
--- a/src/cowboy_http.erl
+++ b/src/cowboy_http.erl
@@ -871,7 +871,14 @@ commands(State0=#state{socket=Socket, transport=Transport, streams=Streams}, Str
 	%% data frame, as that would break the protocol.
 	Size = iolist_size(Data),
 	case Size of
-		0 -> ok;
+		0 ->
+			%% We send the last chunk only if version is HTTP/1.1 and IsFin=fin.
+			case lists:keyfind(StreamID, #stream.id, Streams) of
+				#stream{version='HTTP/1.1'} when IsFin =:= fin ->
+					Transport:send(Socket, <<"0\r\n\r\n">>);
+				_ ->
+					ok
+			end;
 		_ ->
 			%% @todo We need to kill the stream if it tries to send data before headers.
 			%% @todo Same as above.
@@ -961,8 +968,7 @@ stream_reset(State, StreamID, StreamError={internal_error, _, _}) ->
 %	stream_terminate(State#state{out_state=done}, StreamID, StreamError).
 	stream_terminate(State, StreamID, StreamError).
 
-stream_terminate(State0=#state{socket=Socket, transport=Transport,
-		out_streamid=OutStreamID, out_state=OutState,
+stream_terminate(State0=#state{out_streamid=OutStreamID, out_state=OutState,
 		streams=Streams0, children=Children0}, StreamID, Reason) ->
 	#stream{version=Version} = lists:keyfind(StreamID, #stream.id, Streams0),
 	State1 = #state{streams=Streams1} = case OutState of
@@ -971,8 +977,7 @@ stream_terminate(State0=#state{socket=Socket, transport=Transport,
 		wait ->
 			info(State0, StreamID, {response, 204, #{}, <<>>});
 		chunked when Version =:= 'HTTP/1.1' ->
-			_ = Transport:send(Socket, <<"0\r\n\r\n">>),
-			State0;
+			info(State0, StreamID, {data, fin, <<>>});
 		_ -> %% done or Version =:= 'HTTP/1.0'
 			State0
 	end,
diff --git a/src/cowboy_http2.erl b/src/cowboy_http2.erl
index 21f0aeb..9e81957 100644
--- a/src/cowboy_http2.erl
+++ b/src/cowboy_http2.erl
@@ -830,8 +830,7 @@ stream_linger(State=#state{lingering_streams=Lingering0}, StreamID) ->
 	Lingering = [StreamID|lists:sublist(Lingering0, 100 - 1)],
 	State#state{lingering_streams=Lingering}.
 
-stream_terminate(State0=#state{socket=Socket, transport=Transport,
-		streams=Streams0, children=Children0}, StreamID, Reason) ->
+stream_terminate(State0=#state{streams=Streams0, children=Children0}, StreamID, Reason) ->
 	case lists:keytake(StreamID, #stream.id, Streams0) of
 		%% When the stream terminates normally (without sending RST_STREAM)
 		%% and no response was sent, we need to send a proper response back to the client.
@@ -843,10 +842,11 @@ stream_terminate(State0=#state{socket=Socket, transport=Transport,
 			Children = cowboy_children:shutdown(Children0, StreamID),
 			State#state{streams=Streams, children=Children};
 		%% When a response was sent but not terminated, we need to close the stream.
-		{value, Stream=#stream{state=StreamState, local=nofin, local_buffer_size=0}, Streams}
+		{value, Stream=#stream{local=nofin, local_buffer_size=0}, Streams}
 				when Reason =:= normal ->
-			Transport:send(Socket, cow_http2:data(StreamID, fin, <<>>)),
-			State = maybe_skip_body(State0, Stream, Reason),
+			State1 = #state{streams=Streams1} = info(State0, StreamID, {data, fin, <<>>}),
+			State = maybe_skip_body(State1, Stream, Reason),
+			#stream{state=StreamState} = lists:keyfind(StreamID, #stream.id, Streams1),
 			stream_call_terminate(StreamID, Reason, StreamState),
 			Children = cowboy_children:shutdown(Children0, StreamID),
 			State#state{streams=Streams, children=Children};
diff --git a/test/handlers/resp_h.erl b/test/handlers/resp_h.erl
index 94c7f60..add90ad 100644
--- a/test/handlers/resp_h.erl
+++ b/test/handlers/resp_h.erl
@@ -188,10 +188,22 @@ do(<<"stream_reply3">>, Req0, Opts) ->
 	end,
 	stream_body(Req),
 	{ok, Req, Opts};
-do(<<"stream_body">>, Req, Opts) ->
-	%% Call stream_body without initiating streaming.
-	cowboy_req:stream_body(<<0:800000>>, fin, Req),
-	{ok, Req, Opts};
+do(<<"stream_body">>, Req0, Opts) ->
+	case cowboy_req:binding(arg, Req0) of
+		<<"fin0">> ->
+			Req = cowboy_req:stream_reply(200, Req0),
+			cowboy_req:stream_body(<<"Hello world!">>, nofin, Req),
+			cowboy_req:stream_body(<<>>, fin, Req),
+			{ok, Req, Opts};
+		<<"nofin">> ->
+			Req = cowboy_req:stream_reply(200, Req0),
+			cowboy_req:stream_body(<<"Hello world!">>, nofin, Req),
+			{ok, Req, Opts};
+		_ ->
+			%% Call stream_body without initiating streaming.
+			cowboy_req:stream_body(<<0:800000>>, fin, Req0),
+			{ok, Req0, Opts}
+	end;
 do(<<"push">>, Req, Opts) ->
 	case cowboy_req:binding(arg, Req) of
 		<<"method">> ->
diff --git a/test/req_SUITE.erl b/test/req_SUITE.erl
index 107cdd8..862ee53 100644
--- a/test/req_SUITE.erl
+++ b/test/req_SUITE.erl
@@ -827,6 +827,16 @@ stream_reply3(Config) ->
 	{500, _, _} = do_get("/resp/stream_reply3/error", Config),
 	ok.
 
+stream_body_fin0(Config) ->
+	doc("Streamed body with last chunk of size 0."),
+	{200, _, <<"Hello world!">>} = do_get("/resp/stream_body/fin0", Config),
+	ok.
+
+stream_body_nofin(Config) ->
+	doc("Unfinished streamed body."),
+	{200, _, <<"Hello world!">>} = do_get("/resp/stream_body/nofin", Config),
+	ok.
+
 %% @todo Crash when calling stream_body after the fin flag has been set.
 %% @todo Crash when calling stream_body after calling reply.
 %% @todo Crash when calling stream_body before calling stream_reply.