Clean up tests

Makefile

@@ -13,7 +13,22 @@ compile:
 clean:
 	$(REBAR) clean
 
-test:
+test: unit_tests old_tests eunit
+	@echo "====================================================="
+
+unit_tests:
+	@echo "====================================================="
+	@echo "Running tests..."
+	@cd test && make test && cd ..
+
+old_tests:
+	@echo "====================================================="
+	@echo "Running old tests..."
+	@cd test && make old_tests && cd ..
+
+eunit:
+	@echo "====================================================="
+	@echo "Running eunit tests..."
 	$(REBAR) eunit
 
 xref: all
@@ -30,4 +45,4 @@ install: compile
 	mkdir -p $(DESTDIR)/lib/ibrowse-$(IBROWSE_VSN)/
 	cp -r _build/lib/default/ibrowse/ebin $(DESTDIR)/lib/ibrowse-$(IBROWSE_VSN)/
 
-.PHONY: test docs
+.PHONY: test docs

src/ibrowse_http_client.erl

@@ -539,9 +539,9 @@ handle_sock_closed(#state{reply_buffer = Buf, reqs = Reqs, http_status_code = SC
                         true ->
                             {ok, Status_line, Raw_headers, Body, Raw_req};
                         false when Give_raw_req == false ->
-                            {ok, SC, Headers, Buf};
+                            {ok, SC, Headers, Body};
                         false ->
-                            {ok, SC, Headers, Buf, Raw_req}
+                            {ok, SC, Headers, Body, Raw_req}
                     end,
             State_1 = do_reply(State, From, StreamTo, ReqId, Resp_format, Reply),
             case Retry_state of
@@ -1257,7 +1257,7 @@ parse_response(Data, #state{reply_buffer = Acc, reqs = Reqs,
                 undefined when HttpVsn =:= "HTTP/1.0";
                                ConnClose =:= "close" ->
                     send_async_headers(ReqId, StreamTo, Give_raw_headers, State_1),
-                    State_1#state{reply_buffer = Data_1};
+                    accumulate_response(Data_1, State_1);
                 undefined when StatCode =:= "303" ->
                     %% Some servers send 303 requests without a body.
                     %% RFC2616 says that they SHOULD, but they dont.

test/Makefile

@@ -1,10 +1,13 @@
 REBAR ?= $(shell which rebar3)
 
-all: compile
-	erl -noshell -boot start_clean -pa ../../ibrowse/ebin -s ibrowse_test local_unit_tests -s erlang halt
+test: compile
+	@erl -noshell -boot start_clean -pa ../../ibrowse/ebin -s ibrowse_test local_unit_tests -s erlang halt
+
+old_tests: compile
+	@erl -noshell -boot start_clean -pa ../../ibrowse/ebin -s ibrowse_test unit_tests -s erlang halt
 
 compile:
-	erl -pa ../../ibrowse/ebin -make
+	@erl -pa ../../ibrowse/ebin -make
 
 clean:
 	$(REBAR) clean

test/ibrowse_test.erl

@@ -37,7 +37,8 @@
          test_binary_headers/1,
          test_generate_body_0/0,
          test_retry_of_requests/0,
-         test_retry_of_requests/1
+         test_retry_of_requests/1,
+	 test_save_to_file_no_content_length/0
 	]).
 
 -include_lib("ibrowse/include/ibrowse.hrl").
@@ -220,8 +221,13 @@ dump_errors(Key, Iod) ->
                       {local_test_fun, test_head_transfer_encoding, []},
                       {local_test_fun, test_head_response_with_body, []},
                       {local_test_fun, test_303_response_with_a_body, []},
+		      {local_test_fun, test_303_response_with_no_body, []},
                       {local_test_fun, test_binary_headers, []},
