From d47e22edaa1a876081c07bf49c79587c3c2d21d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Mon, 25 Sep 2017 13:52:58 +0200 Subject: Cleanup various comments --- src/cowboy_http.erl | 24 +++++------------------- src/cowboy_http2.erl | 22 +--------------------- 2 files changed, 6 insertions(+), 40 deletions(-) (limited to 'src') diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl index f0f8ed7..a7c9b4d 100644 --- a/src/cowboy_http.erl +++ b/src/cowboy_http.erl @@ -52,18 +52,10 @@ name = undefined :: binary() | undefined }). -%% @todo We need a state where we wait for the stream process to ask for the body. -%% OR DO WE - -%% In HTTP/2 we start receiving data before the body asks for it, even if optionally -%% (and by default), so we need to be able to do the same for HTTP/1.1 too. This means -%% that when we receive data (up to a certain limit, we read from the socket and decode. -%% When we reach a limit, we stop reading from the socket momentarily until the stream -%% process asks for more or the stream ends. - -%% This means that we need to keep a buffer in the stream handler (until the stream -%% process asks for it). And that we need the body state to indicate how much we have -%% left to read (and stop/start reading from the socket depending on value). +%% @todo We need a state where we wait for the stream process to ask for the body +%% and do not attempt to read from the socket while in that state (we should read +%% up to a certain length, and then wait, basically implementing flow control but +%% by not reading from the socket when the window is empty). -record(ps_body, { %% @todo flow @@ -135,9 +127,6 @@ init(Parent, Ref, Socket, Transport, Opts) -> terminate(undefined, {socket_error, Reason, 'An error has occurred on the socket.'}) end. -%% @todo Send a response depending on in_state and whether one was already sent. -%% @todo If we skip the body, skip for a specific duration. - before_loop(State=#state{socket=Socket, transport=Transport}, Buffer) -> %% @todo disable this when we get to the body, until the stream asks for it? %% Perhaps have a threshold for how much we're willing to read before waiting. @@ -608,7 +597,6 @@ request(Buffer, State0=#state{ref=Ref, transport=Transport, peer=Peer, in_stream scheme => Scheme, host => Host, port => Port, - %% @todo The path component needs to be normalized. path => Path, qs => Qs, version => Version, @@ -623,7 +611,6 @@ request(Buffer, State0=#state{ref=Ref, transport=Transport, peer=Peer, in_stream State = case HasBody of true -> State0#state{in_state=#ps_body{ - %% @todo Don't need length anymore? transfer_decode_fun = TDecodeFun, transfer_decode_state = TDecodeState }}; @@ -659,8 +646,6 @@ is_http2_upgrade(#{<<"connection">> := Conn, <<"upgrade">> := Upgrade, is_http2_upgrade(_, _) -> false. -%% Upgrade through an HTTP/1.1 request. - %% Prior knowledge upgrade, without an HTTP/1.1 request. http2_upgrade(State=#state{parent=Parent, ref=Ref, socket=Socket, transport=Transport, opts=Opts, peer=Peer}, Buffer) -> @@ -673,6 +658,7 @@ http2_upgrade(State=#state{parent=Parent, ref=Ref, socket=Socket, transport=Tran 'Clients that support HTTP/2 over TLS MUST use ALPN. (RFC7540 3.4)'}) end. +%% Upgrade via an HTTP/1.1 request. http2_upgrade(State=#state{parent=Parent, ref=Ref, socket=Socket, transport=Transport, opts=Opts, peer=Peer}, Buffer, HTTP2Settings, Req) -> %% @todo diff --git a/src/cowboy_http2.erl b/src/cowboy_http2.erl index fb6bf41..8c6a0d3 100644 --- a/src/cowboy_http2.erl +++ b/src/cowboy_http2.erl @@ -481,7 +481,7 @@ commands(State=#state{socket=Socket, transport=Transport, encode_state=EncodeSta commands(State1#state{encode_state=EncodeState}, Stream1, Tail) end; %% @todo response when local!=idle -%% Send response headers and initiate chunked encoding. +%% Send response headers. commands(State=#state{socket=Socket, transport=Transport, encode_state=EncodeState0}, Stream=#stream{id=StreamID, local=idle}, [{headers, StatusCode, Headers0}|Tail]) -> Headers = Headers0#{<<":status">> => status(StatusCode)}, @@ -490,16 +490,6 @@ commands(State=#state{socket=Socket, transport=Transport, encode_state=EncodeSta commands(State#state{encode_state=EncodeState}, Stream#stream{local=nofin}, Tail); %% @todo headers when local!=idle %% Send a response body chunk. -%% -%% @todo WINDOW_UPDATE stuff require us to buffer some data. -%% -%% When the body is sent using sendfile, the current solution is not -%% very good. The body could be too large, blocking the connection. -%% Also sendfile technically only works over TCP, so it's not that -%% useful for HTTP/2. At the very least the sendfile call should be -%% split into multiple calls and flow control should be used to make -%% sure we only send as fast as the client can receive and don't block -%% anything. commands(State0, Stream0=#stream{local=nofin}, [{data, IsFin, Data}|Tail]) -> {State, Stream} = send_data(State0, Stream0, IsFin, Data), commands(State, Stream, Tail); @@ -507,16 +497,6 @@ commands(State0, Stream0=#stream{local=nofin}, [{data, IsFin, Data}|Tail]) -> %% @todo data when local!=nofin %% Send a file. -%% -%% @todo This implementation is terrible. A good implementation would -%% need to check that Bytes is exact (or we need to document that we -%% trust it to be exact), and would need to send the file asynchronously -%% in many data frames. Perhaps a sendfile call should result in a -%% process being created specifically for this purpose. Or perhaps -%% the protocol should be "dumb" and the stream handler be the one -%% to ensure the file is sent in chunks (which would require a better -%% flow control at the stream handler level). One thing for sure, the -%% implementation necessarily varies between HTTP/1.1 and HTTP/2. commands(State0, Stream0=#stream{local=nofin}, [{sendfile, IsFin, Offset, Bytes, Path}|Tail]) -> {State, Stream} = send_data(State0, Stream0, IsFin, {sendfile, Offset, Bytes, Path}), -- cgit v1.2.3