Browse Source

Fix HTTP/1.1 bug when a flow command is returned after fin

This resulted in a badarith error due to the current flow being
set to infinity when the body has been fully read. A test case
has been added reproducing the issue.
Loïc Hoguin 5 years ago
parent
commit
8fc3da2fc3
3 changed files with 20 additions and 0 deletions
  1. 3 0
      src/cowboy_http.erl
  2. 4 0
      test/handlers/stream_handler_h.erl
  3. 13 0
      test/stream_handler_SUITE.erl

+ 3 - 0
src/cowboy_http.erl

@@ -970,6 +970,9 @@ commands(State=#state{out_streamid=Current, streams=Streams0}, StreamID, Command
 	Streams = lists:keyreplace(StreamID, #stream.id, Streams0,
 	Streams = lists:keyreplace(StreamID, #stream.id, Streams0,
 		Stream#stream{queue=Queue ++ Commands}),
 		Stream#stream{queue=Queue ++ Commands}),
 	State#state{streams=Streams};
 	State#state{streams=Streams};
+%% When we have finished reading the request body, do nothing.
+commands(State=#state{flow=infinity}, StreamID, [{flow, _}|Tail]) ->
+	commands(State, StreamID, Tail);
 %% Read the request body.
 %% Read the request body.
 commands(State0=#state{flow=Flow0}, StreamID, [{flow, Size}|Tail]) ->
 commands(State0=#state{flow=Flow0}, StreamID, [{flow, Size}|Tail]) ->
 	%% We must read *at least* Size of data otherwise functions
 	%% We must read *at least* Size of data otherwise functions

+ 4 - 0
test/handlers/stream_handler_h.erl

@@ -34,6 +34,8 @@ init_commands(_, _, #state{test=crash_in_terminate}) ->
 	[{response, 200, #{<<"content-length">> => <<"12">>}, <<"Hello world!">>}, stop];
 	[{response, 200, #{<<"content-length">> => <<"12">>}, <<"Hello world!">>}, stop];
 init_commands(_, _, #state{test=crash_in_early_error}) ->
 init_commands(_, _, #state{test=crash_in_early_error}) ->
 	error(crash);
 	error(crash);
+init_commands(_, _, #state{test=flow_after_body_fully_read}) ->
+	[];
 init_commands(_, _, #state{test=set_options_ignore_unknown}) ->
 init_commands(_, _, #state{test=set_options_ignore_unknown}) ->
 	[
 	[
 		{set_options, #{unknown_options => true}},
 		{set_options, #{unknown_options => true}},
@@ -81,6 +83,8 @@ init_process(TrapExit, #state{pid=Pid}) ->
 
 
 data(_, _, _, #state{test=crash_in_data}) ->
 data(_, _, _, #state{test=crash_in_data}) ->
 	error(crash);
 	error(crash);
+data(_, fin, <<"Hello world!">>, State=#state{test=flow_after_body_fully_read}) ->
+	{[{flow, 12}, {response, 200, #{}, <<"{}">>}], State};
 data(StreamID, IsFin, Data, State=#state{pid=Pid}) ->
 data(StreamID, IsFin, Data, State=#state{pid=Pid}) ->
 	Pid ! {Pid, self(), data, StreamID, IsFin, Data, State},
 	Pid ! {Pid, self(), data, StreamID, IsFin, Data, State},
 	{[], State}.
 	{[], State}.

+ 13 - 0
test/stream_handler_SUITE.erl

@@ -282,6 +282,19 @@ early_error_stream_error_reason(Config) ->
 	{response, fin, Status, _} = gun:await(ConnPid, Ref),
 	{response, fin, Status, _} = gun:await(ConnPid, Ref),
 	ok.
 	ok.
 
 
+flow_after_body_fully_read(Config) ->
+	doc("A flow command may be returned even after the body was read fully."),
+	Self = self(),
+	ConnPid = gun_open(Config),
+	Ref = gun:post(ConnPid, "/long_polling", [
+		{<<"x-test-case">>, <<"flow_after_body_fully_read">>},
+		{<<"x-test-pid">>, pid_to_list(Self)}
+	], <<"Hello world!">>),
+	%% Receive a 200 response, sent after the second flow command,
+	%% confirming that the flow command was accepted.
+	{response, _, 200, _} = gun:await(ConnPid, Ref),
+	ok.
+
 set_options_ignore_unknown(Config) ->
 set_options_ignore_unknown(Config) ->
 	doc("Confirm that unknown options are ignored when using the set_options commands."),
 	doc("Confirm that unknown options are ignored when using the set_options commands."),
 	Self = self(),
 	Self = self(),