Add a lot of todos

src/cowboy.erl

@@ -19,6 +19,7 @@
 -export([stop_listener/1]).
 -export([set_env/3]).
 
+%% @todo Detailed opts.
 -type opts() :: map().
 -export_type([opts/0]).
 

src/cowboy_http.erl

@@ -270,6 +270,7 @@ after_parse({request, Req=#{streamid := StreamID, headers := Headers, version :=
 		error_logger:error_msg("Exception occurred in ~s:init(~p, ~p, ~p) "
 			"with reason ~p:~p.",
 			[Handler, StreamID, Req, Opts, Class, Reason]),
+		%% @todo Bad value returned here. Crashes.
 		ok
 		%% @todo Status code.
 %		stream_reset(State, StreamID, {internal_error, {Class, Reason},
@@ -287,6 +288,7 @@ after_parse({data, StreamID, IsFin, Data, State=#state{handler=Handler,
 	catch Class:Reason ->
 		error_logger:error_msg("Exception occurred in ~s:data(~p, ~p, ~p, ~p) with reason ~p:~p.",
 			[Handler, StreamID, IsFin, Data, StreamState0, Class, Reason]),
+		%% @todo Bad value returned here. Crashes.
 		ok
 		%% @todo
 %		stream_reset(State, StreamID, {internal_error, {Class, Reason},
@@ -618,6 +620,9 @@ request(Buffer, State0=#state{ref=Ref, transport=Transport, peer=Peer, in_stream
 		scheme => Scheme,
 		host => Host,
 		port => Port,
+
+%% @todo So the path component needs to be normalized.
+
 		path => Path,
 		qs => Qs,
 		version => Version,
@@ -735,6 +740,11 @@ parse_body(Buffer, State=#state{in_streamid=StreamID, in_state=
 
 %% Message handling.
 
+%% @todo There is a difference in behavior between HTTP/1.1 and HTTP/2
+%% when an error or crash occurs after sending a 500 response. In HTTP/2
+%% the error will be printed, in HTTP/1.1 the error will be ignored.
+%% This is due to HTTP/1.1 disabling streams differently after both
+%% requests and responses have been sent.
 down(State=#state{children=Children0}, Pid, Msg) ->
 	case lists:keytake(Pid, 1, Children0) of
 		{value, {_, undefined, _}, Children} ->
@@ -837,8 +847,12 @@ commands(State0=#state{socket=Socket, transport=Transport, streams=Streams}, Str
 %% Send a response body chunk.
 %%
 %% @todo WINDOW_UPDATE stuff require us to buffer some data.
+%% @todo We probably want to allow Data to be the {sendfile, ...} tuple also.
 commands(State=#state{socket=Socket, transport=Transport, streams=Streams}, StreamID,
 		[{data, IsFin, Data}|Tail]) ->
+
+	%% @todo We need to kill the stream if it tries to send data before headers.
+
 	%% @todo Same as above.
 	case lists:keyfind(StreamID, #stream.id, Streams) of
 		#stream{version='HTTP/1.1'} ->
@@ -879,6 +893,9 @@ commands(State0=#state{ref=Ref, parent=Parent, socket=Socket, transport=Transpor
 commands(State, StreamID, [stop|Tail]) ->
 	%% @todo Do we want to run the commands after a stop?
 %	commands(stream_terminate(State, StreamID, stop), StreamID, Tail).
+
+	%% @todo I think that's where we need to terminate streams.
+
 	maybe_terminate(State, StreamID, Tail, fin);
 %% HTTP/1.1 does not support push; ignore.
 commands(State, StreamID, [{push, _, _, _, _, _, _, _}|Tail]) ->

src/cowboy_http2.erl

@@ -418,7 +418,9 @@ commands(State=#state{socket=Socket, transport=Transport}, Stream=#stream{id=Str
 		[{data, IsFin, Data}|Tail]) ->
 	Transport:send(Socket, cow_http2:data(StreamID, IsFin, Data)),
 	commands(State, Stream#stream{local=IsFin}, Tail);
+
 %% @todo data when local!=nofin
+
 %% Send a file.
 %%
 %% @todo This implementation is terrible. A good implementation would

src/cowboy_req.erl

@@ -59,10 +59,12 @@
 -export([set_resp_cookie/3]).
 -export([set_resp_cookie/4]).
 -export([set_resp_header/3]).
+%% @todo set_resp_headers/2
 -export([has_resp_header/2]).
 %% @todo resp_header
 -export([delete_resp_header/2]).
 -export([set_resp_body/2]). %% @todo Use set_resp_body for iodata() | {sendfile ...}
+%% @todo set_resp_body/3 with a ContentType or even Headers argument, to set content headers.
 -export([has_resp_body/1]).
 -export([reply/2]).
 -export([reply/3]).
@@ -139,6 +141,7 @@ scheme(#{scheme := Scheme}) ->
 host(#{host := Host}) ->
 	Host.
 
+%% @todo The host_info is undefined if cowboy_router isn't used. Do we want to crash?
 -spec host_info(req()) -> cowboy_router:tokens() | undefined.
 host_info(#{host_info := HostInfo}) ->
 	HostInfo.
@@ -151,6 +154,7 @@ port(#{port := Port}) ->
 path(#{path := Path}) ->
 	Path.
 
+%% @todo The path_info is undefined if cowboy_router isn't used. Do we want to crash?
 -spec path_info(req()) -> cowboy_router:tokens() | undefined.
 path_info(#{path_info := PathInfo}) ->
 	PathInfo.
@@ -159,6 +163,7 @@ path_info(#{path_info := PathInfo}) ->
 qs(#{qs := Qs}) ->
 	Qs.
 
+%% @todo Might be useful to limit the number of keys.
 -spec parse_qs(req()) -> [{binary(), binary() | true}].
 parse_qs(#{qs := Qs}) ->
 	cow_qs:parse_qs(Qs).
@@ -333,6 +338,7 @@ parse_header(Name = <<"content-length">>, Req) ->
 	parse_header(Name, Req, 0, fun cow_http_hd:parse_content_length/1);
 parse_header(Name = <<"cookie">>, Req) ->
 	parse_header(Name, Req, [], fun cow_cookie:parse_cookie/1);
+%% @todo That header is abstracted out and should never reach cowboy_req.
 parse_header(Name = <<"transfer-encoding">>, Req) ->
 	parse_header(Name, Req, [<<"identity">>], fun cow_http_hd:parse_transfer_encoding/1);
 parse_header(Name, Req) ->
@@ -459,6 +465,8 @@ part(Buffer, Opts, Req=#{multipart := {Boundary, _}}) ->
 			{Data, Req2} = stream_multipart(Req, Opts),
 			part(<< Buffer2/binary, Data/binary >>, Opts, Req2);
 		{ok, Headers, Rest} ->
+			%% @todo We may want headers as a map. Need to check the
+			%% rules for multipart header parsing before taking a decision.
 			{ok, Headers, Req#{multipart => {Boundary, Rest}}};
 		%% Ignore epilogue.
 		{done, _} ->
@@ -529,6 +537,7 @@ set_resp_cookie(Name, Value, Req) ->
 %%
 %% The cookie value cannot contain any of the following characters:
 %%   ,; \t\r\n\013\014
+%% @todo Fix the cookie_opts() type.
 -spec set_resp_cookie(iodata(), iodata(), cookie_opts(), Req)
 	-> Req when Req::req().
 set_resp_cookie(Name, Value, Opts, Req) ->

src/cowboy_stream_h.erl

@@ -152,6 +152,11 @@ report_crash(Ref, StreamID, Pid, Reason, Stacktrace) ->
 
 %% Request process.
 
+%% @todo This should wrap with try/catch to get the full error
+%% in the stream handler. Only then can we decide what to do
+%% about it. This means that we should remove any other try/catch
+%% in the request process.
+
 %% This hack is necessary because proc_lib does not propagate
 %% stacktraces by default. This is ugly because we end up
 %% having two try/catch instead of one (the one in proc_lib),

src/cowboy_websocket.erl

@@ -102,6 +102,7 @@ websocket_upgrade(State, Req) ->
 	-> {ok, #state{}, Req} when Req::cowboy_req:req().
 websocket_extensions(State, Req) ->
 	%% @todo We want different options for this. For example
+	%% @todo This should probably be configurable per handler, like timeout/hibernate.
 	%% * compress everything auto
 	%% * compress only text auto
 	%% * compress only binary auto

test/handlers/long_polling_h.erl

@@ -14,6 +14,7 @@ init(Req, _) ->
 	{cowboy_loop, Req, 2, 5000, hibernate}.
 
 info(timeout, Req, 0) ->
+	%% @todo Why 102?
 	{stop, cowboy_req:reply(102, Req), 0};
 info(timeout, Req, Count) ->
 	erlang:send_after(200, self(), timeout),