-                      {local_test_fun, test_retry_of_requests, []}
+                      {local_test_fun, test_retry_of_requests, []},
+		      {local_test_fun, test_save_to_file_no_content_length, []},
+		      {local_test_fun, verify_chunked_streaming, []},
+		      {local_test_fun, test_chunked_streaming_once, []},
+		      {local_test_fun, test_generate_body_0, []}
                      ]).
 
 -define(TEST_LIST, [{"http://intranet/messenger", get},
@@ -254,17 +260,16 @@ dump_errors(Key, Iod) ->
 		    {"http://jigsaw.w3.org/HTTP/CL/", get},
 		    {"http://www.httpwatch.com/httpgallery/chunked/", get},
                     {"https://github.com", get, [{ssl_options, [{depth, 2}]}]}
-		   ] ++ ?LOCAL_TESTS).
+		   ]).
 
 local_unit_tests() ->
     unit_tests([], ?LOCAL_TESTS).
 
 unit_tests() ->
-    error_logger:tty(false),
-    unit_tests([], ?TEST_LIST),
-    error_logger:tty(true).
+    unit_tests([], ?TEST_LIST).
 
 unit_tests(Options, Test_list) ->
+    error_logger:tty(false),
     application:start(crypto),
     application:start(asn1),
     application:start(public_key),
@@ -284,6 +289,7 @@ unit_tests(Options, Test_list) ->
 	    io:format("Timed out waiting for tests to complete~n", [])
     end,
     catch ibrowse_test_server:stop_server(8181),
+    error_logger:tty(true),
     ok.
 
 unit_tests_1(Parent, Options, Test_list) ->
@@ -320,7 +326,8 @@ verify_chunked_streaming(Options) ->
     Res2 = compare_responses(Result_without_streaming, Async_response_list, Async_response_bin_once),
     case {Res1, Res2} of
         {success, success} ->
-            io:format("  Chunked streaming working~n", []);
+            io:format("  Chunked streaming working~n", []),
+	    success;
         _ ->
             ok
     end.
@@ -335,7 +342,7 @@ test_chunked_streaming_once(Options) ->
     io:format("  Fetching data with streaming as binary, {active, once}...~n", []),
     case do_async_req_list(Url, get, [once, {response_format, binary} | Options]) of
         {ok, _, _, _} ->
-            
iospan>:format("  Success!~n", []);
+            success;
         Err ->
             io:format("  Fail: ~p~n", [Err])
     end.
@@ -445,8 +452,8 @@ maybe_stream_next(Req_id, Options) ->
 
 execute_req(local_test_fun, Method, Args) ->
     reset_ibrowse(),
-    io:format("     ~-54.54w: ", [Method]),
     Result = (catch apply(?MODULE, Method, Args)),
+    io:format("     ~-54.54w: ", [Method]),
     io:format("~p~n", [Result]);
 execute_req(Url, Method, Options) ->
     io:format("~7.7w, ~50.50s: ", [Method, Url]),
@@ -558,6 +565,29 @@ test_303_response_with_a_body(Url) ->
             {test_failed, Res}
     end.
 
+%%------------------------------------------------------------------------------
+%% Test that when the save_response_to_file option is used with a server which
+%% does not send the Content-Length header, the response is saved correctly to
+%% a file
+%%------------------------------------------------------------------------------
+test_save_to_file_no_content_length() ->
+    clear_msg_q(),
+    {{Y, M, D}, {H, Mi, S}} = calendar:local_time(),
+    Test_file = filename:join
+		  ([".", 
+		    lists:flatten(
+		      io_lib:format("test_save_to_file_no_content_length_~p~p~p_~p~p~p.txt", [Y, M, D, H, Mi, S]))]),
+    try
+	case ibrowse:send_req("http://localhost:8181/ibrowse_send_file_conn_close", [], get, [], [{save_response_to_file, Test_file}]) of
+	    {ok, "200", _, {file, Test_file}} ->
+		success;
+	    Res ->
+		{test_failed, Res}
+	end
+    after
+	file:delete(Test_file)
+    end.
+
 %%------------------------------------------------------------------------------
 %% Test that retry of requests happens correctly, and that ibrowse doesn't retry
 %% if there is not enough time left
