diff options
| -rw-r--r-- | src/cowboy.erl | 1 | ||||
| -rw-r--r-- | src/cowboy_http.erl | 17 | ||||
| -rw-r--r-- | src/cowboy_http2.erl | 2 | ||||
| -rw-r--r-- | src/cowboy_req.erl | 9 | ||||
| -rw-r--r-- | src/cowboy_stream_h.erl | 5 | ||||
| -rw-r--r-- | src/cowboy_websocket.erl | 1 | ||||
| -rw-r--r-- | test/handlers/long_polling_h.erl | 1 |
7 files changed, 36 insertions, 0 deletions
diff --git a/src/cowboy.erl b/src/cowboy.erl index 8ef9f8a..3b272c5 100644 --- a/src/cowboy.erl +++ b/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]). diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl index 5cb00ad..279b11a 100644 --- a/src/cowboy_http.erl +++ b/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]) -> diff --git a/src/cowboy_http2.erl b/src/cowboy_http2.erl index 7bd48a7..f7a9634 100644 --- a/src/cowboy_http2.erl +++ b/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 diff --git a/src/cowboy_req.erl b/src/cowboy_req.erl index bbcd2b7..b2d12b2 100644 --- a/src/cowboy_req.erl +++ b/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) -> diff --git a/src/cowboy_stream_h.erl b/src/cowboy_stream_h.erl index 54dcc2d..e29ffc5 100644 --- a/src/cowboy_stream_h.erl +++ b/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), diff --git a/src/cowboy_websocket.erl b/src/cowboy_websocket.erl index 757f52c..9ce7a5a 100644 --- a/src/cowboy_websocket.erl +++ b/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 diff --git a/test/handlers/long_polling_h.erl b/test/handlers/long_polling_h.erl index 4f8e23f..17afa0a 100644 --- a/test/handlers/long_polling_h.erl +++ b/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), |