From 21317093285190f854acaf223db2e08f79069757 Mon Sep 17 00:00:00 2001 From: Bartek Walkowicz Date: Thu, 1 Mar 2018 14:49:57 +0100 Subject: Add case for handling infinity for idle/request_timeout Currently cowboy assumes that idle_timeout or request_timeout is a number and always starts timers. Similar situation takes place in case of preface_timeout for http2. This commit adds case for handling infinity as a timeout, allowing to not start mentioned timers. --- src/cowboy_http.erl | 6 ++++-- src/cowboy_http2.erl | 13 +++++++++---- test/http2_SUITE.erl | 17 +++++++++++++++++ test/http_SUITE.erl | 50 +++++++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 79 insertions(+), 7 deletions(-) diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl index 6a75a1a..2f30279 100644 --- a/src/cowboy_http.erl +++ b/src/cowboy_http.erl @@ -222,8 +222,10 @@ set_timeout(State0=#state{opts=Opts, streams=Streams}) -> [] -> {request_timeout, 5000}; _ -> {idle_timeout, 60000} end, - Timeout = maps:get(Name, Opts, Default), - TimerRef = erlang:start_timer(Timeout, self(), Name), + TimerRef = case maps:get(Name, Opts, Default) of + infinity -> undefined; + Timeout -> erlang:start_timer(Timeout, self(), Name) + end, State#state{timer=TimerRef}. cancel_timeout(State=#state{timer=TimerRef}) -> diff --git a/src/cowboy_http2.erl b/src/cowboy_http2.erl index b21d6da..caa1f07 100644 --- a/src/cowboy_http2.erl +++ b/src/cowboy_http2.erl @@ -122,7 +122,7 @@ %% is established and continues normally. An exception is when a HEADERS %% frame is sent followed by CONTINUATION frames: no other frame can be %% sent in between. - parse_state = undefined :: {preface, sequence, reference()} + parse_state = undefined :: {preface, sequence, undefined | reference()} | {preface, settings, reference()} | normal | {continuation, cowboy_stream:streamid(), cowboy_stream:fin(), binary()}, @@ -203,8 +203,10 @@ preface(#state{socket=Socket, transport=Transport, next_settings=Settings}) -> Transport:send(Socket, cow_http2:settings(Settings)). preface_timeout(Opts) -> - PrefaceTimeout = maps:get(preface_timeout, Opts, 5000), - erlang:start_timer(PrefaceTimeout, self(), preface_timeout). + case maps:get(preface_timeout, Opts, 5000) of + infinity -> undefined; + PrefaceTimeout -> erlang:start_timer(PrefaceTimeout, self(), preface_timeout) + end. %% @todo Add the timeout for last time since we heard of connection. before_loop(State, Buffer) -> @@ -305,7 +307,10 @@ parse(State=#state{local_settings=#{max_frame_size := MaxFrameSize}, end. parse_settings_preface(State, Frame={settings, _}, Rest, TRef) -> - _ = erlang:cancel_timer(TRef, [{async, true}, {info, false}]), + ok = case TRef of + undefined -> ok; + _ -> erlang:cancel_timer(TRef, [{async, true}, {info, false}]) + end, parse(frame(State#state{parse_state=normal}, Frame), Rest); parse_settings_preface(State, _, _, _) -> terminate(State, {connection_error, protocol_error, diff --git a/test/http2_SUITE.erl b/test/http2_SUITE.erl index 8ffe2fd..933a2b2 100644 --- a/test/http2_SUITE.erl +++ b/test/http2_SUITE.erl @@ -59,6 +59,23 @@ inactivity_timeout(Config) -> {ok, << _:24, 7:8, _:72, 2:32 >>} = gen_tcp:recv(Socket, 17, 1000), ok. +preface_timeout_infinity(Config) -> + doc("Ensure infinity for preface_timeout is accepted"), + ProtoOpts = #{ + env => #{dispatch => cowboy_router:compile(init_routes(Config))}, + preface_timeout => infinity + }, + {ok, Pid} = cowboy:start_clear(preface_timeout_infinity, [{port, 0}], ProtoOpts), + Ref = erlang:monitor(process, Pid), + Port = ranch:get_port(preface_timeout_infinity), + {ok, _} = do_handshake([{port, Port}|Config]), + receive + {'DOWN', Ref, process, Pid, Reason} -> + error(Reason) + after 1000 -> + cowboy:stop_listener(preface_timeout_infinity) + end. + resp_iolist_body(Config) -> doc("Regression test when response bodies are iolists that " "include improper lists, empty lists and empty binaries. " diff --git a/test/http_SUITE.erl b/test/http_SUITE.erl index d77b760..a971e45 100644 --- a/test/http_SUITE.erl +++ b/test/http_SUITE.erl @@ -35,13 +35,16 @@ all() -> {group, http_compress}, {group, https_compress}, {group, parse_host}, + {group, request_timeout_infinity}, + {group, idle_timeout_infinity}, {group, set_env}, {group, router_compile} ]. groups() -> Tests = ct_helper:all(?MODULE) -- [ - parse_host, set_env_dispatch, path_allow_colon + parse_host, set_env_dispatch, path_allow_colon, + request_timeout_infinity, idle_timeout_infinity ], [ {http, [], Tests}, %% @todo parallel @@ -51,6 +54,12 @@ groups() -> {parse_host, [], [ parse_host ]}, + {request_timeout_infinity, [], [ + request_timeout_infinity + ]}, + {idle_timeout_infinity, [], [ + idle_timeout_infinity + ]}, {set_env, [], [ set_env_dispatch ]}, @@ -84,6 +93,22 @@ init_per_group(parse_host, Config) -> }), Port = ranch:get_port(parse_host), [{type, tcp}, {protocol, http}, {port, Port}, {opts, []}|Config]; +init_per_group(request_timeout_infinity, Config) -> + Ref = request_timeout_infinity, + {ok, Pid} = cowboy:start_clear(Ref, [{port, 0}], #{ + env => #{dispatch => init_dispatch(Config)}, + request_timeout => infinity + }), + Port = ranch:get_port(request_timeout_infinity), + [{pid, Pid}, {ref, Ref}, {type, tcp}, {protocol, http}, {port, Port}, {opts, []}|Config]; +init_per_group(idle_timeout_infinity, Config) -> + Ref = idle_timeout_infinity, + {ok, Pid} = cowboy:start_clear(Ref, [{port, 0}], #{ + env => #{dispatch => init_dispatch(Config)}, + idle_timeout => infinity + }), + Port = ranch:get_port(idle_timeout_infinity), + [{pid, Pid}, {ref, Ref}, {type, tcp}, {protocol, http}, {port, Port}, {opts, []}|Config]; init_per_group(set_env, Config) -> {ok, _} = cowboy:start_clear(set_env, [{port, 0}], #{ env => #{dispatch => []} @@ -746,3 +771,26 @@ te_identity(Config) -> {response, nofin, 200, _} = gun:await(ConnPid, Ref), {ok, Body} = gun:await_body(ConnPid, Ref), ok. + +request_timeout_infinity(Config) -> + Pid = config(pid, Config), + Ref = erlang:monitor(process, Pid), + _ = gun_open(Config), + receive + {'DOWN', Ref, process, Pid, Reason} -> + error(Reason) + after 1000 -> + ok + end. + +idle_timeout_infinity(Config) -> + Pid = config(pid, Config), + Ref = erlang:monitor(process, Pid), + ConnPid = gun_open(Config), + gun:post(ConnPid, "/echo/body", [], <<"TEST">>), + receive + {'DOWN', Ref, process, Pid, Reason} -> + error(Reason) + after 1000 -> + ok + end. -- cgit v1.2.3