From 1ebad8acf803eb797a6c61f6522ebc3b79f104a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Thu, 12 Nov 2020 12:28:46 +0100 Subject: Review and remove many todos HTTP/1.1 Upgrade to HTTP/2 will not be implemented. There are discussions for this functionality to be removed from the HTTP/2 spec. HTTP/1.1 Upgrade to TLS will most likely not be implemented. --- src/gun.erl | 19 ++----------------- src/gun_http.erl | 1 - src/gun_http2.erl | 1 - src/gun_tunnel.erl | 2 -- 4 files changed, 2 insertions(+), 21 deletions(-) diff --git a/src/gun.erl b/src/gun.erl index c4afead..97cd3c9 100644 --- a/src/gun.erl +++ b/src/gun.erl @@ -193,10 +193,6 @@ }. -export_type([raw_opts/0]). -%% @todo When/if HTTP/2 CONNECT gets implemented, we will want an option here -%% to indicate that the request must be sent on an existing CONNECT stream. -%% This is of course not required for HTTP/1.1 since the CONNECT takes over -%% the entire connection. -type req_opts() :: #{ flow => pos_integer(), reply_to => pid(), @@ -912,9 +908,6 @@ cancel(ServerPid, StreamRef) -> stream_info(ServerPid, StreamRef) -> gen_statem:call(ServerPid, {stream_info, StreamRef}). -%% @todo Allow upgrading an HTTP/1.1 connection to HTTP/2. -%% http2_upgrade - %% Websocket. -spec ws_upgrade(pid(), iodata()) -> stream_ref(). @@ -934,7 +927,6 @@ ws_upgrade(ServerPid, Path, Headers, Opts0) -> ok = gun_ws:check_options(Opts), StreamRef = make_stream_ref(Tunnel), ReplyTo = maps:get(reply_to, Opts, self()), - %% @todo Also accept tunnel option. gen_statem:cast(ServerPid, {ws_upgrade, ReplyTo, StreamRef, Path, normalize_headers(Headers), Opts}), StreamRef. @@ -1291,8 +1283,6 @@ connected(cast, {connect, ReplyTo, StreamRef, Destination, Headers, InitialFlow} {keep_state, State#state{protocol_state=ProtoState2, event_handler_state=EvHandlerState}}; %% Public Websocket interface. -%% @todo Maybe make an interface in the protocol module instead of checking on protocol name. -%% An interface would also make sure that HTTP/1.0 can't upgrade. connected(cast, {ws_upgrade, ReplyTo, StreamRef, Path, Headers}, State=#state{opts=Opts}) -> WsOpts = maps:get(ws_opts, Opts, #{}), connected(cast, {ws_upgrade, ReplyTo, StreamRef, Path, Headers, WsOpts}, State); @@ -1418,9 +1408,6 @@ handle_common_connected_no_input(info, {handle_continue, StreamRef, Msg}, _, maybe_active(commands(Commands, State0#state{cookie_store=CookieStore, 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 -%% received timeouts. Currently the timeout messages are ignored. handle_common_connected_no_input(info, keepalive, _, State=#state{protocol=Protocol, protocol_state=ProtoState0, event_handler=EvHandler, event_handler_state=EvHandlerState0}) -> @@ -1443,7 +1430,6 @@ handle_common_connected_no_input({call, From}, {stream_info, StreamRef}, _, {keep_state_and_data, {reply, From, if %% The stream_ref refers to an intermediary. - %% @todo This is probably wrong. length(StreamRef) =< length(Intermediaries) -> Intermediary = lists:nth(length(StreamRef), lists:reverse(Intermediaries)), {Intermediaries1, Tail} = lists:splitwith( @@ -1576,7 +1562,7 @@ handle_common(cast, shutdown, StateName, State=#state{ {undefined, _} -> {stop, shutdown}; {_, undefined} -> - %% @todo This is missing the disconnect event. + %% @todo This is missing the disconnect/terminate events. Transport:close(Socket), {stop, shutdown}; _ when StateName =:= closing, element(1, Status) =:= up -> @@ -1596,7 +1582,7 @@ handle_common(info, {'DOWN', OwnerRef, process, Owner, Reason}, StateName, State _ -> case Protocol of undefined -> - %% @todo This is missing the disconnect event. + %% @todo This is missing the disconnect/terminate events. Transport:close(Socket), owner_down(Reason, State); %% We are already closing so no need to initiate closing again. @@ -1719,7 +1705,6 @@ disconnect(State0=#state{owner=Owner, status=Status, opts=Opts, {up, _} -> %% We closed the socket, discard any remaining socket events. disconnect_flush(State), - %% @todo Stop keepalive timeout, flush message. KilledStreams = Protocol:down(ProtoState), Owner ! {gun_down, self(), Protocol:name(), Reason, KilledStreams}, Retry = maps:get(retry, Opts, 5), diff --git a/src/gun_http.erl b/src/gun_http.erl index a2c3303..67c774b 100644 --- a/src/gun_http.erl +++ b/src/gun_http.erl @@ -368,7 +368,6 @@ handle_inform(Rest, State=#http_state{ status => Status, headers => Headers }, EvHandlerState0), - %% @todo We might want to switch to the HTTP/2 protocol or to the TLS transport as well. case {Version, Status, StreamRef} of {'HTTP/1.1', 101, #websocket{}} -> {ws_handshake(Rest, State, StreamRef, Headers), CookieStore, EvHandlerState}; diff --git a/src/gun_http2.erl b/src/gun_http2.erl index 04d4de5..07aec7e 100644 --- a/src/gun_http2.erl +++ b/src/gun_http2.erl @@ -182,7 +182,6 @@ init(ReplyTo, Socket, Transport, Opts0) -> BaseStreamRef = maps:get(stream_ref, Opts, undefined), TunnelTransport = maps:get(tunnel_transport, Opts, undefined), {ok, Preface, HTTP2Machine} = cow_http2_machine:init(client, Opts#{message_tag => BaseStreamRef}), - %% @todo Better validate the preface being received. State = #http2_state{reply_to=ReplyTo, socket=Socket, transport=Transport, opts=Opts, base_stream_ref=BaseStreamRef, tunnel_transport=TunnelTransport, content_handlers=Handlers, http2_machine=HTTP2Machine}, diff --git a/src/gun_tunnel.erl b/src/gun_tunnel.erl index 2594d24..72f97f7 100644 --- a/src/gun_tunnel.erl +++ b/src/gun_tunnel.erl @@ -121,8 +121,6 @@ init(ReplyTo, OriginSocket, OriginTransport, Opts=#{stream_ref := StreamRef, tun }, EvHandlerState0), %% When the tunnel protocol is HTTP/1.1 or SOCKS %% the gun_tunnel_up message was already sent. - %% - %% @todo There's probably a better way. _ = case TunnelProtocol of http -> ok; socks -> ok; -- cgit v1.2.3