Merge pull request #1752 from lucafavatella/httpc-connection-close

Teach httpc to honour server connection close

4 files changed, 123 insertions, 43 deletions

diff --git a/lib/inets/doc/src/httpc.xml b/lib/inets/doc/src/httpc.xml
index 14662f257c..ffc6fec518 100644
--- a/lib/inets/doc/src/httpc.xml
+++ b/lib/inets/doc/src/httpc.xml
@@ -312,8 +312,7 @@
         <v>Body = string() | binary()</v>
         <v>Profile = profile() | pid()</v>
 	<d>When started <c>stand_alone</c> only the pid can be used.</d>
-        <v>Reason = {connect_failed, term()} | 
-                    {send_failed, term()} | term()</v>
+        <v>Reason = term()</v>
       </type>
 
       <desc>
diff --git a/lib/inets/src/http_client/httpc_handler.erl b/lib/inets/src/http_client/httpc_handler.erl
index 9b09832eb8..eeb08ce0ee 100644
--- a/lib/inets/src/http_client/httpc_handler.erl
+++ b/lib/inets/src/http_client/httpc_handler.erl
@@ -48,19 +48,17 @@
           queue_timer         :: reference() | 'undefined'
          }).
 
--type session_failed() :: {'connect_failed',term()} | {'send_failed',term()}.
-
 -record(state, 
         {
           request                   :: request() | 'undefined',
-          session                   :: session() | session_failed() | 'undefined',
+          session                   :: session() | 'undefined',
           status_line,               % {Version, StatusCode, ReasonPharse}
           headers                   :: http_response_h() | 'undefined',
           body                      :: binary() | 'undefined',
           mfa,                       % {Module, Function, Args}
           pipeline = queue:new()    :: queue:queue(),
           keep_alive = queue:new()  :: queue:queue(),
-          status,   % undefined | new | pipeline | keep_alive | close | {ssl_tunnel, Request}
+          status                    :: undefined | new | pipeline | keep_alive | close | {ssl_tunnel, request()},
           canceled = [],             % [RequestId]
           max_header_size = nolimit :: nolimit | integer(),
           max_body_size = nolimit   :: nolimit | integer(),
@@ -255,8 +253,8 @@ handle_call(Request, From, State) ->
 	Result ->
 	    Result
     catch
-	_:Reason ->
-	    {stop, {shutdown, Reason} , State}
+	Class:Reason:ST ->
+	    {stop, {shutdown, {{Class, Reason}, ST}}, State}
     end.		
 
 
@@ -271,8 +269,8 @@ handle_cast(Msg, State) ->
 	Result ->
 	    Result
     catch
-	_:Reason ->
-	    {stop, {shutdown, Reason} , State}
+	Class:Reason:ST ->
+	    {stop, {shutdown, {{Class, Reason}, ST}}, State}
     end.		
 
 %%--------------------------------------------------------------------
@@ -286,8 +284,8 @@ handle_info(Info, State) ->
 	Result ->
 	    Result
     catch
-	_:Reason ->
-	    {stop, {shutdown, Reason} , State}
+	Class:Reason:ST ->
+	    {stop, {shutdown, {{Class, Reason}, ST}}, State}
     end.		
 
 %%--------------------------------------------------------------------
@@ -295,23 +293,6 @@ handle_info(Info, State) ->
 %% Description: Shutdown the httpc_handler
 %%--------------------------------------------------------------------
 
-%% Init error there is no socket to be closed.
-terminate(normal, 
-          #state{request = Request, 
-                 session = {send_failed, _} = Reason} = State) ->
-    maybe_send_answer(Request, 
-                      httpc_response:error(Request, Reason), 
-                      State),
-    ok; 
-
-terminate(normal, 
-          #state{request = Request, 
-                 session = {connect_failed, _} = Reason} = State) ->
-    maybe_send_answer(Request, 
-                      httpc_response:error(Request, Reason), 
-                      State),
-    ok; 
-
 terminate(normal, #state{session = undefined}) ->
     ok;  
 
@@ -588,11 +569,11 @@ do_handle_info({Proto, _Socket, Data},
 	    activate_once(Session),
 	    {noreply, State#state{mfa = NewMFA}}
     catch
-	_:Reason ->
+	Class:Reason:ST ->
 	    ClientReason = {could_not_parse_as_http, Data}, 
 	    ClientErrMsg = httpc_response:error(Request, ClientReason),
 	    NewState     = answer_request(Request, ClientErrMsg, State),
-	    {stop, {shutdown, Reason}, NewState}
+	    {stop, {shutdown, {{Class, Reason}, ST}}, NewState}
     end;
 
 do_handle_info({Proto, Socket, Data}, 
@@ -1058,15 +1039,15 @@ 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 ->
+    State = handle_server_closing(State0),
+    #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
@@ -1330,6 +1311,14 @@ try_to_enable_pipeline_or_keep_alive(
 	    State#state{status = close}
     end.
 
+handle_server_closing(State = #state{status = close}) -> State;
+handle_server_closing(State = #state{headers = undefined}) -> State;
+handle_server_closing(State = #state{headers = Headers}) ->
+    case httpc_response:is_server_closing(Headers) of
+        true -> State#state{status = close};
+        false -> State
+    end.
+
 answer_request(#request{id = RequestId, from = From} = Request, Msg, 
 	       #state{session      = Session, 
 		      timers       = Timers, 
diff --git a/lib/inets/src/http_client/httpc_response.erl b/lib/inets/src/http_client/httpc_response.erl
index 58ab9144df..92dc9b0e02 100644
--- a/lib/inets/src/http_client/httpc_response.erl
+++ b/lib/inets/src/http_client/httpc_response.erl
@@ -83,7 +83,6 @@ whole_body(Body, Length) ->
 %% result(Response, Request) ->
 %%   Response - {StatusLine, Headers, Body}
 %%   Request - #request{}
-%%   Session - #tcp_session{}
 %%                                   
 %% Description: Checks the status code ...
 %%-------------------------------------------------------------------------
diff --git a/lib/inets/test/httpc_SUITE.erl b/lib/inets/test/httpc_SUITE.erl
index 38705372c9..1116fdb1b6 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,
@@ -233,7 +239,7 @@ init_per_testcase(pipeline, Config) ->
 init_per_testcase(persistent_connection, Config) ->
     inets:start(httpc, [{profile, persistent}]),
     httpc:set_options([{keep_alive_timeout, 50000},
-		       {max_keep_alive_length, 3}], persistent_connection),
+		       {max_keep_alive_length, 3}], persistent),
 
     Config;
 init_per_testcase(wait_for_whole_response, Config) ->
@@ -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 =:= true ->
+            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 ++ ":";