@@ -775,11 +805,10 @@ do_test_20122010_1(Expected_resp, Req_id, Acc) ->
 %% Test requests where body is generated using a Fun
 %%------------------------------------------------------------------------------
 test_generate_body_0() ->
-    io:format("Testing that generation of body using fun works...~n", []),
     Tid = ets:new(ibrowse_test_state, [public]),
     try
         Body_1 = <<"Part 1 of the body">>,
-        Body_2 = <<"Part 2 of the body\r\n\r\n">>,
+        Body_2 = <<"Part 2 of the body\r\n">>,
         Size = size(Body_1) + size(Body_2),
         Body = list_to_binary([Body_1, Body_2]),
         Fun = fun() ->
@@ -799,9 +828,8 @@ test_generate_body_0() ->
                               post,
                               Fun,
                               [{response_format, binary},
-                               {http_vsn, {1,0}}]) of
+                               {http_vsn, {1,1}}]) of
             {ok, "200", _, Body} ->
-                io:format("    Success~n", []),
                 success;
             Err ->
                 io:format("Test failed : ~p~n", [Err]),

test/ibrowse_test_server.erl

@@ -10,10 +10,10 @@
          get_conn_pipeline_depth/0
         ]).
 
--record(request, {method, uri, version, headers = [], body = []}).
+-record(request, {method, uri, version, headers = [], body = [], state}).
 
 -define(dec2hex(X), erlang:integer_to_list(X, 16)).
--define(ACCEPT_TIMEOUT_MS, 1000).
+-define(ACCEPT_TIMEOUT_MS, 10000).
 -define(CONN_PIPELINE_DEPTH, conn_pipeline_depth).
 
 start_server(Port, Sock_type) ->
@@ -45,7 +45,7 @@ start_server(Port, Sock_type) ->
     spawn_link(Fun).
 
 stop_server(Port) ->
-    server_proc_name(Port) ! stop,
+    catch server_proc_name(Port) ! stop,
     timer:sleep(2000),  % wait for server to receive msg and unregister
     ok.
 
@@ -72,7 +72,6 @@ accept_loop(Sock, Sock_type) ->
         {ok, Conn} ->
             Pid = spawn_link(fun() -> connection(Conn, Sock_type) end),
             set_controlling_process(Conn, Sock_type, Pid),
-            Pid ! {setopts, [{active, true}]},
             accept_loop(Sock, Sock_type);
         {error, timeout} ->
             receive
@@ -88,6 +87,7 @@ accept_loop(Sock, Sock_type) ->
 connection(Conn, Sock_type) ->
     catch ets:insert(?CONN_PIPELINE_DEPTH, {self(), 0}),
     try
