From 69f19635df64ea48af3120c9685f7ad51b338f8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20S=C3=B6derqvist?= Date: Mon, 19 Oct 2020 13:12:04 +0200 Subject: Disconnect when setopts returns an error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is a race condition: After receiving data, before setting the socket back to {active, once}, the server may have closed the socket or an error may have occurred. Since the socket was in passive mode, no message is received. Handling the return value of setopts solves the issue. Amended by Loïc Hoguin to handle the return value from the active/1 function call. --- src/gun.erl | 109 ++++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 61 insertions(+), 48 deletions(-) diff --git a/src/gun.erl b/src/gun.erl index 73779c2..177d395 100644 --- a/src/gun.erl +++ b/src/gun.erl @@ -1242,10 +1242,14 @@ connected(internal, {connected, Socket, NewProtocol}, {Protocol, ProtoOpts} = gun_protocols:handler_and_opts(NewProtocol, Opts), {StateName, ProtoState} = Protocol:init(Owner, Socket, Transport, ProtoOpts), Owner ! {gun_up, self(), Protocol:name()}, - State = active(State0#state{socket=Socket, protocol=Protocol, protocol_state=ProtoState}), - case Protocol:has_keepalive() of - true -> {next_state, StateName, keepalive_timeout(State)}; - false -> {next_state, StateName, State} + case active(State0#state{socket=Socket, protocol=Protocol, protocol_state=ProtoState}) of + {ok, State} -> + case Protocol:has_keepalive() of + true -> {next_state, StateName, keepalive_timeout(State)}; + false -> {next_state, StateName, State} + end; + Disconnect -> + Disconnect end; %% Public HTTP interface. %% @@ -1380,14 +1384,7 @@ handle_common_connected_no_input(info, {OK, Socket, Data}, _, protocol=Protocol, protocol_state=ProtoState, event_handler=EvHandler, event_handler_state=EvHandlerState0}) -> {Commands, EvHandlerState} = Protocol:handle(Data, ProtoState, EvHandler, EvHandlerState0), - case commands(Commands, State0#state{event_handler_state=EvHandlerState}) of - {keep_state, State} -> - {keep_state, active(State)}; - {next_state, closing, State, Actions} -> - {next_state, closing, active(State), Actions}; - Res -> - Res - end; + maybe_active(commands(Commands, State0#state{event_handler_state=EvHandlerState})); handle_common_connected_no_input(info, {Closed, Socket}, _, State=#state{socket=Socket, messages={_, Closed, _}}) -> disconnect(State, closed); @@ -1403,28 +1400,14 @@ handle_common_connected_no_input(info, {Commands, EvHandlerState} = Protocol:handle_continue( dereference_stream_ref(StreamRef, State0), Msg, ProtoState, EvHandler, EvHandlerState0), - case commands(Commands, State0#state{event_handler_state=EvHandlerState}) of - {keep_state, State} -> - {keep_state, active(State)}; - {next_state, closing, State, Actions} -> - {next_state, closing, active(State), Actions}; - Res -> - Res - end; + maybe_active(commands(Commands, State0#state{event_handler_state=EvHandlerState})); handle_common_connected_no_input(info, {handle_continue, StreamRef, Msg}, _, State0=#state{protocol=Protocol, protocol_state=ProtoState, event_handler=EvHandler, event_handler_state=EvHandlerState0}) -> {Commands, EvHandlerState} = Protocol:handle_continue( dereference_stream_ref(StreamRef, State0), Msg, ProtoState, EvHandler, EvHandlerState0), - case commands(Commands, State0#state{event_handler_state=EvHandlerState}) of - {keep_state, State} -> - {keep_state, active(State)}; - {next_state, closing, State, Actions} -> - {next_state, closing, active(State), Actions}; - Res -> - Res - end; + maybe_active(commands(Commands, State0#state{event_handler_state=EvHandlerState})); %% Timeouts. %% @todo HTTP/2 requires more timeouts than just the keepalive timeout. %% We should have a timeout function in protocols that deal with @@ -1438,12 +1421,7 @@ handle_common_connected_no_input(info, keepalive, _, handle_common_connected_no_input(cast, {update_flow, ReplyTo, StreamRef, Flow}, _, State0=#state{protocol=Protocol, protocol_state=ProtoState}) -> Commands = Protocol:update_flow(ProtoState, ReplyTo, StreamRef, Flow), - case commands(Commands, State0) of - {keep_state, State} -> - {keep_state, active(State)}; - Res -> - Res - end; + maybe_active(commands(Commands, State0)); handle_common_connected_no_input(cast, {cancel, ReplyTo, StreamRef}, _, State=#state{protocol=Protocol, protocol_state=ProtoState, event_handler=EvHandler, event_handler_state=EvHandlerState0}) -> @@ -1504,6 +1482,35 @@ handle_common_connected_no_input({call, From}, {stream_info, StreamRef}, _, handle_common_connected_no_input(Type, Event, StateName, State) -> handle_common(Type, Event, StateName, State). +maybe_active({keep_state, State0}) -> + case active(State0) of + {ok, State} -> + {keep_state, State}; + Disconnect -> + Disconnect + end; +maybe_active({next_state, closing, State0, Actions}) -> + case active(State0) of + {ok, State} -> + {next_state, closing, State, Actions}; + Disconnect -> + Disconnect + end; +maybe_active(Other) -> + Other. + +active(State=#state{active=false}) -> + {ok, State}; +active(State=#state{socket=Socket, transport=Transport}) -> + case Transport:setopts(Socket, [{active, once}]) of + ok -> + {ok, State}; + {error, closed} -> + disconnect(State, closed); + Error = {error, _} -> + disconnect(State, Error) + end. + tunnel_info_from_intermediaries(State, Tail) -> case Tail of %% If the next endpoint is an intermediary take its infos. @@ -1679,11 +1686,17 @@ commands([{origin, Scheme, Host, Port, Type}|Tail], commands(Tail, State#state{origin_scheme=Scheme, origin_host=Host, origin_port=Port, intermediaries=[Info|Intermediaries], event_handler_state=EvHandlerState}); -commands([{switch_transport, Transport, Socket}|Tail], State=#state{ +commands([{switch_transport, Transport, Socket}|Tail], State0=#state{ protocol=Protocol, protocol_state=ProtoState0}) -> ProtoState = Protocol:switch_transport(Transport, Socket, ProtoState0), - commands(Tail, active(State#state{socket=Socket, transport=Transport, - messages=Transport:messages(), protocol_state=ProtoState})); + State1 = State0#state{socket=Socket, transport=Transport, + messages=Transport:messages(), protocol_state=ProtoState}, + case active(State1) of + {ok, State} -> + commands(Tail, State); + Disconnect -> + Disconnect + end; commands([{switch_protocol, NewProtocol, ReplyTo}], State0=#state{ opts=Opts, socket=Socket, transport=Transport, event_handler=EvHandler, event_handler_state=EvHandlerState0}) -> @@ -1702,11 +1715,17 @@ commands([{switch_protocol, NewProtocol, ReplyTo}], State0=#state{ EvHandlerState = EvHandler:protocol_changed(ProtocolChangedEvent, EvHandlerState0), %% We cancel the existing keepalive and, depending on the protocol, %% we enable keepalive again, effectively resetting the timer. - State = keepalive_cancel(active(State0#state{protocol=Protocol, protocol_state=ProtoState, - event_handler_state=EvHandlerState})), - case Protocol:has_keepalive() of - true -> {next_state, StateName, keepalive_timeout(State)}; - false -> {next_state, StateName, State} + State1 = State0#state{protocol=Protocol, protocol_state=ProtoState, + event_handler_state=EvHandlerState}, + case active(State1) of + {ok, State2} -> + State = keepalive_cancel(State2), + case Protocol:has_keepalive() of + true -> {next_state, StateName, keepalive_timeout(State)}; + false -> {next_state, StateName, State} + end; + Disconnect -> + Disconnect end; %% Perform a TLS handshake. commands([TLSHandshake={tls_handshake, _, _, _}], State) -> @@ -1750,12 +1769,6 @@ disconnect_flush(State=#state{socket=Socket, messages={OK, Closed, Error}}) -> ok end. -active(State=#state{active=false}) -> - State; -active(State=#state{socket=Socket, transport=Transport}) -> - Transport:setopts(Socket, [{active, once}]), - State. - status(State=#state{status={up, OwnerRef}}, NewStatus) -> demonitor(OwnerRef, [flush]), State#state{status=NewStatus}; -- cgit v1.2.3