Merge branch 'scrapinghub/not_streamed_once' into maint

* scrapinghub/not_streamed_once:
  inets: fix {self, once} for not streamed request
  inets: send correct nonstreamed response with streaming

OTP-13093

2 files changed, 86 insertions, 28 deletions

diff --git a/lib/inets/src/http_client/httpc_handler.erl b/lib/inets/src/http_client/httpc_handler.erl
index 6e6cc38c06..1044cffe6f 100644
--- a/lib/inets/src/http_client/httpc_handler.erl
+++ b/lib/inets/src/http_client/httpc_handler.erl
@@ -26,6 +26,7 @@
 -include_lib("inets/src/http_lib/http_internal.hrl").
 -include("httpc_internal.hrl").
 
+-define(IS_STREAMED(Code), ((Code =:= 200) orelse (Code =:= 206))).
 
 %%--------------------------------------------------------------------
 %% Internal Application API
@@ -163,22 +164,22 @@ info(Pid) ->
 %% Request should not be streamed
 stream(BodyPart, #request{stream = none} = Request, _) ->
     ?hcrt("stream - none", []),
-    {BodyPart, Request};
+    {false, BodyPart, Request};
 
 %% Stream to caller
 stream(BodyPart, #request{stream = Self} = Request, Code) 
-  when ((Code =:= 200) orelse  (Code =:= 206)) andalso 
+  when ?IS_STREAMED(Code) andalso
        ((Self =:= self) orelse (Self =:= {self, once})) ->
     ?hcrt("stream - self", [{stream, Self}, {code, Code}]),
     httpc_response:send(Request#request.from, 
                         {Request#request.id, stream, BodyPart}),
-    {<<>>, Request};
+    {true, <<>>, Request};
 
 %% Stream to file
 %% This has been moved to start_stream/3
 %% We keep this for backward compatibillity...
 stream(BodyPart, #request{stream = Filename} = Request, Code)
-  when ((Code =:= 200) orelse (Code =:= 206)) andalso is_list(Filename) -> 
+  when ?IS_STREAMED(Code) andalso is_list(Filename) ->
     ?hcrt("stream - filename", [{stream, Filename}, {code, Code}]),
     case file:open(Filename, [write, raw, append, delayed_write]) of
         {ok, Fd} ->
@@ -190,18 +191,18 @@ stream(BodyPart, #request{stream = Filename} = Request, Code)
 
 %% Stream to file
 stream(BodyPart, #request{stream = Fd} = Request, Code)  
-  when ((Code =:= 200) orelse (Code =:= 206)) -> 
+  when ?IS_STREAMED(Code) ->
     ?hcrt("stream to file", [{stream, Fd}, {code, Code}]),
     case file:write(Fd, BodyPart) of
         ok ->
-            {<<>>, Request};
+            {true, <<>>, Request};
         {error, Reason} ->
             exit({stream_to_file_failed, Reason})
     end;
 
 stream(BodyPart, Request,_) -> % only 200 and 206 responses can be streamed
     ?hcrt("stream - ignore", [{request, Request}]),
-    {BodyPart, Request}.
+    {false, BodyPart, Request}.
 
 
 %%====================================================================
@@ -474,18 +475,18 @@ handle_info({Proto, _Socket, Data},
             {Module, whole_body, [Body, Length]} ->
                 ?hcrd("data processed - whole body", [{length, Length}]),
                 {_, Code, _} = StatusLine,
-                {NewBody, NewRequest} = stream(Body, Request, Code),
+                {Streamed, NewBody, NewRequest} = stream(Body, Request, Code),
                 %% When we stream we will not keep the already
                 %% streamed data, that would be a waste of memory.
                 NewLength = 
-                    case Stream of
-                        none ->   
+                    case Streamed of
+                        false ->
                             Length;
-                        _ ->
+                        true ->
                             Length - size(Body)                     
                     end,
                 
-                NewState = next_body_chunk(State),
+                NewState = next_body_chunk(State, Code),
                 NewMFA   = {Module, whole_body, [NewBody, NewLength]}, 
                 {noreply, NewState#state{mfa     = NewMFA,
                                          request = NewRequest}};
@@ -497,8 +498,8 @@ handle_info({Proto, _Socket, Data},
                 %% The response body is chunk-encoded. Steal decoded
                 %% chunks as much as possible to stream.
                 {_, Code, _} = StatusLine,
-                {NewBody, NewRequest} = stream(BodySoFar, Request, Code),
-                NewState = next_body_chunk(State),
+                {_, NewBody, NewRequest} = stream(BodySoFar, Request, Code),
+                NewState = next_body_chunk(State, Code),
                 NewMFA   = {Module, decode_size,
                             [TotalChunk, HexList,
                              {MaxBodySize, NewBody, AccLength, MaxHeaderSize}]},
@@ -517,8 +518,8 @@ handle_info({Proto, _Socket, Data},
                 NewChunkSize = ChunkSize - ChunkSizeToSteal,
                 {_, Code, _} = StatusLine,
 
-                {NewBody, NewRequest} = stream(StolenBody, Request, Code),
-                NewState = next_body_chunk(State),
+                {_, NewBody, NewRequest} = stream(StolenBody, Request, Code),
+                NewState = next_body_chunk(State, Code),
                 NewMFA   = {Module, decode_data,
                             [NewChunkSize, NewTotalChunk,
                              {MaxBodySize, NewBody, AccLength, MaxHeaderSize}]},
@@ -1071,13 +1072,13 @@ handle_http_msg({ChunkedHeaders, Body},
     ?hcrt("handle_http_msg", 
 	  [{chunked_headers, ChunkedHeaders}, {headers, Headers}]),
     NewHeaders = http_chunk:handle_headers(Headers, ChunkedHeaders),
-    {NewBody, NewRequest} = stream(Body, State#state.request, Code),
+    {_, NewBody, NewRequest} = stream(Body, State#state.request, Code),
     handle_response(State#state{headers = NewHeaders,
                                 body    = NewBody,
                                 request = NewRequest});
 handle_http_msg(Body, #state{status_line = {_,Code, _}} = State) ->
     ?hcrt("handle_http_msg", [{code, Code}]),
-    {NewBody, NewRequest} = stream(Body, State#state.request, Code),
+    {_, NewBody, NewRequest} = stream(Body, State#state.request, Code),
     handle_response(State#state{body = NewBody, request = NewRequest}).
 
 handle_http_body(_, #state{status = {ssl_tunnel, _},
@@ -1119,7 +1120,7 @@ handle_http_body(Body, #state{headers       = Headers,
 			  [{module,   Module}, 
 			   {function, Function}, 
 			   {args,     Args}]),
-		    NewState = next_body_chunk(State),
+		    NewState = next_body_chunk(State, Code),
 		    {noreply, NewState#state{mfa = 
 					     {Module, Function, Args}}};
 		{ok, {ChunkedHeaders, NewBody}} ->
@@ -1133,7 +1134,7 @@ handle_http_body(Body, #state{headers       = Headers,
                            handle_response(State#state{headers = NewHeaders,
                                                 body    = NewBody});
                         _ ->
-                           {NewBody2, _NewRequest} =
+                           {_, NewBody2, _} =
                                 stream(NewBody, Request, Code),
                            handle_response(State#state{headers = NewHeaders,
                                        body    = NewBody2})
@@ -1147,12 +1148,12 @@ handle_http_body(Body, #state{headers       = Headers,
                 true ->
                     case httpc_response:whole_body(Body, Length) of
                         {ok, Body} ->
-			    {NewBody, NewRequest} = 
+			    {_, NewBody, NewRequest} =
 				stream(Body, Request, Code),
 			    handle_response(State#state{body    = NewBody,
 							request = NewRequest});
                         MFA ->
-			    NewState = next_body_chunk(State),   
+			    NewState = next_body_chunk(State, Code),
 			    {noreply, NewState#state{mfa = MFA}}
 		    end;
                 false ->
@@ -1646,21 +1647,21 @@ start_stream({_Version, _Code, _ReasonPhrase}, _Headers,
     {ok, Request};
 start_stream({_Version, Code, _ReasonPhrase}, Headers, 
 	     #request{stream = self} = Request) 
-  when (Code =:= 200) orelse (Code =:= 206) ->
+  when ?IS_STREAMED(Code) ->
     ?hcrt("start stream - self", [{code, Code}]), 
     Msg = httpc_response:stream_start(Headers, Request, ignore),
     httpc_response:send(Request#request.from, Msg),
     {ok, Request};
 start_stream({_Version, Code, _ReasonPhrase}, Headers, 
 	     #request{stream = {self, once}} = Request) 
-  when (Code =:= 200) orelse (Code =:= 206) ->
+  when ?IS_STREAMED(Code) ->
     ?hcrt("start stream - self:once", [{code, Code}]), 
     Msg = httpc_response:stream_start(Headers, Request, self()),
     httpc_response:send(Request#request.from, Msg),
     {ok, Request};    
 start_stream({_Version, Code, _ReasonPhrase}, _Headers, 
 	     #request{stream = Filename} = Request)
-  when ((Code =:= 200) orelse (Code =:= 206)) andalso is_list(Filename) ->
+  when ?IS_STREAMED(Code) andalso is_list(Filename) ->
     ?hcrt("start stream", [{code, Code}, {filename, Filename}]),
     case file:open(Filename, [write, raw, append, delayed_write]) of
         {ok, Fd} ->
@@ -1712,13 +1713,15 @@ end_stream(SL, R) ->
 
 next_body_chunk(#state{request = #request{stream = {self, once}}, 
 		       once    = once, 
-		       session = Session} = State) ->
+		       session = Session} = State,
+		Code) when ?IS_STREAMED(Code) ->
     activate_once(Session), 
     State#state{once = inactive};
 next_body_chunk(#state{request = #request{stream = {self, once}}, 
-		       once = inactive} = State) ->
+		       once = inactive} = State,
+		Code) when ?IS_STREAMED(Code) ->
     State; %% Wait for user to call stream_next
-next_body_chunk(#state{session = Session} = State) ->
+next_body_chunk(#state{session = Session} = State, _) ->
     activate_once(Session), 
     State.
 
diff --git a/lib/inets/test/httpc_SUITE.erl b/lib/inets/test/httpc_SUITE.erl
index 2ad00bdf76..989563cdbc 100644
--- a/lib/inets/test/httpc_SUITE.erl
+++ b/lib/inets/test/httpc_SUITE.erl
@@ -98,6 +98,8 @@ only_simulated() ->
      stream_once,
      stream_single_chunk,
      stream_no_length,
+     not_streamed_once,
+     stream_large_not_200_or_206,
      no_content_204,
      tolerate_missing_CR,
      userinfo,
@@ -408,6 +410,21 @@ stream_no_length(Config) when is_list(Config) ->
     stream_test(Request1, {stream, self}),
     Request2 = {url(group_name(Config), "/http_1_0_no_length_multiple.html", Config), []},
     stream_test(Request2, {stream, self}).
+%%-------------------------------------------------------------------------
+stream_large_not_200_or_206() ->
+    [{doc, "Test the option stream for large responses with status codes "
+      "other than 200 or 206" }].
+stream_large_not_200_or_206(Config) when is_list(Config) ->
+    Request = {url(group_name(Config), "/large_404_response.html", Config), []},
+    {404, _} = not_streamed_test(Request, {stream, self}).
+%%-------------------------------------------------------------------------
+not_streamed_once() ->
+    [{doc, "Test not streamed responses with once streaming"}].
+not_streamed_once(Config) when is_list(Config) ->
+    Request0 = {url(group_name(Config), "/404.html", Config), []},
+    {404, _} = not_streamed_test(Request0, {stream, {self, once}}),
+    Request1 = {url(group_name(Config), "/404_chunked.html", Config), []},
+    {404, _} = not_streamed_test(Request1, {stream, {self, once}}).
 
 
 %%-------------------------------------------------------------------------
@@ -1117,6 +1134,19 @@ stream_test(Request, To) ->
 
     Body = binary_to_list(StreamedBody).
 
+not_streamed_test(Request, To) ->
+    {ok, {{_,Code,_}, [_ | _], Body}} =
+	httpc:request(get, Request, [], [{body_format, binary}]),
+    {ok, RequestId} =
+	httpc:request(get, Request, [], [{body_format, binary}, {sync, false}, To]),
+
+    receive
+	{http, {RequestId, {{_, Code, _}, _Headers, Body}}} ->
+	    {Code, binary_to_list(Body)};
+	{http, Msg} ->
+	    ct:fail(Msg)
+    end.
+
 url(http, End, Config) ->
     Port = ?config(port, Config),
     {ok,Host} = inet:gethostname(),
@@ -1648,6 +1678,11 @@ handle_uri(_,"/307.html",Port,_,Socket,_) ->
 	"Content-Length:" ++ integer_to_list(length(Body))
 	++ "\r\n\r\n" ++ Body;
 
+handle_uri(_,"/404.html",_,_,_,_) ->
+    "HTTP/1.1 404 not found\r\n" ++
+	"Content-Length:14\r\n\r\n" ++
+	"Page not found";
+
 handle_uri(_,"/500.html",_,_,_,_) ->
     "HTTP/1.1 500 Internal Server Error\r\n" ++
 	"Content-Length:47\r\n\r\n" ++
@@ -1783,6 +1818,15 @@ handle_uri(_,"/once_chunked.html",_,_,Socket,_) ->
 	 http_chunk:encode("obar</BODY></HTML>")),
     http_chunk:encode_last();
 
+handle_uri(_,"/404_chunked.html",_,_,Socket,_) ->
+    Head =  "HTTP/1.1 404 not found\r\n" ++
+	"Transfer-Encoding:Chunked\r\n\r\n",
+    send(Socket, Head),
+    send(Socket, http_chunk:encode("<HTML><BODY>Not ")),
+    send(Socket,
+	 http_chunk:encode("found</BODY></HTML>")),
+    http_chunk:encode_last();
+
 handle_uri(_,"/single_chunk.html",_,_,Socket,_) ->
     Chunk =  "HTTP/1.1 200 ok\r\n" ++
         "Transfer-Encoding:Chunked\r\n\r\n" ++
@@ -1807,6 +1851,17 @@ handle_uri(_,"/http_1_0_no_length_multiple.html",_,_,Socket,_) ->
     send(Socket, string:copies("other multiple packets ", 200)),
     close(Socket);
 
+handle_uri(_,"/large_404_response.html",_,_,Socket,_) ->
+    %% long body to make sure it will be sent in multiple tcp packets
+    Body = string:copies("other multiple packets ", 200),
+    Head = io_lib:format("HTTP/1.1 404 not found\r\n"
+                         "Content-length: ~B\r\n"
+                         "Content-type: text/plain\r\n\r\n",
+                         [length(Body)]),
+    send(Socket, Head),
+    send(Socket, Body),
+    close(Socket);
+
 handle_uri(_,"/once.html",_,_,Socket,_) ->
     Head =  "HTTP/1.1 200 ok\r\n" ++
 	"Content-Length:32\r\n\r\n",