From 724ae2b149b60063ff140f58d9d9a86f2fd93048 Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Mon, 19 Sep 2011 14:28:06 +0200 Subject: [ftpc] Add a config option to specify a data connect timeout. That is how long the ftp client will wait for the server to connect to the data socket. If this timeout occurs, an error will be returned to the caller and the ftp client process will be terminated. OTP-9545 --- lib/inets/doc/src/ftp.xml | 21 +++- lib/inets/doc/src/notes.xml | 13 +- lib/inets/src/ftp/ftp.erl | 151 ++++++++++++++++++------ lib/inets/test/ftp_suite_lib.erl | 82 +++++++++---- lib/inets/test/ftp_windows_2003_server_test.erl | 22 ++-- 5 files changed, 217 insertions(+), 72 deletions(-) (limited to 'lib') diff --git a/lib/inets/doc/src/ftp.xml b/lib/inets/doc/src/ftp.xml index ca902d8d9d..f8f11ec705 100644 --- a/lib/inets/doc/src/ftp.xml +++ b/lib/inets/doc/src/ftp.xml @@ -1,10 +1,10 @@ - +
- 19972010 + 19972011 Ericsson AB. All Rights Reserved. @@ -141,11 +141,21 @@ {timeout, Timeout} -

Timeout = integer() >= 0

+

Timeout = non_neg_integer()

Connection timeout.

Default is 60000 (milliseconds).

+ {dtimeout, DTimeout} + + +

DTimeout = non_neg_integer() | infinity

+

Data Connect timeout. + The time the client will wait for the server to connect to the + data socket.

+

Default is infinity.

