aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLoïc Hoguin <[email protected]>2014-07-12 12:09:43 +0200
committerLoïc Hoguin <[email protected]>2014-07-12 12:09:43 +0200
commit97a3108576c6a9d64c03e1455654dba88367992a (patch)
treec9701a8ba567e777509e0fd3508ed6f0bc6b5ea0
parent20f598f3736cdaab8fd0e9de09f16d18d1dc97f8 (diff)
downloadcowboy-97a3108576c6a9d64c03e1455654dba88367992a.tar.gz
cowboy-97a3108576c6a9d64c03e1455654dba88367992a.tar.bz2
cowboy-97a3108576c6a9d64c03e1455654dba88367992a.zip
Reply with 400 on header parsing crash
This is a first step to improve the HTTP status codes returned by Cowboy on crashes. We will tweak it over time. Also fixes a small bug where two replies may have been sent when using loop handlers under rare conditions.
-rw-r--r--src/cowboy_handler.erl27
-rw-r--r--src/cowboy_protocol.erl2
-rw-r--r--src/cowboy_req.erl13
-rw-r--r--src/cowboy_rest.erl10
-rw-r--r--src/cowboy_spdy.erl4
-rw-r--r--src/cowboy_websocket.erl13
-rw-r--r--test/handlers/input_crash_h.erl10
-rw-r--r--test/http_SUITE.erl2
8 files changed, 55 insertions, 26 deletions
diff --git a/src/cowboy_handler.erl b/src/cowboy_handler.erl
index 263c659..d78321b 100644
--- a/src/cowboy_handler.erl
+++ b/src/cowboy_handler.erl
@@ -85,11 +85,12 @@ handler_init(Req, State, Handler, HandlerOpts) ->
{upgrade, protocol, Module, Req2, HandlerOpts2} ->
upgrade_protocol(Req2, State, Handler, HandlerOpts2, Module)
catch Class:Reason ->
- cowboy_req:maybe_reply(500, Req),
+ Stacktrace = erlang:get_stacktrace(),
+ cowboy_req:maybe_reply(Stacktrace, Req),
erlang:Class([
{reason, Reason},
{mfa, {Handler, init, 3}},
- {stacktrace, erlang:get_stacktrace()},
+ {stacktrace, Stacktrace},
{req, cowboy_req:to_list(Req)},
{opts, HandlerOpts}
])
@@ -113,12 +114,13 @@ handler_handle(Req, State, Handler, HandlerState) ->
terminate_request(Req2, State, Handler, HandlerState2,
{normal, shutdown})
catch Class:Reason ->
- cowboy_req:maybe_reply(500, Req),
+ Stacktrace = erlang:get_stacktrace(),
+ cowboy_req:maybe_reply(Stacktrace, Req),
handler_terminate(Req, Handler, HandlerState, Reason),
erlang:Class([
{reason, Reason},
{mfa, {Handler, handle, 2}},
- {stacktrace, erlang:get_stacktrace()},
+ {stacktrace, Stacktrace},
{req, cowboy_req:to_list(Req)},
{state, HandlerState}
])
@@ -170,8 +172,8 @@ handler_loop_timeout(State=#state{loop_timeout=Timeout,
-> {ok, Req, cowboy_middleware:env()} | {suspend, module(), atom(), [any()]}
when Req::cowboy_req:req().
handler_loop(Req, State=#state{loop_buffer_size=NbBytes,
- loop_max_buffer=Threshold, loop_timeout_ref=TRef},
- Handler, HandlerState) ->
+ loop_max_buffer=Threshold, loop_timeout_ref=TRef,
+ resp_sent=RespSent}, Handler, HandlerState) ->
[Socket, Transport] = cowboy_req:get([socket, transport], Req),
{OK, Closed, Error} = Transport:messages(),
receive
@@ -180,7 +182,9 @@ handler_loop(Req, State=#state{loop_buffer_size=NbBytes,
if NbBytes2 > Threshold ->
_ = handler_terminate(Req, Handler, HandlerState,
{error, overflow}),
- cowboy_req:maybe_reply(500, Req),
+ _ = if RespSent -> ok; true ->
+ cowboy_req:reply(500, Req)
+ end,
exit(normal);
true ->
Req2 = cowboy_req:append_buffer(Data, Req),
@@ -229,16 +233,15 @@ handler_call(Req, State=#state{resp_sent=RespSent},
handler_after_callback(Req2, State#state{hibernate=true},
Handler, HandlerState2)
catch Class:Reason ->
- if RespSent ->
- ok;
- true ->
- cowboy_req:maybe_reply(500, Req)
+ Stacktrace = erlang:get_stacktrace(),
+ if RespSent -> ok; true ->
+ cowboy_req:maybe_reply(Stacktrace, Req)
end,
handler_terminate(Req, Handler, HandlerState, Reason),
erlang:Class([
{reason, Reason},
{mfa, {Handler, info, 3}},
- {stacktrace, erlang:get_stacktrace()},
+ {stacktrace, Stacktrace},
{req, cowboy_req:to_list(Req)},
{state, HandlerState}
])
diff --git a/src/cowboy_protocol.erl b/src/cowboy_protocol.erl
index 3ac967e..84c4401 100644
--- a/src/cowboy_protocol.erl
+++ b/src/cowboy_protocol.erl
@@ -494,7 +494,7 @@ error_terminate(Status, State=#state{socket=Socket, transport=Transport,
-spec error_terminate(cowboy:http_status(), cowboy_req:req(), #state{}) -> ok.
error_terminate(Status, Req, State) ->
- cowboy_req:maybe_reply(Status, Req),
+ _ = cowboy_req:reply(Status, Req),
terminate(State).
-spec terminate(#state{}) -> ok.
diff --git a/src/cowboy_req.erl b/src/cowboy_req.erl
index 02089c2..86f06ec 100644
--- a/src/cowboy_req.erl
+++ b/src/cowboy_req.erl
@@ -1036,15 +1036,22 @@ continue(#http_req{socket=Socket, transport=Transport,
<< HTTPVer/binary, " ", (status(100))/binary, "\r\n\r\n" >>).
%% Meant to be used internally for sending errors after crashes.
--spec maybe_reply(cowboy:http_status(), req()) -> ok.
-maybe_reply(Status, Req) ->
+-spec maybe_reply([{module(), atom(), arity() | [term()], _}], req()) -> ok.
+maybe_reply(Stacktrace, Req) ->
receive
{cowboy_req, resp_sent} -> ok
after 0 ->
- _ = cowboy_req:reply(Status, Req),
+ _ = do_maybe_reply(Stacktrace, Req),
ok
end.
+do_maybe_reply([
+ {cow_http_hd, _, _, _},
+ {cowboy_req, parse_header, _, _}|_], Req) ->
+ cowboy_req:reply(400, Req);
+do_maybe_reply(_, Req) ->
+ cowboy_req:reply(500, Req).
+
-spec ensure_response(req(), cowboy:http_status()) -> ok.
%% The response has already been fully sent to the client.
ensure_response(#http_req{resp_state=done}, _) ->
diff --git a/src/cowboy_rest.erl b/src/cowboy_rest.erl
index fd1588a..f779612 100644
--- a/src/cowboy_rest.erl
+++ b/src/cowboy_rest.erl
@@ -66,11 +66,12 @@ upgrade(Req, Env, Handler, HandlerOpts) ->
service_available(Req2, #state{env=Env, method=Method,
handler=Handler, handler_state=HandlerState})
catch Class:Reason ->
- cowboy_req:maybe_reply(500, Req),
+ Stacktrace = erlang:get_stacktrace(),
+ cowboy_req:maybe_reply(Stacktrace, Req),
erlang:Class([
{reason, Reason},
{mfa, {Handler, rest_init, 2}},
- {stacktrace, erlang:get_stacktrace()},
+ {stacktrace, Stacktrace},
{req, cowboy_req:to_list(Req)},
{opts, HandlerOpts}
])
@@ -999,11 +1000,12 @@ terminate(Req, State=#state{env=Env}) ->
error_terminate(Req, State=#state{handler=Handler, handler_state=HandlerState},
Class, Reason, Callback) ->
rest_terminate(Req, State),
- cowboy_req:maybe_reply(500, Req),
+ Stacktrace = erlang:get_stacktrace(),
+ cowboy_req:maybe_reply(Stacktrace, Req),
erlang:Class([
{reason, Reason},
{mfa, {Handler, Callback, 2}},
- {stacktrace, erlang:get_stacktrace()},
+ {stacktrace, Stacktrace},
{req, cowboy_req:to_list(Req)},
{state, HandlerState}
]).
diff --git a/src/cowboy_spdy.erl b/src/cowboy_spdy.erl
index faea142..8da9613 100644
--- a/src/cowboy_spdy.erl
+++ b/src/cowboy_spdy.erl
@@ -422,7 +422,7 @@ execute(Req, Env, [Middleware|Tail]) ->
{halt, Req2} ->
cowboy_req:ensure_response(Req2, 204);
{error, Status, Req2} ->
- cowboy_req:maybe_reply(Status, Req2)
+ cowboy_req:reply(Status, Req2)
end.
-spec resume(cowboy_middleware:env(), [module()],
@@ -437,7 +437,7 @@ resume(Env, Tail, Module, Function, Args) ->
{halt, Req2} ->
cowboy_req:ensure_response(Req2, 204);
{error, Status, Req2} ->
- cowboy_req:maybe_reply(Status, Req2)
+ cowboy_req:reply(Status, Req2)
end.
%% Reply functions used by cowboy_req.
diff --git a/src/cowboy_websocket.erl b/src/cowboy_websocket.erl
index 6c41137..d13441d 100644
--- a/src/cowboy_websocket.erl
+++ b/src/cowboy_websocket.erl
@@ -71,8 +71,12 @@ upgrade(Req, Env, Handler, HandlerOpts) ->
{ok, State2, Req2} ->
handler_init(State2, Req2, HandlerOpts)
catch _:_ ->
- cowboy_req:maybe_reply(400, Req),
- exit(normal)
+ receive
+ {cowboy_req, resp_sent} -> ok
+ after 0 ->
+ cowboy_req:reply(400, Req),
+ exit(normal)
+ end
end.
-spec websocket_upgrade(#state{}, Req)
@@ -144,11 +148,12 @@ handler_init(State=#state{env=Env, transport=Transport,
cowboy_req:ensure_response(Req2, 400),
{ok, Req2, [{result, closed}|Env]}
catch Class:Reason ->
- cowboy_req:maybe_reply(400, Req),
+ Stacktrace = erlang:get_stacktrace(),
+ cowboy_req:maybe_reply(Stacktrace, Req),
erlang:Class([
{reason, Reason},
{mfa, {Handler, websocket_init, 3}},
- {stacktrace, erlang:get_stacktrace()},
+ {stacktrace, Stacktrace},
{req, cowboy_req:to_list(Req)},
{opts, HandlerOpts}
])
diff --git a/test/handlers/input_crash_h.erl b/test/handlers/input_crash_h.erl
new file mode 100644
index 0000000..668d053
--- /dev/null
+++ b/test/handlers/input_crash_h.erl
@@ -0,0 +1,10 @@
+%% This module crashes on request input data
+%% depending on the given option.
+
+-module(input_crash_h).
+
+-export([init/3]).
+
+init(_, Req, content_length) ->
+ cowboy_error_h:ignore(cow_http_hd, number, 2),
+ cowboy_req:parse_header(<<"content-length">>, Req).
diff --git a/test/http_SUITE.erl b/test/http_SUITE.erl
index 5888421..e5a9256 100644
--- a/test/http_SUITE.erl
+++ b/test/http_SUITE.erl
@@ -193,6 +193,7 @@ init_dispatch(Config) ->
{"/multipart/large", http_multipart_stream, []},
{"/echo/body", http_echo_body, []},
{"/echo/body_qs", http_body_qs, []},
+ {"/crash/content-length", input_crash_h, content_length},
{"/param_all", rest_param_all, []},
{"/bad_accept", rest_simple_resource, []},
{"/bad_content_type", rest_patch_resource, []},
@@ -274,6 +275,7 @@ The document has moved
{400, "GET / HTTP/1.1\r\nHost: ninenines.eu\r\n\r\n"},
{400, "GET http://proxy/ HTTP/1.1\r\n\r\n"},
{400, "GET / HTTP/1.1\r\nHost: localhost:bad_port\r\n\r\n"},
+ {400, ["POST /crash/content-length HTTP/1.1\r\nHost: localhost\r\nContent-Length: 5000,5000\r\n\r\n", Huge]},
{505, ResponsePacket},
{408, "GET / HTTP/1.1\r\n"},
{408, "GET / HTTP/1.1\r\nHost: localhost"},