aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLoïc Hoguin <[email protected]>2018-09-12 16:16:29 +0200
committerLoïc Hoguin <[email protected]>2018-09-12 16:16:29 +0200
commitb56a5a1d60715ae115723c11c27ff7e032a3c4a5 (patch)
tree5997c621337049429f2197918319f9d6316565a1
parent26bc4afad430c81f987597f409822452a8348657 (diff)
downloadcowboy-b56a5a1d60715ae115723c11c27ff7e032a3c4a5.tar.gz
cowboy-b56a5a1d60715ae115723c11c27ff7e032a3c4a5.tar.bz2
cowboy-b56a5a1d60715ae115723c11c27ff7e032a3c4a5.zip
Do not send a 101 after a final response in switch_protocol
-rw-r--r--doc/src/guide/migrating_from_2.4.asciidoc5
-rw-r--r--doc/src/manual/cowboy_stream.asciidoc3
-rw-r--r--src/cowboy_http.erl9
-rw-r--r--test/handlers/stream_handler_h.erl6
-rw-r--r--test/stream_handler_SUITE.erl48
5 files changed, 68 insertions, 3 deletions
diff --git a/doc/src/guide/migrating_from_2.4.asciidoc b/doc/src/guide/migrating_from_2.4.asciidoc
index 4ffb5b9..edf7d5b 100644
--- a/doc/src/guide/migrating_from_2.4.asciidoc
+++ b/doc/src/guide/migrating_from_2.4.asciidoc
@@ -81,6 +81,11 @@ also been worked on.
* Improve the validation of HTTP/1.1 absolute-form requests.
+* When the `switch_protocol` is used after a response was
+ sent, Cowboy will no longer attempt to send the 101 informational
+ response for the protocol upgrade. This caused a crash of the
+ connection previously.
+
* Errors that occur when a callback returned by
`content_types_provided` does not exist have been improved.
diff --git a/doc/src/manual/cowboy_stream.asciidoc b/doc/src/manual/cowboy_stream.asciidoc
index 763cbd5..4e6e34a 100644
--- a/doc/src/manual/cowboy_stream.asciidoc
+++ b/doc/src/manual/cowboy_stream.asciidoc
@@ -205,6 +205,9 @@ Contains the headers that will be sent in the 101 response,
along with the module implementing the protocol we are
switching to and its initial state.
+Note that the 101 informational response will not be sent
+after a final response.
+
=== stop
Stop the stream.
diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl
index e1af0e9..71d33ce 100644
--- a/src/cowboy_http.erl
+++ b/src/cowboy_http.erl
@@ -1064,7 +1064,7 @@ commands(State0=#state{socket=Socket, transport=Transport}, StreamID,
end;
%% Protocol takeover.
commands(State0=#state{ref=Ref, parent=Parent, socket=Socket, transport=Transport,
- opts=Opts, children=Children}, StreamID,
+ out_state=OutState, opts=Opts, children=Children}, StreamID,
[{switch_protocol, Headers, Protocol, InitialState}|_Tail]) ->
%% @todo This should be the last stream running otherwise we need to wait before switching.
%% @todo If there's streams opened after this one, fail instead of 101.
@@ -1076,8 +1076,11 @@ commands(State0=#state{ref=Ref, parent=Parent, socket=Socket, transport=Transpor
%% @todo Handle cases where the request came with a body. We need
%% to process or skip the body before the upgrade can be completed.
Transport:setopts(Socket, [{active, false}]),
- %% Send a 101 response, then terminate the stream.
- #state{streams=Streams} = info(State, StreamID, {inform, 101, Headers}),
+ %% Send a 101 response if necessary, then terminate the stream.
+ #state{streams=Streams} = case OutState of
+ wait -> info(State, StreamID, {inform, 101, Headers});
+ _ -> State
+ end,
#stream{state=StreamState} = lists:keyfind(StreamID, #stream.id, Streams),
%% @todo We need to shutdown processes here first.
stream_call_terminate(StreamID, switch_protocol, StreamState, State),
diff --git a/test/handlers/stream_handler_h.erl b/test/handlers/stream_handler_h.erl
index 7aa3195..0f245c6 100644
--- a/test/handlers/stream_handler_h.erl
+++ b/test/handlers/stream_handler_h.erl
@@ -46,6 +46,12 @@ init_commands(_, _, State=#state{test=shutdown_timeout_on_stream_stop}) ->
init_commands(_, _, State=#state{test=shutdown_timeout_on_socket_close}) ->
Spawn = init_process(true, State),
[{headers, 200, #{}}, {spawn, Spawn, 2000}];
+init_commands(_, _, State=#state{test=switch_protocol_after_headers}) ->
+ [{headers, 200, #{}}, {switch_protocol, #{}, ?MODULE, State}];
+init_commands(_, _, State=#state{test=switch_protocol_after_headers_data}) ->
+ [{headers, 200, #{}}, {data, fin, <<"{}">>}, {switch_protocol, #{}, ?MODULE, State}];
+init_commands(_, _, State=#state{test=switch_protocol_after_response}) ->
+ [{response, 200, #{}, <<"{}">>}, {switch_protocol, #{}, ?MODULE, State}];
init_commands(_, _, State=#state{test=terminate_on_switch_protocol}) ->
[{switch_protocol, #{}, ?MODULE, State}];
init_commands(_, _, #state{test=terminate_on_stop}) ->
diff --git a/test/stream_handler_SUITE.erl b/test/stream_handler_SUITE.erl
index 231e97a..9003e23 100644
--- a/test/stream_handler_SUITE.erl
+++ b/test/stream_handler_SUITE.erl
@@ -325,6 +325,54 @@ shutdown_timeout_on_socket_close(Config) ->
receive {'DOWN', MRef, process, Spawn, killed} -> ok after 1000 -> error(timeout) end,
ok.
+switch_protocol_after_headers(Config) ->
+ case config(protocol, Config) of
+ http -> do_switch_protocol_after_response(
+ <<"switch_protocol_after_headers">>, Config);
+ http2 -> doc("The switch_protocol command is not currently supported for HTTP/2.")
+ end.
+
+switch_protocol_after_headers_data(Config) ->
+ case config(protocol, Config) of
+ http -> do_switch_protocol_after_response(
+ <<"switch_protocol_after_headers_data">>, Config);
+ http2 -> doc("The switch_protocol command is not currently supported for HTTP/2.")
+ end.
+
+switch_protocol_after_response(Config) ->
+ case config(protocol, Config) of
+ http -> do_switch_protocol_after_response(
+ <<"switch_protocol_after_response">>, Config);
+ http2 -> doc("The switch_protocol command is not currently supported for HTTP/2.")
+ end.
+
+do_switch_protocol_after_response(TestCase, Config) ->
+ doc("The 101 informational response must not be sent when a response "
+ "has already been sent before the switch_protocol is returned."),
+ Self = self(),
+ ConnPid = gun_open(Config),
+ Ref = gun:get(ConnPid, "/long_polling", [
+ {<<"accept-encoding">>, <<"gzip">>},
+ {<<"x-test-case">>, TestCase},
+ {<<"x-test-pid">>, pid_to_list(Self)}
+ ]),
+ %% Confirm init/3 is called and receive the response.
+ Pid = receive {Self, P, init, _, _, _} -> P after 1000 -> error(timeout) end,
+ {response, nofin, 200, _} = gun:await(ConnPid, Ref),
+ case TestCase of
+ <<"switch_protocol_after_headers">> ->
+ ok;
+ _ ->
+ {ok, <<"{}">>} = gun:await_body(ConnPid, Ref),
+ ok
+ end,
+ {error, _} = gun:await(ConnPid, Ref),
+ %% Confirm terminate/3 is called.
+ receive {Self, Pid, terminate, _, _, _} -> ok after 1000 -> error(timeout) end,
+ %% Confirm takeover/7 is called.
+ receive {Self, Pid, takeover, _, _, _, _, _, _, _} -> ok after 1000 -> error(timeout) end,
+ ok.
+
terminate_on_socket_close(Config) ->
doc("Confirm terminate/3 is called when the socket gets closed brutally."),
Self = self(),