+ + {progress, Progress} @@ -542,11 +552,12 @@ verbose() = boolean() (defaults to false) debug() = disable | debug | trace (defaults to disable) - open_option() = {ipfamily, ipfamily()} | {port, port()} | {mode, mode()} | {timeout, timeout()} | {progress, progress()} + open_option() = {ipfamily, ipfamily()} | {port, port()} | {mode, mode()} | {timeout, timeout()} | {dtimeout, dtimeout()} | {progress, progress()} ipfamily() = inet | inet6 | inet6fb4 (defaults to inet) port() = integer() > 0 (defaults to 21) mode() = active | passive (defaults to passive) - timeout() = integer() >= 0 (defaults to 60000 milliseconds) + timeout() = integer() > 0 (defaults to 60000 milliseconds) + dtimeout() = integer() > 0 | infinity (defaults to infinity) pogress() = ignore | {module(), function(), initial_data()} (defaults to ignore) module() = atom() function() = atom() diff --git a/lib/inets/doc/src/notes.xml b/lib/inets/doc/src/notes.xml index c53aa653d8..061c8aa87c 100644 --- a/lib/inets/doc/src/notes.xml +++ b/lib/inets/doc/src/notes.xml @@ -35,17 +35,22 @@
Inets 5.8
Improvements and New Features + -
diff --git a/lib/inets/src/ftp/ftp.erl b/lib/inets/src/ftp/ftp.erl index ac72963347..b6da92947c 100644 --- a/lib/inets/src/ftp/ftp.erl +++ b/lib/inets/src/ftp/ftp.erl @@ -55,9 +55,10 @@ -include("ftp_internal.hrl"). %% Constante used in internal state definition --define(CONNECTION_TIMEOUT, 60*1000). --define(DEFAULT_MODE, passive). --define(PROGRESS_DEFAULT, ignore). +-define(CONNECTION_TIMEOUT, 60*1000). +-define(DATA_ACCEPT_TIMEOUT, infinity). +-define(DEFAULT_MODE, passive). +-define(PROGRESS_DEFAULT, ignore). %% Internal Constants -define(FTP_PORT, 21). @@ -88,7 +89,8 @@ %% data needed further on. caller = undefined, % term() ipfamily, % inet | inet6 | inet6fb4 - progress = ignore % ignore | pid() + progress = ignore, % ignore | pid() + dtimeout = ?DATA_ACCEPT_TIMEOUT % non_neg_integer() | infinity }). @@ -847,6 +849,7 @@ start_options(Options) -> %% host %% port %% timeout +%% dtimeout %% progress open_options(Options) -> ?fcrt("open_options", [{options, Options}]), @@ -875,7 +878,12 @@ open_options(Options) -> (_) -> false end, ValidateTimeout = - fun(Timeout) when is_integer(Timeout) andalso (Timeout > 0) -> true; + fun(Timeout) when is_integer(Timeout) andalso (Timeout >= 0) -> true; + (_) -> false + end, + ValidateDTimeout = + fun(DTimeout) when is_integer(DTimeout) andalso (DTimeout >= 0) -> true; + (infinity) -> true; (_) -> false end, ValidateProgress = @@ -893,6 +901,7 @@ open_options(Options) -> {port, ValidatePort, false, ?FTP_PORT}, {ipfamily, ValidateIpFamily, false, inet}, {timeout, ValidateTimeout, false, ?CONNECTION_TIMEOUT}, + {dtimeout, ValidateDTimeout, false, ?DATA_ACCEPT_TIMEOUT}, {progress, ValidateProgress, false, ?PROGRESS_DEFAULT}], validate_options(Options, ValidOptions, []). @@ -1037,13 +1046,15 @@ handle_call({_, {open, ip_comm, Opts}}, From, State) -> Mode = key_search(mode, Opts, ?DEFAULT_MODE), Port = key_search(port, Opts, ?FTP_PORT), Timeout = key_search(timeout, Opts, ?CONNECTION_TIMEOUT), + DTimeout = key_search(dtimeout, Opts, ?DATA_ACCEPT_TIMEOUT), Progress = key_search(progress, Opts, ignore), IpFamily = key_search(ipfamily, Opts, inet), - + State2 = State#state{client = From, mode = Mode, progress = progress(Progress), - ipfamily = IpFamily}, + ipfamily = IpFamily, + dtimeout = DTimeout}, ?fcrd("handle_call(open) -> setup ctrl connection with", [{host, Host}, {port, Port}, {timeout, Timeout}]), @@ -1064,11 +1075,13 @@ handle_call({_, {open, ip_comm, Host, Opts}}, From, State) -> Mode = key_search(mode, Opts, ?DEFAULT_MODE), Port = key_search(port, Opts, ?FTP_PORT), Timeout = key_search(timeout, Opts, ?CONNECTION_TIMEOUT), + DTimeout = key_search(dtimeout, Opts, ?DATA_ACCEPT_TIMEOUT), Progress = key_search(progress, Opts, ignore), State2 = State#state{client = From, mode = Mode, - progress = progress(Progress)}, + progress = progress(Progress), + dtimeout = DTimeout}, case setup_ctrl_connection(Host, Port, Timeout, State2) of {ok, State3, WaitTimeout} -> @@ -1657,9 +1670,19 @@ handle_ctrl_result({pos_compl, Lines}, %%-------------------------------------------------------------------------- %% Directory listing handle_ctrl_result({pos_prel, _}, #state{caller = {dir, Dir}} = State) -> - NewState = accept_data_connection(State), - activate_data_connection(NewState), - {noreply, NewState#state{caller = {handle_dir_result, Dir}}}; + case accept_data_connection(State) of + {ok, NewState} -> + activate_data_connection(NewState), + {noreply, NewState#state{caller = {handle_dir_result, Dir}}}; + {error, _Reason} = ERROR -> + case State#state.client of + undefined -> + {stop, ERROR, State}; + From -> + gen_server:reply(From, ERROR), + {stop, normal, State#state{client = undefined}} + end + end; handle_ctrl_result({pos_compl, _}, #state{caller = {handle_dir_result, Dir, Data}, client = From} @@ -1756,9 +1779,19 @@ handle_ctrl_result({Status, _}, %%-------------------------------------------------------------------------- %% File handling - recv_bin handle_ctrl_result({pos_prel, _}, #state{caller = recv_bin} = State) -> - NewState = accept_data_connection(State), - activate_data_connection(NewState), - {noreply, NewState}; + case accept_data_connection(State) of + {ok, NewState} -> + activate_data_connection(NewState), + {noreply, NewState}; + {error, _Reason} = ERROR -> + case State#state.client of + undefined -> + {stop, ERROR, State}; + From -> + gen_server:reply(From, ERROR), + {stop, normal, State#state{client = undefined}} + end + end; handle_ctrl_result({pos_compl, _}, #state{caller = {recv_bin, Data}, client = From} = State) -> @@ -1780,16 +1813,37 @@ handle_ctrl_result({Status, _}, #state{caller = {recv_bin, _}} = State) -> handle_ctrl_result({pos_prel, _}, #state{client = From, caller = start_chunk_transfer} = State) -> - NewState = accept_data_connection(State), - gen_server:reply(From, ok), - {noreply, NewState#state{chunk = true, client = undefined, - caller = undefined}}; + case accept_data_connection(State) of + {ok, NewState} -> + gen_server:reply(From, ok), + {noreply, NewState#state{chunk = true, client = undefined, + caller = undefined}}; + {error, _Reason} = ERROR -> + case State#state.client of + undefined -> + {stop, ERROR, State}; + From -> + gen_server:reply(From, ERROR), + {stop, normal, State#state{client = undefined}} + end + end; + %%-------------------------------------------------------------------------- %% File handling - recv_file handle_ctrl_result({pos_prel, _}, #state{caller = {recv_file, _}} = State) -> - NewState = accept_data_connection(State), - activate_data_connection(NewState), - {noreply, NewState}; + case accept_data_connection(State) of + {ok, NewState} -> + activate_data_connection(NewState), + {noreply, NewState}; + {error, _Reason} = ERROR -> + case State#state.client of + undefined -> + {stop, ERROR, State}; + From -> + gen_server:reply(From, ERROR), + {stop, normal, State#state{client = undefined}} + end + end; handle_ctrl_result({Status, _}, #state{caller = {recv_file, Fd}} = State) -> file_close(Fd), @@ -1800,17 +1854,38 @@ handle_ctrl_result({Status, _}, #state{caller = {recv_file, Fd}} = State) -> %% File handling - transfer_* handle_ctrl_result({pos_prel, _}, #state{caller = {transfer_file, Fd}} = State) -> - NewState = accept_data_connection(State), - send_file(Fd, NewState); + case accept_data_connection(State) of + {ok, NewState} -> + send_file(Fd, NewState); + {error, _Reason} = ERROR -> + case State#state.client of + undefined -> + {stop, ERROR, State}; + From -> + gen_server:reply(From, ERROR), + {stop, normal, State#state{client = undefined}} + end + end; handle_ctrl_result({pos_prel, _}, #state{caller = {transfer_data, Bin}} = State) -> - NewState = accept_data_connection(State), - send_data_message(NewState, Bin), - close_data_connection(NewState), - activate_ctrl_connection(NewState), - {noreply, NewState#state{caller = transfer_data_second_phase, - dsock = undefined}}; + case accept_data_connection(State) of + {ok, NewState} -> + send_data_message(NewState, Bin), + close_data_connection(NewState), + activate_ctrl_connection(NewState), + {noreply, NewState#state{caller = transfer_data_second_phase, + dsock = undefined}}; + {error, _Reason} = ERROR -> + case State#state.client of + undefined -> + {stop, ERROR, State}; + From -> + gen_server:reply(From, ERROR), + {stop, normal, State#state{client = undefined}} + end + end; + %%-------------------------------------------------------------------------- %% Default handle_ctrl_result({Status, Lines}, #state{client = From} = State) @@ -2009,16 +2084,20 @@ connect2(Host, Port, IpFam, Timeout) -> Error end. - -accept_data_connection(#state{mode = active, - dsock = {lsock, LSock}} = State) -> - {ok, Socket} = gen_tcp:accept(LSock), - gen_tcp:close(LSock), - State#state{dsock = Socket}; +accept_data_connection(#state{mode = active, + dtimeout = DTimeout, + dsock = {lsock, LSock}} = State) -> + case gen_tcp:accept(LSock, DTimeout) of + {ok, Socket} -> + gen_tcp:close(LSock), + {ok, State#state{dsock = Socket}}; + {error, Reason} -> + {error, {data_connect_failed, Reason}} + end; accept_data_connection(#state{mode = passive} = State) -> - State. + {ok, State}. send_ctrl_message(#state{csock = Socket, verbose = Verbose}, Message) -> %% io:format("send control message: ~n~p~n", [lists:flatten(Message)]), diff --git a/lib/inets/test/ftp_suite_lib.erl b/lib/inets/test/ftp_suite_lib.erl index 3ebd02229e..ffb58c91b6 100644 --- a/lib/inets/test/ftp_suite_lib.erl +++ b/lib/inets/test/ftp_suite_lib.erl @@ -196,7 +196,9 @@ test_filenames() -> %% variable, but should NOT alter/remove any existing entries. %%-------------------------------------------------------------------- init_per_testcase(Case, Config) - when (Case =:= open) orelse (Case =:= open_port) -> + when (Case =:= open) orelse + (Case =:= open_port) -> + put(ftp_testcase, Case), io:format(user, "~n~n*** INIT ~w:~w ***~n~n", [?MODULE, Case]), inets:start(), NewConfig = data_dir(Config), @@ -266,7 +268,7 @@ do_init_per_testcase(Case, Config) -> end, Opts2 = case string:tokens(atom_to_list(Case), [$_]) of - [_, "active" | _] -> + ["active" | _] -> [{mode, active} | Opts1]; _ -> [{mode, passive} | Opts1] @@ -367,8 +369,11 @@ open(Config) when is_list(Config) -> tc_open(Host) -> + p("tc_open -> entry with" + "~n Host: ~p", [Host]), {ok, Pid} = ?ftp_open(Host, []), ok = ftp:close(Pid), + p("tc_open -> try (ok) open 1"), {ok, Pid1} = ftp:open({option_list, [{host,Host}, {port, ?FTP_PORT}, @@ -376,11 +381,13 @@ tc_open(Host) -> {timeout, 30000}]}), ok = ftp:close(Pid1), + p("tc_open -> try (fail) open 2"), {error, ehost} = ftp:open({option_list, [{port, ?FTP_PORT}, {flags, [verbose]}]}), {ok, Pid2} = ftp:open(Host), ok = ftp:close(Pid2), + p("tc_open -> try (ok) open 3"), {ok, NewHost} = inet:getaddr(Host, inet), {ok, Pid3} = ftp:open(NewHost), ftp:user(Pid3, ?FTP_USER, ?FTP_PASS), @@ -392,33 +399,68 @@ tc_open(Host) -> %% Bad input that has default values are ignored and the defult %% is used. + p("tc_open -> try (ok) open 4"), {ok, Pid4} = - ftp:open({option_list, [{host, Host}, {port, badarg}, - {flags, [verbose]}, + ftp:open({option_list, [{host, Host}, + {port, badarg}, + {flags, [verbose]}, {timeout, 30000}]}), test_server:sleep(100), ok = ftp:close(Pid4), + + p("tc_open -> try (ok) open 5"), {ok, Pid5} = - ftp:open({option_list, [{host, Host}, {port, ?FTP_PORT}, - {flags, [verbose]}, + ftp:open({option_list, [{host, Host}, + {port, ?FTP_PORT}, + {flags, [verbose]}, {timeout, -42}]}), test_server:sleep(100), ok = ftp:close(Pid5), + + p("tc_open -> try (ok) open 6"), {ok, Pid6} = - ftp:open({option_list, [{host, Host}, {port, ?FTP_PORT}, + ftp:open({option_list, [{host, Host}, + {port, ?FTP_PORT}, {flags, [verbose]}, - {mode, cool}]}), + {mode, cool}]}), test_server:sleep(100), ok = ftp:close(Pid6), + p("tc_open -> try (ok) open 7"), {ok, Pid7} = ftp:open(Host, [{port, ?FTP_PORT}, {verbose, true}, {timeout, 30000}]), ok = ftp:close(Pid7), + p("tc_open -> try (ok) open 8"), {ok, Pid8} = ftp:open(Host, ?FTP_PORT), ok = ftp:close(Pid8), + p("tc_open -> try (ok) open 9"), + {ok, Pid9} = + ftp:open(Host, [{port, ?FTP_PORT}, + {verbose, true}, + {timeout, 30000}, + {dtimeout, -99}]), + ok = ftp:close(Pid9), + + p("tc_open -> try (ok) open 10"), + {ok, Pid10} = + ftp:open(Host, [{port, ?FTP_PORT}, + {verbose, true}, + {timeout, 30000}, + {dtimeout, "foobar"}]), + ok = ftp:close(Pid10), + + p("tc_open -> try (ok) open 11"), + {ok, Pid11} = + ftp:open(Host, [{port, ?FTP_PORT}, + {verbose, true}, + {timeout, 30000}, + {dtimeout, 1}]), + ok = ftp:close(Pid11), + + p("tc_open -> done"), ok. @@ -445,7 +487,7 @@ passive_user(suite) -> []; passive_user(Config) when is_list(Config) -> Pid = ?config(ftp, Config), - io:format("Pid: ~p~n",[Pid]), + p("Pid: ~p",[Pid]), do_user(Pid). @@ -967,13 +1009,13 @@ api_missuse(doc)-> ["Test that behaviour of the ftp process if the api is abused"]; api_missuse(suite) -> []; api_missuse(Config) when is_list(Config) -> - io:format("api_missuse -> entry~n", []), + p("api_missuse -> entry"), Flag = process_flag(trap_exit, true), Pid = ?config(ftp, Config), Host = ftp_host(Config), %% Serious programming fault, connetion will be shut down - io:format("api_missuse -> verify bad call termination (~p)~n", [Pid]), + p("api_missuse -> verify bad call termination (~p)", [Pid]), case (catch gen_server:call(Pid, {self(), foobar, 10}, infinity)) of {error, {connection_terminated, 'API_violation'}} -> ok; @@ -983,23 +1025,23 @@ api_missuse(Config) when is_list(Config) -> test_server:sleep(500), undefined = process_info(Pid, status), - io:format("api_missuse -> start new client~n", []), + p("api_missuse -> start new client"), {ok, Pid2} = ?ftp_open(Host, []), %% Serious programming fault, connetion will be shut down - io:format("api_missuse -> verify bad cast termination~n", []), + p("api_missuse -> verify bad cast termination"), gen_server:cast(Pid2, {self(), foobar, 10}), test_server:sleep(500), undefined = process_info(Pid2, status), - io:format("api_missuse -> start new client~n", []), + p("api_missuse -> start new client"), {ok, Pid3} = ?ftp_open(Host, []), %% Could be an innocent misstake the connection lives. - io:format("api_missuse -> verify bad bang~n", []), + p("api_missuse -> verify bad bang"), Pid3 ! foobar, test_server:sleep(500), {status, _} = process_info(Pid3, status), process_flag(trap_exit, Flag), - io:format("api_missuse -> done~n", []), + p("api_missuse -> done"), ok. @@ -1567,9 +1609,9 @@ split([], I, Is) -> lists:reverse([lists:reverse(I)| Is]). do_ftp_open(Host, Opts) -> - io:format("do_ftp_open -> entry with" - "~n Host: ~p" - "~n Opts: ~p", [Host, Opts]), + p("do_ftp_open -> entry with" + "~n Host: ~p" + "~n Opts: ~p", [Host, Opts]), case ftp:open(Host, Opts) of {ok, _} = OK -> OK; @@ -1595,7 +1637,7 @@ passwd() -> ftpd_hosts(Config) -> DataDir = ?config(data_dir, Config), FileName = filename:join([DataDir, "../ftp_SUITE_data/", ftpd_hosts]), - io:format("FileName: ~p~n", [FileName]), + p("FileName: ~p", [FileName]), case file:consult(FileName) of {ok, [Hosts]} when is_list(Hosts) -> Hosts; diff --git a/lib/inets/test/ftp_windows_2003_server_test.erl b/lib/inets/test/ftp_windows_2003_server_test.erl index 57f1ae8358..32f25713f8 100644 --- a/lib/inets/test/ftp_windows_2003_server_test.erl +++ b/lib/inets/test/ftp_windows_2003_server_test.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2005-2010. All Rights Reserved. +%% Copyright Ericsson AB 2005-2011. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -87,14 +87,22 @@ end_per_testcase(Case, Config) -> %% Description: Returns a list of all test cases in this test suite %%-------------------------------------------------------------------- all() -> - [open, open_port, {group, passive}, {group, active}, - api_missuse, not_owner, {group, progress_report}]. + [ + open, + open_port, + {group, passive}, + {group, active}, + api_missuse, + not_owner, + {group, progress_report} + ]. groups() -> - [{passive, [], ftp_suite_lib:passive(suite)}, - {active, [], ftp_suite_lib:active(suite)}, - {progress_report, [], - ftp_suite_lib:progress_report(suite)}]. + [ + {passive, [], ftp_suite_lib:passive(suite)}, + {active, [], ftp_suite_lib:active(suite)}, + {progress_report, [], ftp_suite_lib:progress_report(suite)} + ]. init_per_group(_GroupName, Config) -> Config. -- cgit v1.2.3