+	inet:setopts(Conn, [{packet, http}, {active, true}]),
         server_loop(Conn, Sock_type, #request{})
     after
         catch ets:delete(?CONN_PIPELINE_DEPTH, self())
@@ -118,10 +118,22 @@ server_loop(Sock, Sock_type, #request{headers = Headers} = Req) ->
                     gen_tcp:shutdown(Sock, read_write);
                 not_done ->
                     ok;
+		collect_body ->
+		    server_loop(Sock, Sock_type, Req#request{state = collect_body});
                 _ ->
                     catch ets:update_counter(?CONN_PIPELINE_DEPTH, self(), -1)
             end,
             server_loop(Sock, Sock_type, #request{});
+        {http, Sock, {http_error, Packet}} when Req#request.state == collect_body ->
+	    Req_1 = Req#request{body = list_to_binary([Packet, Req#request.body])},
+	    case process_request(Sock, Sock_type, Req_1) of
+		close_connection ->
+                    gen_tcp:shutdown(Sock, read_write);
+                ok ->
+                    server_loop(Sock, Sock_type, #request{});
+		collect_body ->
+		    server_loop(Sock, Sock_type, Req_1)
+	    end;	    
         {http, Sock, {http_error, Err}} ->
             io:format("Error parsing HTTP request:~n"
                       "Req so far : ~p~n"
@@ -185,6 +197,19 @@ process_request(Sock, Sock_type,
                          uri = {abs_path, "/ibrowse_head_transfer_enc"}}) ->
     Resp = <<"HTTP/1.1 400 Bad Request\r\nServer: Apache-Coyote/1.1\r\nContent-Length:5\r\nDate: Wed, 04 Apr 2012 16:53:49 GMT\r\n\r\nabcde">>,
     do_send(Sock, Sock_type, Resp);
+process_request(Sock, Sock_type,
+                #request{method='POST',
+                         headers = Headers,
+                         uri = {abs_path, "/echo_body"},
+			body = Body}) ->
+    Content_len = get_content_length(Headers),
+    case iolist_size(Body) == Content_len of
+	true ->
+	    Resp = [<<"HTTP/1.1 200 OK\r\nContent-Length: ">>, integer_to_list(Content_len), <<"\r\nServer: ibrowse_test_server\r\n\r\n">>, Body],
+	    do_send(Sock, Sock_type, list_to_binary(Resp));
+	false ->
+	    collect_body
+    end;
 process_request(Sock, Sock_type,
                 #request{method='GET',
                          headers = Headers,
@@ -213,7 +238,7 @@ process_request(Sock, Sock_type,
                 #request{method='POST',
                          headers = _Headers,
                          uri = {abs_path, "/ibrowse_303_no_body_test"}}) ->
-    Resp = <<"HTTP/1.1 303 See Other\r\nLocation: http://example.org\r\n">>,
+    Resp = <<"HTTP/1.1 303 See Other\r\nLocation: http://example.org\r\n\r\n">>,
     do_send(Sock, Sock_type, Resp);
 process_request(Sock, Sock_type,
                 #request{method='POST',
@@ -236,6 +261,15 @@ process_request(Sock, Sock_type,
     Resp = <<"HTTP/1.1 200 OK\r\nServer: Apache-Coyote/1.1\r\nDate: Wed, 04 Apr 2012 16:53:49 GMT\r\nConnection: close\r\n\r\n">>,
     do_send(Sock, Sock_type, Resp),
     close_connection;
+process_request(Sock, Sock_type,
+    #request{method='GET',
+        headers = _Headers,
+        uri = {abs_path, "/ibrowse_send_file_conn_close"}}) ->
+    Resp = <<"HTTP/1.1 200 OK\r\nServer: Apache-Coyote/1.1\r\nDate: Wed, 04 Apr 2012 16:53:49 GMT\r\nConnection: close\r\n\r\nblahblah-">>,    
+    do_send(Sock, Sock_type, Resp),
+    timer:sleep(1000),
+    do_send(Sock, Sock_type, <<"blahblah">>),
+    close_connection;
 process_request(_Sock, _Sock_type, #request{uri = {abs_path, "/never_respond"} } ) ->
     not_done;
 process_request(Sock, Sock_type, Req) ->
@@ -300,3 +334,10 @@ to_lower(X) when is_atom(X) ->
     list_to_atom(to_lower(atom_to_list(X)));
 to_lower(X) when is_list(X) ->
     string:to_lower(X).
+
+get_content_length([{http_header, _, 'Content-Length', _, V} | _]) ->
+    list_to_integer(V);
+get_content_length([{http_header, _, _X, _, _Y} | T]) ->
+    get_content_length(T);
+get_content_length([]) ->
+    undefined.

test/ibrowse_tests.erl

@@ -1,11 +1,11 @@
-%%% File    : ibrowse_functional_tests.erl
+%%% File    : ibrowse_tests.erl
 %%% Authors : Benjamin Lee <http://github.com/benjaminplee>
 %%%           Dan Schwabe <http://github.com/dfschwabe>
 %%%           Brian Richards <http://github.com/richbria>
 %%% Description : Functional tests of the ibrowse library using a live test HTTP server
 %%% Created : 18 November 2014 by Benjamin Lee <yardspoon@gmail.com>
 
--module(ibrowse_functional_tests).
+-module(ibrowse_tests).
 
 -include_lib("eunit/include/eunit.hrl").
 -define(PER_TEST_TIMEOUT_SEC, 60).