From 40b8d0befcd8b310a7a48e07df7d629153903cca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Mon, 11 Feb 2013 09:03:13 +0100 Subject: Better handle socket closing with loop handlers We now read from the socket to be able to detect errors or TCP close events, and buffer the data if any. Once the data receive goes over a certain limit, which defaults to 5000 bytes, we simply close the connection with an {error, overflow} reason. --- src/cowboy_handler.erl | 77 +++++++++++++++++++++++++++++--------- src/cowboy_req.erl | 6 +++ test/http_SUITE.erl | 16 ++++++++ test/http_handler_long_polling.erl | 2 + 4 files changed, 84 insertions(+), 17 deletions(-) diff --git a/src/cowboy_handler.erl b/src/cowboy_handler.erl index 7ed7db3..65e22b7 100644 --- a/src/cowboy_handler.erl +++ b/src/cowboy_handler.erl @@ -18,6 +18,16 @@ %% environment values. The result of this execution is added to the %% environment under the result value. %% +%% When using loop handlers, we are receiving data from the socket because we +%% want to know when the socket gets closed. This is generally not an issue +%% because these kinds of requests are generally not pipelined, and don't have +%% a body. If they do have a body, this body is often read in the +%% init/3 callback and this is no problem. Otherwise, this data +%% accumulates in a buffer until we reach a certain threshold of 5000 bytes +%% by default. This can be configured through the loop_max_buffer +%% environment value. The request will be terminated with an +%% {error, overflow} reason if this threshold is reached. +%% %% @see cowboy_http_handler -module(cowboy_handler). -behaviour(cowboy_middleware). @@ -28,8 +38,10 @@ -record(state, { env :: cowboy_middleware:env(), hibernate = false :: boolean(), + loop_buffer_size = 0 :: non_neg_integer(), + loop_max_buffer = 5000 :: non_neg_integer() | infinity, loop_timeout = infinity :: timeout(), - loop_timeout_ref :: undefined | reference(), + loop_timeout_ref = undefined :: undefined | reference(), resp_sent = false :: boolean() }). @@ -41,7 +53,12 @@ execute(Req, Env) -> {_, Handler} = lists:keyfind(handler, 1, Env), {_, HandlerOpts} = lists:keyfind(handler_opts, 1, Env), - handler_init(Req, #state{env=Env}, Handler, HandlerOpts). + case lists:keyfind(loop_max_buffer, 1, Env) of + false -> MaxBuffer = 5000, ok; + {_, MaxBuffer} -> ok + end, + handler_init(Req, #state{env=Env, loop_max_buffer=MaxBuffer}, + Handler, HandlerOpts). -spec handler_init(Req, #state{}, module(), any()) -> {ok, Req, cowboy_middleware:env()} @@ -53,17 +70,17 @@ handler_init(Req, State, Handler, HandlerOpts) -> {ok, Req2, HandlerState} -> handler_handle(Req2, State, Handler, HandlerState); {loop, Req2, HandlerState} -> - handler_before_loop(Req2, State#state{hibernate=false}, - Handler, HandlerState); + handler_before_loop(Req2, State, Handler, HandlerState); {loop, Req2, HandlerState, hibernate} -> handler_before_loop(Req2, State#state{hibernate=true}, Handler, HandlerState); {loop, Req2, HandlerState, Timeout} -> - handler_before_loop(Req2, State#state{loop_timeout=Timeout}, - Handler, HandlerState); + State2 = handler_loop_timeout(State#state{loop_timeout=Timeout}), + handler_before_loop(Req2, State2, Handler, HandlerState); {loop, Req2, HandlerState, Timeout, hibernate} -> - handler_before_loop(Req2, State#state{ - hibernate=true, loop_timeout=Timeout}, Handler, HandlerState); + State2 = handler_loop_timeout(State#state{ + hibernate=true, loop_timeout=Timeout}), + handler_before_loop(Req2, State2, Handler, HandlerState); {shutdown, Req2, HandlerState} -> terminate_request(Req2, State, Handler, HandlerState, {normal, shutdown}); @@ -123,12 +140,14 @@ handler_handle(Req, State, Handler, HandlerState) -> | {error, 500, Req} | {suspend, module(), function(), [any()]} when Req::cowboy_req:req(). handler_before_loop(Req, State=#state{hibernate=true}, Handler, HandlerState) -> - State2 = handler_loop_timeout(State), + [Socket, Transport] = cowboy_req:get([socket, transport], Req), + Transport:setopts(Socket, [{active, once}]), {suspend, ?MODULE, handler_loop, - [Req, State2#state{hibernate=false}, Handler, HandlerState]}; + [Req, State#state{hibernate=false}, Handler, HandlerState]}; handler_before_loop(Req, State, Handler, HandlerState) -> - State2 = handler_loop_timeout(State), - handler_loop(Req, State2, Handler, HandlerState). + [Socket, Transport] = cowboy_req:get([socket, transport], Req), + Transport:setopts(Socket, [{active, once}]), + handler_loop(Req, State, Handler, HandlerState). %% Almost the same code can be found in cowboy_websocket. -spec handler_loop_timeout(#state{}) -> #state{}. @@ -136,8 +155,10 @@ handler_loop_timeout(State=#state{loop_timeout=infinity}) -> State#state{loop_timeout_ref=undefined}; handler_loop_timeout(State=#state{loop_timeout=Timeout, loop_timeout_ref=PrevRef}) -> - _ = case PrevRef of undefined -> ignore; PrevRef -> - erlang:cancel_timer(PrevRef) end, + _ = case PrevRef of + undefined -> ignore; + PrevRef -> erlang:cancel_timer(PrevRef) + end, TRef = erlang:start_timer(Timeout, self(), ?MODULE), State#state{loop_timeout_ref=TRef}. @@ -146,16 +167,38 @@ handler_loop_timeout(State=#state{loop_timeout=Timeout, -> {ok, Req, cowboy_middleware:env()} | {error, 500, Req} | {suspend, module(), function(), [any()]} when Req::cowboy_req:req(). -handler_loop(Req, State=#state{loop_timeout_ref=TRef}, Handler, HandlerState) -> +handler_loop(Req, State=#state{loop_buffer_size=NbBytes, + loop_max_buffer=Threshold, loop_timeout_ref=TRef}, + Handler, HandlerState) -> + [Socket, Transport] = cowboy_req:get([socket, transport], Req), + {OK, Closed, Error} = Transport:messages(), receive + {OK, Socket, Data} -> + NbBytes2 = NbBytes + byte_size(Data), + if NbBytes2 > Threshold -> + _ = handler_terminate(Req, Handler, HandlerState, + {error, overflow}), + error_terminate(Req, State); + true -> + Req2 = cowboy_req:append_buffer(Data, Req), + State2 = handler_loop_timeout(State#state{ + loop_buffer_size=NbBytes2}), + handler_loop(Req2, State2, Handler, HandlerState) + end; + {Closed, Socket} -> + terminate_request(Req, State, Handler, HandlerState, + {error, closed}); + {Error, Socket, Reason} -> + terminate_request(Req, State, Handler, HandlerState, + {error, Reason}); {cowboy_req, resp_sent} -> - handler_loop(Req, State#state{resp_sent=true}, + handler_before_loop(Req, State#state{resp_sent=true}, Handler, HandlerState); {timeout, TRef, ?MODULE} -> terminate_request(Req, State, Handler, HandlerState, {normal, timeout}); {timeout, OlderTRef, ?MODULE} when is_reference(OlderTRef) -> - handler_loop(Req, State, Handler, HandlerState); + handler_before_loop(Req, State, Handler, HandlerState); Message -> handler_call(Req, State, Handler, HandlerState, Message) end. diff --git a/src/cowboy_req.erl b/src/cowboy_req.erl index c807a75..d416916 100644 --- a/src/cowboy_req.erl +++ b/src/cowboy_req.erl @@ -104,6 +104,7 @@ -export([ensure_response/2]). %% Private setter/getter API. +-export([append_buffer/2]). -export([get/2]). -export([set/2]). -export([set_bindings/4]). @@ -1065,6 +1066,11 @@ ensure_response(#http_req{socket=Socket, transport=Transport, %% Private setter/getter API. +%% @private +-spec append_buffer(binary(), Req) -> Req when Req::req(). +append_buffer(Suffix, Req=#http_req{buffer=Buffer}) -> + Req#http_req{buffer= << Buffer/binary, Suffix/binary >>}. + %% @private -spec get(atom(), req()) -> any(); ([atom()], req()) -> any(). get(List, Req) when is_list(List) -> diff --git a/test/http_SUITE.erl b/test/http_SUITE.erl index afe62c3..209be7e 100644 --- a/test/http_SUITE.erl +++ b/test/http_SUITE.erl @@ -49,6 +49,7 @@ -export([onresponse_crash/1]). -export([onresponse_reply/1]). -export([pipeline/1]). +-export([pipeline_long_polling/1]). -export([rest_bad_accept/1]). -export([rest_created_path/1]). -export([rest_expires/1]). @@ -112,6 +113,7 @@ groups() -> nc_rand, nc_zero, pipeline, + pipeline_long_polling, rest_bad_accept, rest_created_path, rest_expires, @@ -432,6 +434,8 @@ The document has moved here. ", Tests = [ + {102, <<"GET /long_polling HTTP/1.1\r\nHost: localhost\r\n" + "Content-Length: 5000\r\n\r\n", 0:5000/unit:8 >>}, {200, ["GET / HTTP/1.0\r\nHost: localhost\r\n" "Set-Cookie: ", HugeCookie, "\r\n\r\n"]}, {200, "\r\n\r\n\r\n\r\n\r\nGET / HTTP/1.1\r\nHost: localhost\r\n\r\n"}, @@ -449,6 +453,8 @@ The document has moved {408, "GET / HTTP/1.1\r\nHost: localhost\r\n\r"}, {414, Huge}, {400, "GET / HTTP/1.1\r\n" ++ Huge}, + {500, <<"GET /long_polling HTTP/1.1\r\nHost: localhost\r\n" + "Content-Length: 100000\r\n\r\n", 0:100000/unit:8 >>}, {505, "GET / HTTP/1.2\r\nHost: localhost\r\n\r\n"}, {closed, ""}, {closed, "\r\n"}, @@ -758,6 +764,16 @@ pipeline(Config) -> {ok, 200, _, Client11} = cowboy_client:response(Client10), {error, closed} = cowboy_client:response(Client11). +pipeline_long_polling(Config) -> + Client = ?config(client, Config), + {ok, Client2} = cowboy_client:request(<<"GET">>, + build_url("/long_polling", Config), Client), + {ok, Client3} = cowboy_client:request(<<"GET">>, + build_url("/long_polling", Config), Client2), + {ok, 102, _, Client4} = cowboy_client:response(Client3), + {ok, 102, _, Client5} = cowboy_client:response(Client4), + {error, closed} = cowboy_client:response(Client5). + rest_bad_accept(Config) -> Client = ?config(client, Config), {ok, Client2} = cowboy_client:request(<<"GET">>, diff --git a/test/http_handler_long_polling.erl b/test/http_handler_long_polling.erl index 763e1fe..e8cf610 100644 --- a/test/http_handler_long_polling.erl +++ b/test/http_handler_long_polling.erl @@ -19,4 +19,6 @@ info(timeout, Req, State) -> {loop, Req, State - 1, hibernate}. terminate({normal, shutdown}, _, _) -> + ok; +terminate({error, overflow}, _, _) -> ok. -- cgit v1.2.3