From f4b02cf574552f40ff354261b4c7cf02cb568212 Mon Sep 17 00:00:00 2001 From: Kirilll Zaborsky Date: Tue, 20 Jan 2015 21:06:13 +0300 Subject: inets: send correct nonstreamed response with streaming httpc_handler should respond with correct and complete responses seeing non-streamed status codes i.e. codes other than 200 or 206. --- lib/inets/src/http_client/httpc_handler.erl | 28 ++++++++++++------------- lib/inets/test/httpc_SUITE.erl | 32 +++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/lib/inets/src/http_client/httpc_handler.erl b/lib/inets/src/http_client/httpc_handler.erl index 6e6cc38c06..0300a80b03 100644 --- a/lib/inets/src/http_client/httpc_handler.erl +++ b/lib/inets/src/http_client/httpc_handler.erl @@ -163,7 +163,7 @@ 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) @@ -172,7 +172,7 @@ stream(BodyPart, #request{stream = Self} = Request, Code) ?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 @@ -194,14 +194,14 @@ stream(BodyPart, #request{stream = Fd} = Request, 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,14 +474,14 @@ 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, @@ -497,7 +497,7 @@ 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), + {_, NewBody, NewRequest} = stream(BodySoFar, Request, Code), NewState = next_body_chunk(State), NewMFA = {Module, decode_size, [TotalChunk, HexList, @@ -517,7 +517,7 @@ handle_info({Proto, _Socket, Data}, NewChunkSize = ChunkSize - ChunkSizeToSteal, {_, Code, _} = StatusLine, - {NewBody, NewRequest} = stream(StolenBody, Request, Code), + {_, NewBody, NewRequest} = stream(StolenBody, Request, Code), NewState = next_body_chunk(State), NewMFA = {Module, decode_data, [NewChunkSize, NewTotalChunk, @@ -1071,13 +1071,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, _}, @@ -1133,7 +1133,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,7 +1147,7 @@ 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}); diff --git a/lib/inets/test/httpc_SUITE.erl b/lib/inets/test/httpc_SUITE.erl index 2ad00bdf76..cc0fa5d4d9 100644 --- a/lib/inets/test/httpc_SUITE.erl +++ b/lib/inets/test/httpc_SUITE.erl @@ -98,6 +98,7 @@ only_simulated() -> stream_once, stream_single_chunk, stream_no_length, + stream_large_not_200_or_206, no_content_204, tolerate_missing_CR, userinfo, @@ -408,6 +409,13 @@ 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,_}, _, _} = non_streamed_async_test(Request, {stream, self}). %%------------------------------------------------------------------------- @@ -1117,6 +1125,19 @@ stream_test(Request, To) -> Body = binary_to_list(StreamedBody). +non_streamed_async_test(Request, To) -> + {ok, Response} = + httpc:request(get, Request, [], [{body_format, binary}]), + {ok, RequestId} = + httpc:request(get, Request, [], [{sync, false}, To]), + + receive + {http, {RequestId, Response}} -> + Response; + {http, Msg} -> + ct:fail(Msg) + end. + url(http, End, Config) -> Port = ?config(port, Config), {ok,Host} = inet:gethostname(), @@ -1807,6 +1828,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", -- cgit v1.2.3 From 45251cd60ebad4ef08ffc7cb0797d1fd7a603eea Mon Sep 17 00:00:00 2001 From: Kirilll Zaborsky Date: Wed, 23 Sep 2015 16:51:33 +0300 Subject: inets: fix {self, once} for not streamed request httpc should work properly if streaming option {self, once} is chosen and the corresponding response does not get streamed. --- lib/inets/src/http_client/httpc_handler.erl | 31 +++++++++++---------- lib/inets/test/httpc_SUITE.erl | 43 ++++++++++++++++++++++------- 2 files changed, 50 insertions(+), 24 deletions(-) diff --git a/lib/inets/src/http_client/httpc_handler.erl b/lib/inets/src/http_client/httpc_handler.erl index 0300a80b03..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 @@ -167,7 +168,7 @@ stream(BodyPart, #request{stream = none} = 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, @@ -178,7 +179,7 @@ stream(BodyPart, #request{stream = Self} = Request, Code) %% 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,7 +191,7 @@ 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 -> @@ -485,7 +486,7 @@ handle_info({Proto, _Socket, Data}, 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}}; @@ -498,7 +499,7 @@ handle_info({Proto, _Socket, Data}, %% chunks as much as possible to stream. {_, Code, _} = StatusLine, {_, NewBody, NewRequest} = stream(BodySoFar, Request, Code), - NewState = next_body_chunk(State), + NewState = next_body_chunk(State, Code), NewMFA = {Module, decode_size, [TotalChunk, HexList, {MaxBodySize, NewBody, AccLength, MaxHeaderSize}]}, @@ -518,7 +519,7 @@ handle_info({Proto, _Socket, Data}, {_, Code, _} = StatusLine, {_, NewBody, NewRequest} = stream(StolenBody, Request, Code), - NewState = next_body_chunk(State), + NewState = next_body_chunk(State, Code), NewMFA = {Module, decode_data, [NewChunkSize, NewTotalChunk, {MaxBodySize, NewBody, AccLength, MaxHeaderSize}]}, @@ -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}} -> @@ -1152,7 +1153,7 @@ handle_http_body(Body, #state{headers = Headers, 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 cc0fa5d4d9..989563cdbc 100644 --- a/lib/inets/test/httpc_SUITE.erl +++ b/lib/inets/test/httpc_SUITE.erl @@ -98,6 +98,7 @@ 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, @@ -415,7 +416,15 @@ stream_large_not_200_or_206() -> "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,_}, _, _} = non_streamed_async_test(Request, {stream, self}). + {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}}). %%------------------------------------------------------------------------- @@ -1125,18 +1134,18 @@ stream_test(Request, To) -> Body = binary_to_list(StreamedBody). -non_streamed_async_test(Request, To) -> - {ok, Response} = +not_streamed_test(Request, To) -> + {ok, {{_,Code,_}, [_ | _], Body}} = httpc:request(get, Request, [], [{body_format, binary}]), {ok, RequestId} = - httpc:request(get, Request, [], [{sync, false}, To]), + httpc:request(get, Request, [], [{body_format, binary}, {sync, false}, To]), - receive - {http, {RequestId, Response}} -> - Response; - {http, Msg} -> - ct:fail(Msg) - end. + 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), @@ -1669,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" ++ @@ -1804,6 +1818,15 @@ handle_uri(_,"/once_chunked.html",_,_,Socket,_) -> http_chunk:encode("obar")), 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("Not ")), + send(Socket, + http_chunk:encode("found")), + 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" ++ -- cgit v1.2.3