From 7096f9228af1cdf98843660560a0d84b48cf766b Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Tue, 17 Nov 2015 15:03:14 +0100 Subject: inets: Remove exit that generates an incorrect internal server error --- lib/inets/src/http_server/mod_esi.erl | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'lib/inets') diff --git a/lib/inets/src/http_server/mod_esi.erl b/lib/inets/src/http_server/mod_esi.erl index b9a0797977..6764e2c51e 100644 --- a/lib/inets/src/http_server/mod_esi.erl +++ b/lib/inets/src/http_server/mod_esi.erl @@ -473,13 +473,12 @@ handle_body(Pid, ModData, Body, Timeout, Size, IsDisableChunkedSend) -> {'EXIT', Pid, Reason} when is_pid(Pid) -> ?hdrv("handle_body - exit", [{reason, Reason}]), httpd_response:send_final_chunk(ModData, IsDisableChunkedSend), - exit({mod_esi_linked_process_died, Pid, Reason}) - + done after Timeout -> ?hdrv("handle_body - timeout", []), process_flag(trap_exit,false), httpd_response:send_final_chunk(ModData, IsDisableChunkedSend), - exit({mod_esi_linked_process_timeout, Pid}) + done end. erl_script_timeout(Db) -> -- cgit v1.2.3 From ebbbd714ad2398858b09065bce82830d8affe2a4 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Wed, 18 Nov 2015 10:58:22 +0100 Subject: inets: Add warning header in "chunk trailer" when mod_esi callback times out or fails Also remove legacy debug macros and add help function httpd_util:error_log/2 to avoid code duplication. --- lib/inets/src/http_lib/http_chunk.erl | 19 ++++++++-- lib/inets/src/http_lib/http_response.erl | 2 ++ lib/inets/src/http_server/httpd_example.erl | 10 +++++- .../src/http_server/httpd_request_handler.erl | 15 ++------ lib/inets/src/http_server/httpd_response.erl | 27 +++++--------- lib/inets/src/http_server/httpd_util.erl | 16 ++++++++- lib/inets/src/http_server/mod_esi.erl | 42 +++++++++++++++------- lib/inets/test/httpd_1_1.erl | 11 +++++- lib/inets/test/httpd_SUITE.erl | 9 ++++- lib/inets/test/httpd_test_lib.erl | 16 ++++++--- 10 files changed, 112 insertions(+), 55 deletions(-) (limited to 'lib/inets') diff --git a/lib/inets/src/http_lib/http_chunk.erl b/lib/inets/src/http_lib/http_chunk.erl index 9699856bf8..7325f24809 100644 --- a/lib/inets/src/http_lib/http_chunk.erl +++ b/lib/inets/src/http_lib/http_chunk.erl @@ -25,7 +25,7 @@ -include("http_internal.hrl"). %% API --export([decode/3, encode/1, encode_last/0, handle_headers/2]). +-export([decode/3, encode/1, encode_last/0, encode_last/1, handle_headers/2]). %% Callback API - used for example if the chunkedbody is received a %% little at a time on a socket. -export([decode_size/1, ignore_extensions/1, decode_data/1, decode_trailer/1]). @@ -85,6 +85,11 @@ encode(Chunk) when is_list(Chunk)-> encode_last() -> <<$0, ?CR, ?LF, ?CR, ?LF >>. +encode_last([]) -> + encode_last(); +encode_last(Trailers0) -> + Trailers = list_to_binary(encode_trailers(Trailers0)), + <<$0, ?CR, ?LF, Trailers/binary>>. %%------------------------------------------------------------------------- %% handle_headers(HeaderRecord, ChunkedHeaders) -> NewHeaderRecord @@ -276,10 +281,18 @@ decode_trailer(<>, Header, Headers, Body, BodyLength, Rem Body, BodyLength, RemainingSize, TotalMaxHeaderSize); decode_trailer(<>, Header, Headers, Body, BodyLength, RemainingSize, TotalMaxHeaderSize) -> - decode_trailer(Rest, [Octet | Header], Headers, - Body, BodyLength, RemainingSize - 1, TotalMaxHeaderSize). + decode_trailer(Rest, [Octet | Header], Headers, + Body, BodyLength, remaing_size(RemainingSize, 1), TotalMaxHeaderSize). remaing_size(nolimit, _) -> nolimit; remaing_size(Total, Consumed) -> Total - Consumed. + +encode_trailers(Trailers) -> + encode_trailers(Trailers, ""). + +encode_trailers([], Acc) -> + Acc ++ ?CRLF ++ ?CRLF; +encode_trailers([{Header, Value} | Rest], Acc) -> + encode_trailers(Rest, Header ++ ":" ++ Value ++ ?CRLF ++ Acc). diff --git a/lib/inets/src/http_lib/http_response.erl b/lib/inets/src/http_lib/http_response.erl index d13670700c..42e5dd263d 100644 --- a/lib/inets/src/http_lib/http_response.erl +++ b/lib/inets/src/http_lib/http_response.erl @@ -65,6 +65,8 @@ header_list(Headers) -> %%%======================================================================== fill_headers([], _, Headers) -> Headers; +fill_headers([[]], _, Headers) -> + Headers; fill_headers([[Ch|HeaderFold]|Tail], Folded, Headers) when Ch == $\t; Ch == $\s -> fill_headers(Tail, [HeaderFold|Folded], Headers); diff --git a/lib/inets/src/http_server/httpd_example.erl b/lib/inets/src/http_server/httpd_example.erl index d729affd6d..0222487a4b 100644 --- a/lib/inets/src/http_server/httpd_example.erl +++ b/lib/inets/src/http_server/httpd_example.erl @@ -24,7 +24,7 @@ -export([newformat/3]). %% These are used by the inets test-suite --export([delay/1]). +-export([delay/1, chunk_timeout/3]). print(String) -> @@ -142,3 +142,11 @@ i(F) -> i(F,[]). i(F,A) -> io:format(F ++ "~n",A). sleep(T) -> receive after T -> ok end. + +%% ------------------------------------------------------ + +chunk_timeout(SessionID, _, StrInt) -> + mod_esi:deliver(SessionID, "Tranfer-Encoding:chunked/html\r\n\r\n"), + mod_esi:deliver(SessionID, top("Test chunk encoding timeout")), + timer:sleep(20000), + mod_esi:deliver(SessionID, footer()). diff --git a/lib/inets/src/http_server/httpd_request_handler.erl b/lib/inets/src/http_server/httpd_request_handler.erl index 143d599edb..134576059d 100644 --- a/lib/inets/src/http_server/httpd_request_handler.erl +++ b/lib/inets/src/http_server/httpd_request_handler.erl @@ -630,21 +630,10 @@ decrease(N) when is_integer(N) -> decrease(N) -> N. -error_log(ReasonString, Info) -> +error_log(ReasonString, #mod{config_db = ConfigDB}) -> Error = lists:flatten( io_lib:format("Error reading request: ~s", [ReasonString])), - error_log(mod_log, Info, Error), - error_log(mod_disk_log, Info, Error). - -error_log(Mod, #mod{config_db = ConfigDB} = Info, String) -> - Modules = httpd_util:lookup(ConfigDB, modules, - [mod_get, mod_head, mod_log]), - case lists:member(Mod, Modules) of - true -> - Mod:error_log(Info, String); - _ -> - ok - end. + httpd_util:error_log(ConfigDB, Error). %%-------------------------------------------------------------------- diff --git a/lib/inets/src/http_server/httpd_response.erl b/lib/inets/src/http_server/httpd_response.erl index 71243f525a..c0b5f09faf 100644 --- a/lib/inets/src/http_server/httpd_response.erl +++ b/lib/inets/src/http_server/httpd_response.erl @@ -20,8 +20,8 @@ %% -module(httpd_response). -export([generate_and_send_response/1, send_status/3, send_header/3, - send_body/3, send_chunk/3, send_final_chunk/2, split_header/2, - is_disable_chunked_send/1, cache_headers/2]). + send_body/3, send_chunk/3, send_final_chunk/2, send_final_chunk/3, + split_header/2, is_disable_chunked_send/1, cache_headers/2]). -export([map_status_code/2]). -include_lib("inets/src/inets_app/inets_internal.hrl"). @@ -89,8 +89,7 @@ traverse_modules(ModData,[Module|Rest]) -> "~n Error: ~p" "~n Stack trace: ~p", [Module, T, E, ?STACK()])), - report_error(mod_log, ModData#mod.config_db, String), - report_error(mod_disk_log, ModData#mod.config_db, String), + httpd_util:error_log(ModData#mod.config_db, String), send_status(ModData, 500, none), done end. @@ -245,7 +244,6 @@ send_chunk(_, <<>>, _) -> ok; send_chunk(_, [], _) -> ok; - send_chunk(#mod{http_version = "HTTP/1.1", socket_type = Type, socket = Sock}, Response0, false) -> Response = http_chunk:encode(Response0), @@ -254,10 +252,13 @@ send_chunk(#mod{http_version = "HTTP/1.1", send_chunk(#mod{socket_type = Type, socket = Sock} = _ModData, Response, _) -> httpd_socket:deliver(Type, Sock, Response). +send_final_chunk(Mod, IsDisableChunkedSend) -> + send_final_chunk(Mod, [], IsDisableChunkedSend). + send_final_chunk(#mod{http_version = "HTTP/1.1", - socket_type = Type, socket = Sock}, false) -> - httpd_socket:deliver(Type, Sock, http_chunk:encode_last()); -send_final_chunk(#mod{socket_type = Type, socket = Sock}, _) -> + socket_type = Type, socket = Sock}, Trailers, false) -> + httpd_socket:deliver(Type, Sock, http_chunk:encode_last(Trailers)); +send_final_chunk(#mod{socket_type = Type, socket = Sock}, _, _) -> httpd_socket:close(Type, Sock). is_disable_chunked_send(Db) -> @@ -397,16 +398,6 @@ send_response_old(#mod{socket_type = Type, content_length(Body)-> integer_to_list(httpd_util:flatlength(Body)). -report_error(Mod, ConfigDB, Error) -> - Modules = httpd_util:lookup(ConfigDB, modules, - [mod_get, mod_head, mod_log]), - case lists:member(Mod, Modules) of - true -> - Mod:report_error(ConfigDB, Error); - _ -> - ok - end. - handle_headers([], NewHeaders) -> {ok, NewHeaders}; diff --git a/lib/inets/src/http_server/httpd_util.erl b/lib/inets/src/http_server/httpd_util.erl index 0387d71911..ab43f0b378 100644 --- a/lib/inets/src/http_server/httpd_util.erl +++ b/lib/inets/src/http_server/httpd_util.erl @@ -31,7 +31,7 @@ convert_netscapecookie_date/1, enable_debug/1, valid_options/3, modules_validate/1, module_validate/1, dir_validate/2, file_validate/2, mime_type_validate/1, - mime_types_validate/1, custom_date/0]). + mime_types_validate/1, custom_date/0, error_log/2]). -export([encode_hex/1, decode_hex/1]). -include_lib("kernel/include/file.hrl"). @@ -776,3 +776,17 @@ do_enable_debug([{Level,Modules}|Rest]) ok end, do_enable_debug(Rest). + +error_log(ConfigDb, Error) -> + error_log(mod_log, ConfigDb, Error), + error_log(mod_disk_log, ConfigDb, Error). + +error_log(Mod, ConfigDB, Error) -> + Modules = httpd_util:lookup(ConfigDB, modules, + [mod_get, mod_head, mod_log]), + case lists:member(Mod, Modules) of + true -> + Mod:report_error(ConfigDB, Error); + _ -> + ok + end. diff --git a/lib/inets/src/http_server/mod_esi.erl b/lib/inets/src/http_server/mod_esi.erl index 6764e2c51e..1923411449 100644 --- a/lib/inets/src/http_server/mod_esi.erl +++ b/lib/inets/src/http_server/mod_esi.erl @@ -376,7 +376,6 @@ erl_scheme_webpage_chunk(Mod, Func, Env, Input, ModData) -> end), Response = deliver_webpage_chunk(ModData, Pid), - process_flag(trap_exit,false), Response. @@ -418,7 +417,6 @@ deliver_webpage_chunk(#mod{config_db = Db} = ModData, Pid, Timeout) -> ?hdrv("deliver_webpage_chunk - timeout", []), send_headers(ModData, 504, [{"connection", "close"}]), httpd_socket:close(ModData#mod.socket_type, ModData#mod.socket), - process_flag(trap_exit,false), {proceed,[{response, {already_sent, 200, 0}} | ModData#mod.data]} end. @@ -446,7 +444,6 @@ send_headers(ModData, StatusCode, HTTPHeaders) -> ExtraHeaders ++ HTTPHeaders). handle_body(_, #mod{method = "HEAD"} = ModData, _, _, Size, _) -> - process_flag(trap_exit,false), {proceed, [{response, {already_sent, 200, Size}} | ModData#mod.data]}; handle_body(Pid, ModData, Body, Timeout, Size, IsDisableChunkedSend) -> @@ -454,33 +451,54 @@ handle_body(Pid, ModData, Body, Timeout, Size, IsDisableChunkedSend) -> httpd_response:send_chunk(ModData, Body, IsDisableChunkedSend), receive {esi_data, Data} when is_binary(Data) -> - ?hdrt("handle_body - received binary data (esi)", []), handle_body(Pid, ModData, Data, Timeout, Size + byte_size(Data), IsDisableChunkedSend); {esi_data, Data} -> - ?hdrt("handle_body - received data (esi)", []), handle_body(Pid, ModData, Data, Timeout, Size + length(Data), IsDisableChunkedSend); {ok, Data} -> - ?hdrt("handle_body - received data (ok)", []), handle_body(Pid, ModData, Data, Timeout, Size + length(Data), IsDisableChunkedSend); {'EXIT', Pid, normal} when is_pid(Pid) -> - ?hdrt("handle_body - exit:normal", []), httpd_response:send_final_chunk(ModData, IsDisableChunkedSend), {proceed, [{response, {already_sent, 200, Size}} | ModData#mod.data]}; {'EXIT', Pid, Reason} when is_pid(Pid) -> - ?hdrv("handle_body - exit", [{reason, Reason}]), - httpd_response:send_final_chunk(ModData, IsDisableChunkedSend), + Error = lists:flatten(io_lib:format("mod_esi process failed with reason ~p", [Reason])), + httpd_util:error_log(ModData#mod.config_db, Error), + httpd_response:send_final_chunk(ModData, + [{"Warning", "199 inets server - body maybe incomplete, " + "internal server error"}], + IsDisableChunkedSend), done after Timeout -> - ?hdrv("handle_body - timeout", []), - process_flag(trap_exit,false), - httpd_response:send_final_chunk(ModData, IsDisableChunkedSend), + kill_esi_delivery_process(Pid), + httpd_response:send_final_chunk(ModData, [{"Warning", "199 inets server - " + "body maybe incomplete, timed out"}], + IsDisableChunkedSend), done end. +kill_esi_delivery_process(Pid) -> + exit(Pid, kill), + receive + {'EXIT', Pid, killed} -> + %% Clean message queue + receive + {esi_data, _} -> + ok + after 0 -> + ok + end, + receive + {ok, _} -> + ok + after 0 -> + ok + end + end. + + erl_script_timeout(Db) -> httpd_util:lookup(Db, erl_script_timeout, ?DEFAULT_ERL_TIMEOUT). diff --git a/lib/inets/test/httpd_1_1.erl b/lib/inets/test/httpd_1_1.erl index dd9d21bbfc..db6def9d17 100644 --- a/lib/inets/test/httpd_1_1.erl +++ b/lib/inets/test/httpd_1_1.erl @@ -24,7 +24,7 @@ -include_lib("kernel/include/file.hrl"). -export([host/4, chunked/4, expect/4, range/4, if_test/5, trace/4, - head/4, mod_cgi_chunked_encoding_test/5]). + head/4, mod_cgi_chunked_encoding_test/5, mod_esi_chunk_timeout/4]). %% -define(all_keys_lower_case,true). -ifndef(all_keys_lower_case). @@ -274,6 +274,15 @@ mod_cgi_chunked_encoding_test(Type, Port, Host, Node, [Request| Rest])-> [{statuscode, 200}]), mod_cgi_chunked_encoding_test(Type, Port, Host, Node, Rest). + +mod_esi_chunk_timeout(Type, Port, Host, Node) -> + ok = httpd_test_lib:verify_request(Type, Host, Port, Node, + "GET /cgi-bin/erl/httpd_example/chunk_timeout?input=20000 HTTP/1.1\r\n" + "Host:"++ Host ++"\r\n" + "\r\n", + [{statuscode, 200}, + {header, "warning"}]). + %%-------------------------------------------------------------------- %% Internal functions %%-------------------------------------------------------------------- diff --git a/lib/inets/test/httpd_SUITE.erl b/lib/inets/test/httpd_SUITE.erl index 9bd6f3636c..1d8a603981 100644 --- a/lib/inets/test/httpd_SUITE.erl +++ b/lib/inets/test/httpd_SUITE.erl @@ -117,7 +117,7 @@ groups() -> {htaccess, [], [htaccess_1_1, htaccess_1_0, htaccess_0_9]}, {security, [], [security_1_1, security_1_0]}, %% Skip 0.9 as causes timing issus in test code {http_1_1, [], [host, chunked, expect, cgi, cgi_chunked_encoding_test, - trace, range, if_modified_since] ++ http_head() ++ http_get() ++ load()}, + trace, range, if_modified_since, mod_esi_chunk_timeout] ++ http_head() ++ http_get() ++ load()}, {http_1_0, [], [host, cgi, trace] ++ http_head() ++ http_get() ++ load()}, {http_0_9, [], http_head() ++ http_get() ++ load()} ]. @@ -757,6 +757,13 @@ esi(Config) when is_list(Config) -> Config, [{statuscode, 200}, {no_header, "cache-control"}]). %%------------------------------------------------------------------------- +mod_esi_chunk_timeout(Config) when is_list(Config) -> + ok = httpd_1_1:mod_esi_chunk_timeout(?config(type, Config), + ?config(port, Config), + ?config(host, Config), + ?config(node, Config)). + +%%------------------------------------------------------------------------- cgi() -> [{doc, "Test mod_cgi"}]. diff --git a/lib/inets/test/httpd_test_lib.erl b/lib/inets/test/httpd_test_lib.erl index a5b836f651..c58966ce10 100644 --- a/lib/inets/test/httpd_test_lib.erl +++ b/lib/inets/test/httpd_test_lib.erl @@ -235,11 +235,17 @@ validate(RequestStr, #state{status_line = {Version, StatusCode, _}, _ -> ok end, - do_validate(http_response:header_list(Headers), Options, N, P), - check_body(RequestStr, StatusCode, - Headers#http_response_h.'content-type', - list_to_integer(Headers#http_response_h.'content-length'), - Body). + HList = http_response:header_list(Headers), + do_validate(HList, Options, N, P), + case lists:keysearch("warning", 1, HList) of + {value, _} -> + ok; + _ -> + check_body(RequestStr, StatusCode, + Headers#http_response_h.'content-type', + list_to_integer(Headers#http_response_h.'content-length'), + Body) + end. %-------------------------------------------------------------------- %% Internal functions -- cgit v1.2.3