diff options
author | Luca Favatella <[email protected]> | 2018-03-16 18:00:00 +0000 |
---|---|---|
committer | Luca Favatella <[email protected]> | 2018-03-23 17:02:53 +0000 |
commit | 9a19dc099198585bd4f019116663d5b8be506ce0 (patch) | |
tree | 742ef416e52af1c37f92814ccf2735f62a1674b8 /lib/inets | |
parent | cef626165bc62edcc78c3be0c2cb107ae961b805 (diff) | |
download | otp-9a19dc099198585bd4f019116663d5b8be506ce0.tar.gz otp-9a19dc099198585bd4f019116663d5b8be506ce0.tar.bz2 otp-9a19dc099198585bd4f019116663d5b8be506ce0.zip |
inets: Teach httpc to honour `Connection: close` from server
From https://tools.ietf.org/html/rfc7230#section-6.6
> A client that receives a "close" connection option MUST cease
sending requests on that connection and close the connection after
reading the response message containing the "close"; if additional
pipelined requests had been sent on the connection, the client
SHOULD NOT assume that they will be processed by the server.
Notes on tests:
* The new tests are added only in group sim_http and not sim_https
because the same test approach appears to be not valid because when
processing the first response the server already sent data (> 0) for
the TLS/SSL handshake;
* The order of tests is relevant as it appears some test cases leave
reusable sessions behind.
Diffstat (limited to 'lib/inets')
-rw-r--r-- | lib/inets/src/http_client/httpc_handler.erl | 32 | ||||
-rw-r--r-- | lib/inets/test/httpc_SUITE.erl | 95 |
2 files changed, 117 insertions, 10 deletions
diff --git a/lib/inets/src/http_client/httpc_handler.erl b/lib/inets/src/http_client/httpc_handler.erl index 0ca8155054..ca29523f84 100644 --- a/lib/inets/src/http_client/httpc_handler.erl +++ b/lib/inets/src/http_client/httpc_handler.erl @@ -1043,15 +1043,29 @@ handle_response(#state{status = new} = State) -> ?hcrd("handle response - status = new", []), handle_response(try_to_enable_pipeline_or_keep_alive(State)); -handle_response(#state{request = Request, - status = Status, - session = Session, - status_line = StatusLine, - headers = Headers, - body = Body, - options = Options, - profile_name = ProfileName} = State) - when Status =/= new -> +handle_response(#state{status = Status0} = State0) when Status0 =/= new -> + Status = + case Status0 of + close -> close; + _ -> + case State0#state.headers of + undefined -> Status0; + _ -> + case httpc_response:is_server_closing( + State0#state.headers) of + true -> close; + false -> Status0 + end + end + end, + State = State0#state{status = Status}, + #state{request = Request, + session = Session, + status_line = StatusLine, + headers = Headers, + body = Body, + options = Options, + profile_name = ProfileName} = State, handle_cookies(Headers, Request, Options, ProfileName), case httpc_response:result({StatusLine, Headers, Body}, Request) of %% 100-continue diff --git a/lib/inets/test/httpc_SUITE.erl b/lib/inets/test/httpc_SUITE.erl index c8eaa4b3df..287db81bad 100644 --- a/lib/inets/test/httpc_SUITE.erl +++ b/lib/inets/test/httpc_SUITE.erl @@ -67,7 +67,7 @@ groups() -> %% process_leak_on_keepalive is depending on stream_fun_server_close %% and it shall be the last test case in the suite otherwise cookie %% will fail. - {sim_http, [], only_simulated() ++ [process_leak_on_keepalive]}, + {sim_http, [], only_simulated() ++ server_closing_connection() ++ [process_leak_on_keepalive]}, {http_internal, [], real_requests_esi()}, {http_unix_socket, [], simulated_unix_socket()}, {https, [], real_requests()}, @@ -154,6 +154,12 @@ only_simulated() -> stream_fun_server_close ]. +server_closing_connection() -> + [ + server_closing_connection_on_first_response, + server_closing_connection_on_second_response + ]. + misc() -> [ server_does_not_exist, @@ -252,6 +258,24 @@ end_per_testcase(pipeline, _Config) -> inets:stop(httpc, pipeline); end_per_testcase(persistent_connection, _Config) -> inets:stop(httpc, persistent); +end_per_testcase(Case, Config) + when Case == server_closing_connection_on_first_response; + Case == server_closing_connection_on_second_response -> + %% Test case uses at most one session. Ensure no leftover + %% sessions left behind. + {_, Status} = proplists:lookup(tc_status, Config), + ShallCleanup = case Status of + ok -> true; + {failed, _} -> true; + {skipped, _} -> false + end, + if ShallCleanup -> + httpc:request(url(group_name(Config), "/just_close.html", Config)), + ok; + true -> + ct:pal("Not cleaning up because test case status was ~p", [Status]), + ok + end; end_per_testcase(_Case, _Config) -> ok. @@ -1275,6 +1299,53 @@ stream_fun_server_close(Config) when is_list(Config) -> end. %%-------------------------------------------------------------------- +server_closing_connection_on_first_response() -> + [{doc, "Client receives \"Connection: close\" on first response." + "A client that receives a \"close\" connection option MUST cease sending" + "requests on that connection and close the connection after reading" + "the response message containing the \"close\""}]. +server_closing_connection_on_first_response(Config) when is_list(Config) -> + ReqSrvSendOctFun = + fun(V, U, S) -> + {ok, {{V, S, _}, Headers0, []}} = + httpc:request(get, {U, []}, [{version, V}], []), + {_, SendOctStr} = + proplists:lookup("x-socket-stat-send-oct", Headers0), + list_to_integer(SendOctStr) + end, + V = "HTTP/1.1", + Url0 = url(group_name(Config), "/http_1_1_send_oct.html", Config), + Url1 = url(group_name(Config), "/http_1_1_send_oct_and_connection_close.html", Config), + %% Test case assumes at most one reusable past session. + _ = ReqSrvSendOctFun(V, Url1, 204), + 0 = ReqSrvSendOctFun(V, Url0, 204), + ok. + +%%-------------------------------------------------------------------- +server_closing_connection_on_second_response() -> + [{doc, "Client receives \"Connection: close\" on second response." + "A client that receives a \"close\" connection option MUST cease sending" + "requests on that connection and close the connection after reading" + "the response message containing the \"close\""}]. +server_closing_connection_on_second_response(Config) when is_list(Config) -> + ReqSrvSendOctFun = + fun(V, U, S) -> + {ok, {{V, S, _}, Headers0, []}} = + httpc:request(get, {U, []}, [{version, V}], []), + {_, SendOctStr} = + proplists:lookup("x-socket-stat-send-oct", Headers0), + list_to_integer(SendOctStr) + end, + V = "HTTP/1.1", + Url0 = url(group_name(Config), "/http_1_1_send_oct.html", Config), + Url1 = url(group_name(Config), "/http_1_1_send_oct_and_connection_close.html", Config), + %% Test case assumes no reusable past sessions. + SendOct0 = 0 = ReqSrvSendOctFun(V, Url0, 204), + case ReqSrvSendOctFun(V, Url1, 204) of SendOct1 when SendOct1 > SendOct0 -> ok end, + 0 = ReqSrvSendOctFun(V, Url0, 204), + ok. + +%%-------------------------------------------------------------------- slow_connection() -> [{doc, "Test that a request on a slow keep-alive connection won't crash the httpc_manager"}]. slow_connection(Config) when is_list(Config) -> @@ -2232,10 +2303,32 @@ handle_uri("GET","/v1/kv/foo",_,_,_,_) -> "Content-Length: 24\r\n" ++ "Content-Type: application/json\r\n\r\n" ++ "[{\"Value\": \"aGVsbG8=\"}]\n"; +handle_uri(_,"/http_1_1_send_oct.html",_,_,Socket,_) -> + "HTTP/1.1 204 No Content\r\n" ++ + "X-Socket-Stat-Send-Oct: " ++ integer_to_list(get_stat(Socket, send_oct)) ++ "\r\n" ++ + "\r\n"; +handle_uri(_,"/http_1_1_send_oct_and_connection_close.html",_,_,Socket,_) -> + "HTTP/1.1 204 No Content\r\n" ++ + "X-Socket-Stat-Send-Oct: " ++ integer_to_list(get_stat(Socket, send_oct)) ++ "\r\n" ++ + "Connection: close\r\n" ++ + "\r\n"; handle_uri(_,_,_,_,_,DefaultResponse) -> DefaultResponse. +get_stat(S, Opt) -> + case getstat(S, [Opt]) of + {ok, [{Opt, V}]} when is_integer(V) -> + V; + {error, _} = E -> + E + end. + +getstat(#sslsocket{} = S, Opts) -> + ssl:getstat(S, Opts); +getstat(S, Opts) -> + inet:getstat(S, Opts). + url_start(#sslsocket{}) -> {ok,Host} = inet:gethostname(), ?TLS_URL_START ++ Host ++ ":"; |