aboutsummaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorLoïc Hoguin <[email protected]>2017-09-14 13:42:17 +0200
committerLoïc Hoguin <[email protected]>2017-09-14 13:42:17 +0200
commit3cbf885961dc93df1b39d2de89f2a871402acbd5 (patch)
tree8f8f80ea6e6756fb5136cd6eff9bc03ea00cf481 /src
parent15ceaf1edff2a68e461fafd8cd77fc7f98918c8a (diff)
downloadcowboy-3cbf885961dc93df1b39d2de89f2a871402acbd5.tar.gz
cowboy-3cbf885961dc93df1b39d2de89f2a871402acbd5.tar.bz2
cowboy-3cbf885961dc93df1b39d2de89f2a871402acbd5.zip
Improve how we detect request errors
When the request process exits with a {request_error, Reason, Human} exit reason, Cowboy will return a 400 status code instead of 500. Cowboy may also return a more specific status code depending on the error. Currently it may also return 408 or 413. This should prove to be more solid that looking inside the stack trace.
Diffstat (limited to 'src')
-rw-r--r--src/cowboy_http.erl11
-rw-r--r--src/cowboy_req.erl57
-rw-r--r--src/cowboy_stream_h.erl21
3 files changed, 68 insertions, 21 deletions
diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl
index 5ee5ceb..c9c6383 100644
--- a/src/cowboy_http.erl
+++ b/src/cowboy_http.erl
@@ -785,8 +785,15 @@ commands(State, StreamID, [{flow, _Length}|Tail]) ->
commands(State, StreamID, Tail);
%% Error responses are sent only if a response wasn't sent already.
-commands(State=#state{out_state=wait}, StreamID, [{error_response, StatusCode, Headers, Body}|Tail]) ->
- commands(State, StreamID, [{response, StatusCode, Headers, Body}|Tail]);
+commands(State=#state{out_state=wait}, StreamID, [{error_response, Status, Headers0, Body}|Tail]) ->
+ %% We close the connection when the error response is 408, as it
+ %% indicates a timeout and the RFC recommends that we stop here. (RFC7231 6.5.7)
+ Headers = case Status of
+ 408 -> Headers0#{<<"connection">> => <<"close">>};
+ <<"408", _/bits>> -> Headers0#{<<"connection">> => <<"close">>};
+ _ -> Headers0
+ end,
+ commands(State, StreamID, [{response, Status, Headers, Body}|Tail]);
commands(State, StreamID, [{error_response, _, _, _}|Tail]) ->
commands(State, StreamID, Tail);
%% Send an informational response.
diff --git a/src/cowboy_req.erl b/src/cowboy_req.erl
index 0106771..84e1b9d 100644
--- a/src/cowboy_req.erl
+++ b/src/cowboy_req.erl
@@ -184,7 +184,13 @@ 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).
+ try
+ cow_qs:parse_qs(Qs)
+ catch _:_ ->
+ erlang:raise(exit, {request_error, qs,
+ 'Malformed query string; application/x-www-form-urlencoded expected.'
+ }, erlang:get_stacktrace())
+ end.
-spec match_qs(cowboy:fields(), req()) -> map().
match_qs(Fields, Req) ->
@@ -353,15 +359,21 @@ headers(#{headers := Headers}) ->
-spec parse_header(binary(), Req) -> any() when Req::req().
parse_header(Name = <<"content-length">>, Req) ->
- parse_header(Name, Req, 0, fun cow_http_hd:parse_content_length/1);
+ parse_header(Name, Req, 0);
parse_header(Name = <<"cookie">>, Req) ->
- parse_header(Name, Req, [], fun cow_cookie:parse_cookie/1);
+ parse_header(Name, Req, []);
parse_header(Name, Req) ->
parse_header(Name, Req, undefined).
-spec parse_header(binary(), Req, any()) -> any() when Req::req().
parse_header(Name, Req, Default) ->
- parse_header(Name, Req, Default, parse_header_fun(Name)).
+ try
+ parse_header(Name, Req, Default, parse_header_fun(Name))
+ catch _:_ ->
+ erlang:raise(exit, {request_error, {header, Name},
+ 'Malformed header. Please consult the relevant specification.'
+ }, erlang:get_stacktrace())
+ end.
parse_header_fun(<<"accept">>) -> fun cow_http_hd:parse_accept/1;
parse_header_fun(<<"accept-charset">>) -> fun cow_http_hd:parse_accept_charset/1;
@@ -447,8 +459,26 @@ read_urlencoded_body(Req) ->
-spec read_urlencoded_body(Req, read_body_opts()) -> {ok, [{binary(), binary() | true}], Req} when Req::req().
read_urlencoded_body(Req0, Opts) ->
- {ok, Body, Req} = read_body(Req0, Opts),
- {ok, cow_qs:parse_qs(Body), Req}.
+ case read_body(Req0, Opts) of
+ {ok, Body, Req} ->
+ try
+ {ok, cow_qs:parse_qs(Body), Req}
+ catch _:_ ->
+ erlang:raise(exit, {request_error, urlencoded_body,
+ 'Malformed body; application/x-www-form-urlencoded expected.'
+ }, erlang:get_stacktrace())
+ end;
+ {more, Body, _} ->
+ Length = maps:get(length, Opts, 64000),
+ if
+ byte_size(Body) < Length ->
+ exit({request_error, timeout,
+ 'The request body was not received within the configured time.'});
+ true ->
+ exit({request_error, payload_too_large,
+ 'The request body is larger than allowed by configuration.'})
+ end
+ end.
%% Multipart.
@@ -471,7 +501,7 @@ read_part(Req, Opts) ->
end.
read_part(Buffer, Opts, Req=#{multipart := {Boundary, _}}) ->
- case cow_multipart:parse_headers(Buffer, Boundary) of
+ try cow_multipart:parse_headers(Buffer, Boundary) of
more ->
{Data, Req2} = stream_multipart(Req, Opts),
read_part(<< Buffer/binary, Data/binary >>, Opts, Req2);
@@ -486,6 +516,10 @@ read_part(Buffer, Opts, Req=#{multipart := {Boundary, _}}) ->
%% Ignore epilogue.
{done, _} ->
{done, Req#{multipart => done}}
+ catch _:_ ->
+ erlang:raise(exit, {request_error, {multipart, headers},
+ 'Malformed body; multipart expected.'
+ }, erlang:get_stacktrace())
end.
-spec read_part_body(Req)
@@ -529,8 +563,13 @@ read_part_body(Buffer, Opts, Req=#{multipart := {Boundary, _}}, Acc) ->
init_multipart(Req) ->
{<<"multipart">>, _, Params} = parse_header(<<"content-type">>, Req),
- {_, Boundary} = lists:keyfind(<<"boundary">>, 1, Params),
- Req#{multipart => {Boundary, <<>>}}.
+ case lists:keyfind(<<"boundary">>, 1, Params) of
+ {_, Boundary} ->
+ Req#{multipart => {Boundary, <<>>}};
+ false ->
+ exit({request_error, {multipart, boundary},
+ 'Missing boundary parameter for multipart media type.'})
+ end.
stream_multipart(Req=#{multipart := done}, _) ->
{<<>>, Req};
diff --git a/src/cowboy_stream_h.erl b/src/cowboy_stream_h.erl
index 5113f2e..4459679 100644
--- a/src/cowboy_stream_h.erl
+++ b/src/cowboy_stream_h.erl
@@ -69,14 +69,17 @@ data(_StreamID, IsFin, Data, State=#state{pid=Pid, read_body_ref=Ref,
-spec info(cowboy_stream:streamid(), any(), State)
-> {cowboy_stream:commands(), State} when State::#state{}.
info(_StreamID, {'EXIT', Pid, normal}, State=#state{pid=Pid}) ->
- %% @todo Do we even reach this clause?
{[stop], State};
-info(_StreamID, {'EXIT', Pid, {_Reason, [T1, T2|_]}}, State=#state{pid=Pid})
- when element(1, T1) =:= cow_http_hd; element(1, T2) =:= cow_http_hd ->
- %% @todo Have an option to enable/disable this specific crash report?
+info(_StreamID, {'EXIT', Pid, {{request_error, Reason, _HumanReadable}, _}}, State=#state{pid=Pid}) ->
+ %% @todo Optionally report the crash to help debugging.
%%report_crash(Ref, StreamID, Pid, Reason, Stacktrace),
+ Status = case Reason of
+ timeout -> 408;
+ payload_too_large -> 413;
+ _ -> 400
+ end,
%% @todo Headers? Details in body? More stuff in debug only?
- {[{error_response, 400, #{<<"content-length">> => <<"0">>}, <<>>}, stop], State};
+ {[{error_response, Status, #{<<"content-length">> => <<"0">>}, <<>>}, stop], State};
info(StreamID, Exit = {'EXIT', Pid, {Reason, Stacktrace}}, State=#state{ref=Ref, pid=Pid}) ->
report_crash(Ref, StreamID, Pid, Reason, Stacktrace),
{[
@@ -162,11 +165,9 @@ report_crash(Ref, StreamID, Pid, Reason, Stacktrace) ->
proc_lib_hack(Req, Env, Middlewares) ->
try
execute(Req, Env, Middlewares)
- catch
- _:Reason when element(1, Reason) =:= cowboy_handler ->
- exit(Reason);
- _:Reason ->
- exit({Reason, erlang:get_stacktrace()})
+ catch _:Reason ->
+ %% @todo Have a way to identify OTP 20 to not do this twice?
+ exit({Reason, erlang:get_stacktrace()})
end.
%% @todo