diff options
authorLoïc Hoguin <[email protected]>2017-09-25 13:52:58 +0200
committerLoïc Hoguin <[email protected]>2017-09-25 13:52:58 +0200
commitd47e22edaa1a876081c07bf49c79587c3c2d21d5 (patch)
parentea80e7a3097c985befa60278dde1c4cedc145fb6 (diff)
Cleanup various comments
2 files changed, 6 insertions, 40 deletions
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.'})
-%% @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 ->
- %% @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(_, _) ->
-%% 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)'})
+%% 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)
%% @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}),