diff options
| -rw-r--r-- | doc/src/guide/migrating_from_2.4.asciidoc | 5 | ||||
| -rw-r--r-- | doc/src/manual/cowboy_stream.asciidoc | 3 | ||||
| -rw-r--r-- | src/cowboy_http.erl | 9 | ||||
| -rw-r--r-- | test/handlers/stream_handler_h.erl | 6 | ||||
| -rw-r--r-- | test/stream_handler_SUITE.erl | 48 |
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(), |