From 2f4046c13213ed2cd7d51d6422e74170bf22d2fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Mon, 5 Oct 2020 16:28:10 +0200 Subject: Improve some 'todo' return values and arguments While most of this functionality isn't implemented this is not a reason to let them return invalid values. --- src/gun_http2.erl | 45 ++++++++++++++++++++++---------------- src/gun_tunnel.erl | 63 +++++++++++++++++++++++++++++------------------------- 2 files changed, 61 insertions(+), 47 deletions(-) diff --git a/src/gun_http2.erl b/src/gun_http2.erl index 9dfc67c..7677076 100644 --- a/src/gun_http2.erl +++ b/src/gun_http2.erl @@ -338,21 +338,30 @@ data_frame(State, StreamID, IsFin, Data, EvHandler, EvHandlerState0) -> Stream=#stream{tunnel=#tunnel{protocol=Proto, protocol_state=ProtoState0}} -> % %% @todo What about IsFin? {Commands, EvHandlerState} = Proto:handle(Data, ProtoState0, EvHandler, EvHandlerState0), - {tunnel_commands(Commands, Stream, State), EvHandlerState} + tunnel_commands(Commands, Stream, State, EvHandler, EvHandlerState) end. -tunnel_commands(Command, Stream, State) when not is_list(Command) -> - tunnel_commands([Command], Stream, State); -tunnel_commands([], Stream, State) -> - store_stream(State, Stream); -tunnel_commands([{send, IsFin, Data}|Tail], Stream=#stream{id=StreamID}, State0) -> - %% @todo EvHandler EvHandlerState - {State, _EvHandlerState} = maybe_send_data(State0, StreamID, IsFin, Data, todo, todo), - tunnel_commands(Tail, Stream, State); -tunnel_commands([{state, ProtoState}|Tail], Stream=#stream{tunnel=Tunnel}, State) -> - tunnel_commands(Tail, Stream#stream{tunnel=Tunnel#tunnel{protocol_state=ProtoState}}, State); -tunnel_commands([SetCookie={set_cookie, _, _, _, _}|Tail], Stream, State=#http2_state{commands_queue=Queue}) -> - tunnel_commands(Tail, Stream, State#http2_state{commands_queue=[SetCookie|Queue]}). +tunnel_commands(Command, Stream, State, EvHandler, EvHandlerState) + when not is_list(Command) -> + tunnel_commands([Command], Stream, State, EvHandler, EvHandlerState); +tunnel_commands([], Stream, State, _EvHandler, EvHandlerState) -> + {store_stream(State, Stream), EvHandlerState}; +tunnel_commands([{send, IsFin, Data}|Tail], Stream=#stream{id=StreamID}, + State0, EvHandler, EvHandlerState0) -> + {State, EvHandlerState} = maybe_send_data(State0, StreamID, + IsFin, Data, EvHandler, EvHandlerState0), + tunnel_commands(Tail, Stream, State, EvHandler, EvHandlerState); +tunnel_commands([{state, ProtoState}|Tail], Stream=#stream{tunnel=Tunnel}, + State, EvHandler, EvHandlerState) -> + tunnel_commands(Tail, Stream#stream{tunnel=Tunnel#tunnel{protocol_state=ProtoState}}, + State, EvHandler, EvHandlerState); +tunnel_commands([SetCookie={set_cookie, _, _, _, _}|Tail], Stream, + State=#http2_state{commands_queue=Queue}, EvHandler, EvHandlerState) -> + tunnel_commands(Tail, Stream, State#http2_state{commands_queue=[SetCookie|Queue]}, + EvHandler, EvHandlerState); +tunnel_commands([{error, _Reason}|_], #stream{id=StreamID}, + State, _EvHandler, EvHandlerState) -> + {delete_stream(State, StreamID), EvHandlerState}. continue_stream_ref(#http2_state{socket=#{handle_continue_stream_ref := ContinueStreamRef}}, StreamRef) -> case ContinueStreamRef of @@ -599,17 +608,17 @@ ignored_frame(State=#http2_state{http2_machine=HTTP2Machine0}) -> end. %% We always pass handle_continue messages to the tunnel. -handle_continue(ContinueStreamRef, Msg, State, EvHandler, EvHandlerState0) -> +handle_continue(ContinueStreamRef, Msg, State0, EvHandler, EvHandlerState0) -> StreamRef = case ContinueStreamRef of [SR|_] -> SR; _ -> ContinueStreamRef end, - case get_stream_by_ref(State, StreamRef) of + case get_stream_by_ref(State0, StreamRef) of Stream=#stream{tunnel=#tunnel{protocol=Proto, protocol_state=ProtoState0}} -> - {Commands, EvHandlerState} = Proto:handle_continue(ContinueStreamRef, + {Commands, EvHandlerState1} = Proto:handle_continue(ContinueStreamRef, Msg, ProtoState0, EvHandler, EvHandlerState0), - {{state, tunnel_commands(Commands, Stream, State)}, - EvHandlerState}%; + {State, EvHandlerState} = tunnel_commands(Commands, Stream, State0, EvHandler, EvHandlerState1), + {{state, State}, EvHandlerState} %% The stream may have ended while TLS was being decoded. @todo What should we do? % error -> % {error_stream_not_found(State, StreamRef, ReplyTo), EvHandlerState0} diff --git a/src/gun_tunnel.erl b/src/gun_tunnel.erl index 71ad137..f7d0232 100644 --- a/src/gun_tunnel.erl +++ b/src/gun_tunnel.erl @@ -189,7 +189,7 @@ handle_continue(ContinueStreamRef, {gun_tls_proxy, ProxyPid, {ok, Negotiated}, ProtoOpts#{stream_ref => StreamRef, tunnel_transport => tls}), ReplyTo ! {gun_tunnel_up, self(), StreamRef, Proto:name()}, {{state, State#tunnel_state{protocol=Proto, protocol_state=ProtoState}}, EvHandlerState0}; -handle_continue(ContinueStreamRef, {gun_tls_proxy, ProxyPid, {error, _Reason}, +handle_continue(ContinueStreamRef, {gun_tls_proxy, ProxyPid, {error, Reason}, {handle_continue, _, _HandshakeEvent, _}}, #tunnel_state{socket=ProxyPid}, _EvHandler, EvHandlerState0) when is_reference(ContinueStreamRef) -> @@ -206,27 +206,28 @@ handle_continue(ContinueStreamRef, {gun_tls_proxy, ProxyPid, {error, _Reason}, %% receives a TCP segment with the FIN bit set sends a DATA frame with %% the END_STREAM flag set. Note that the final TCP segment or DATA %% frame could be empty. - {[], EvHandlerState0}; + {{error, Reason}, EvHandlerState0}; %% Send the data. This causes TLS to encrypt the data and send it to the inner layer. handle_continue(ContinueStreamRef, {data, _ReplyTo, _StreamRef, IsFin, Data}, #tunnel_state{}, _EvHandler, EvHandlerState) when is_reference(ContinueStreamRef) -> - {[{send, IsFin, Data}], EvHandlerState}; + {{send, IsFin, Data}, EvHandlerState}; handle_continue(ContinueStreamRef, {tls_proxy, ProxyPid, Data}, State=#tunnel_state{socket=ProxyPid, protocol=Proto, protocol_state=ProtoState}, EvHandler, EvHandlerState0) when is_reference(ContinueStreamRef) -> {Commands, EvHandlerState} = Proto:handle(Data, ProtoState, EvHandler, EvHandlerState0), {{state, commands(Commands, State)}, EvHandlerState}; -%% @todo What to do about those? Does it matter which one closes/errors out? handle_continue(ContinueStreamRef, {tls_proxy_closed, ProxyPid}, - #tunnel_state{socket=ProxyPid}, _EvHandler, _EvHandlerState0) + #tunnel_state{socket=ProxyPid}, _EvHandler, EvHandlerState0) when is_reference(ContinueStreamRef) -> - todo; -handle_continue(ContinueStreamRef, {tls_proxy_error, ProxyPid, _Reason}, - #tunnel_state{socket=ProxyPid}, _EvHandler, _EvHandlerState0) + %% @todo All sub-streams must produce a stream_error. + {{error, closed}, EvHandlerState0}; +handle_continue(ContinueStreamRef, {tls_proxy_error, ProxyPid, Reason}, + #tunnel_state{socket=ProxyPid}, _EvHandler, EvHandlerState0) when is_reference(ContinueStreamRef) -> - todo; + %% @todo All sub-streams must produce a stream_error. + {{error, Reason}, EvHandlerState0}; %% We always dereference the ContinueStreamRef because it includes a %% reference() for Socks layers too. %% @@ -242,21 +243,23 @@ handle_continue([_StreamRef|ContinueStreamRef0], Msg, Msg, ProtoState, EvHandler, EvHandlerState0), {{state, commands(Commands, State)}, EvHandlerState}. -%% @todo Probably just pass it forward? -update_flow(_State, _ReplyTo, _StreamRef, _Inc) -> - todo. +update_flow(State=#tunnel_state{protocol=Proto, protocol_state=ProtoState}, + ReplyTo, StreamRef0, Inc) -> + StreamRef = maybe_dereference(State, StreamRef0), + Commands = Proto:update_flow(ProtoState, ReplyTo, StreamRef, Inc), + {state, commands(Commands, State)}. -%% @todo ? -closing(_Reason, _State, _EvHandler, _EvHandlerState) -> - todo. +closing(_Reason, _State, _EvHandler, EvHandlerState) -> + %% @todo Graceful shutdown must be propagated to tunnels. + {[], EvHandlerState}. -%% @todo ? -close(_Reason, _State, _EvHandler, _EvHandlerState) -> - todo. +close(_Reason, _State, _EvHandler, EvHandlerState) -> + %% @todo Closing must be propagated to tunnels. + EvHandlerState. -%% @todo ? -keepalive(_State, _EvHandler, _EvHandlerState) -> - todo. +keepalive(State, _EvHandler, EvHandlerState) -> + %% @todo Need to figure out how to handle keepalive for tunnels. + {State, EvHandlerState}. %% We pass the headers forward and optionally dereference StreamRef. headers(State=#tunnel_state{protocol=Proto, protocol_state=ProtoState0}, @@ -315,14 +318,16 @@ connect(State=#tunnel_state{info=#{origin_host := Host, origin_port := Port}, ReplyTo, Destination, #{host => Host, port => Port}, Headers, InitialFlow), State#tunnel_state{protocol_state=ProtoState}. -%% @todo ? -cancel(_State, _StreamRef, _ReplyTo, _EvHandler, _EvHandlerState) -> - todo. +cancel(State=#tunnel_state{protocol=Proto, protocol_state=ProtoState}, + StreamRef0, ReplyTo, EvHandler, EvHandlerState0) -> + StreamRef = maybe_dereference(State, StreamRef0), + {Commands, EvHandlerState} = Proto:cancel(ProtoState, StreamRef, ReplyTo, EvHandler, EvHandlerState0), + {{state, commands(Commands, State)}, EvHandlerState}. -%% @todo ? -%% ... we might have to do update Cowlib there... timeout(_State, {cow_http2_machine, _Name}, _TRef) -> - todo. + %% @todo We currently have no way of routing timeout events to the right layer. + %% We will need to update Cowlib to include routing information in the timeout message. + []. stream_info(#tunnel_state{transport=Transport0, stream_ref=TunnelStreamRef, reply_to=ReplyTo, tunnel_protocol=TunnelProtocol, @@ -383,9 +388,9 @@ tunneled_name(#tunnel_state{tunnel_protocol=TunnelProto}, false) -> tunneled_name(#tunnel_state{protocol=Proto}, _) -> Proto:name(). -%% @todo ? down(_State) -> - todo. + %% @todo Tunnels must be included in the gun_down message. + []. %% Internal. -- cgit v1.2.3