From 75ca7672b5c7bb07196a3a2b294157479ff4f00a Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Mon, 15 Dec 2014 09:54:26 +0100 Subject: ssh: Improve errorhandling in ssh_connection.erl If a channel is closed by the peer while using a function with call semantics in ssh_connection.erl return {error, closed}. Document that the functions can return {error, timeout | closed} and not only ssh_request_status() --- lib/ssh/doc/src/ssh_connection.xml | 14 ++++++++------ lib/ssh/src/ssh_connection.erl | 22 ++++++++++++++-------- lib/ssh/src/ssh_connection_handler.erl | 15 ++++++++++++--- lib/ssh/test/ssh_connection_SUITE.erl | 28 +++++++++++++++++++++++++++- 4 files changed, 61 insertions(+), 18 deletions(-) diff --git a/lib/ssh/doc/src/ssh_connection.xml b/lib/ssh/doc/src/ssh_connection.xml index ff72cf7ee0..5e2926dfa6 100644 --- a/lib/ssh/doc/src/ssh_connection.xml +++ b/lib/ssh/doc/src/ssh_connection.xml @@ -62,6 +62,7 @@

ssh_request_status() = success | failure

event() = {ssh_cm, ssh_connection_ref(), ssh_event_msg()}

ssh_event_msg() = data_events() | status_events() | terminal_events()

+

reason() = timeout | closed

data_events() @@ -218,7 +219,7 @@ - exec(ConnectionRef, ChannelId, Command, TimeOut) -> ssh_request_status() + exec(ConnectionRef, ChannelId, Command, TimeOut) -> ssh_request_status() | {error, reason()} Request that the server start the execution of the given command. ConnectionRef = ssh_connection_ref() @@ -274,7 +275,8 @@ - ptty_alloc(ConnectionRef, ChannelId, Options, Timeout) -> success | failure + ptty_alloc(ConnectionRef, ChannelId, Options) -> + ptty_alloc(ConnectionRef, ChannelId, Options, Timeout) -> > ssh_request_status() | {error, reason()} Send status replies to requests that want such replies. ConnectionRef = ssh_connection_ref() @@ -374,7 +376,7 @@ session_channel(ConnectionRef, Timeout) -> session_channel(ConnectionRef, InitialWindowSize, - MaxPacketSize, Timeout) -> {ok, ssh_channel_id()} | {error, Reason} + MaxPacketSize, Timeout) -> {ok, ssh_channel_id()} | {error, reason()} Opens a channel for a ssh session. ConnectionRef = ssh_connection_ref() @@ -391,7 +393,7 @@ - setenv(ConnectionRef, ChannelId, Var, Value, TimeOut) -> ssh_request_status() + setenv(ConnectionRef, ChannelId, Var, Value, TimeOut) -> ssh_request_status() | {error, reason()} Environment variables may be passed to the shell/command to be started later. @@ -409,7 +411,7 @@ - shell(ConnectionRef, ChannelId) -> ssh_request_status() + shell(ConnectionRef, ChannelId) -> ssh_request_status() | {error, closed} Requests that the user's default shell (typically defined in /etc/passwd in UNIX systems) shall be executed at the server @@ -426,7 +428,7 @@ - subsystem(ConnectionRef, ChannelId, Subsystem, Timeout) -> ssh_request_status() + subsystem(ConnectionRef, ChannelId, Subsystem, Timeout) -> ssh_request_status() | {error, reason()} ConnectionRef = ssh_connection_ref() diff --git a/lib/ssh/src/ssh_connection.erl b/lib/ssh/src/ssh_connection.erl index 01141622d6..c66f810948 100644 --- a/lib/ssh/src/ssh_connection.erl +++ b/lib/ssh/src/ssh_connection.erl @@ -56,8 +56,8 @@ %%-------------------------------------------------------------------- %%-------------------------------------------------------------------- --spec session_channel(pid(), timeout()) -> {ok, channel_id()} | {error, term()}. --spec session_channel(pid(), integer(), integer(), timeout()) -> {ok, channel_id()} | {error, term()}. +-spec session_channel(pid(), timeout()) -> {ok, channel_id()} | {error, timeout | closed}. +-spec session_channel(pid(), integer(), integer(), timeout()) -> {ok, channel_id()} | {error, timeout | closed}. %% Description: Opens a channel for a ssh session. A session is a %% remote execution of a program. The program may be a shell, an @@ -81,7 +81,8 @@ session_channel(ConnectionHandler, InitialWindowSize, end. %%-------------------------------------------------------------------- --spec exec(pid(), channel_id(), string(), timeout()) -> success | failure. +-spec exec(pid(), channel_id(), string(), timeout()) -> + success | failure | {error, timeout | closed}. %% Description: Will request that the server start the %% execution of the given command. @@ -101,8 +102,8 @@ shell(ConnectionHandler, ChannelId) -> ssh_connection_handler:request(ConnectionHandler, self(), ChannelId, "shell", false, <<>>, 0). %%-------------------------------------------------------------------- --spec subsystem(pid(), channel_id(), string(), timeout()) -> - success | failure | {error, timeout}. +-spec subsystem(pid(), channel_id(), string(), timeout()) -> + success | failure | {error, timeout | closed}. %% %% Description: Executes a predefined subsystem. %%-------------------------------------------------------------------- @@ -142,7 +143,7 @@ send_eof(ConnectionHandler, Channel) -> ssh_connection_handler:send_eof(ConnectionHandler, Channel). %%-------------------------------------------------------------------- --spec adjust_window(pid(), channel_id(), integer()) -> ok. +-spec adjust_window(pid(), channel_id(), integer()) -> ok | {error, closed}. %% %% %% Description: Adjusts the ssh flowcontrol window. @@ -151,7 +152,8 @@ adjust_window(ConnectionHandler, Channel, Bytes) -> ssh_connection_handler:adjust_window(ConnectionHandler, Channel, Bytes). %%-------------------------------------------------------------------- --spec setenv(pid(), channel_id(), string(), string(), timeout()) -> success | failure. +-spec setenv(pid(), channel_id(), string(), string(), timeout()) -> + success | failure | {error, timeout | closed}. %% %% %% Description: Environment variables may be passed to the shell/command to be @@ -183,7 +185,11 @@ reply_request(_,false, _, _) -> ok. %%-------------------------------------------------------------------- --spec ptty_alloc(pid(), channel_id(), proplists:proplist()) -> success | failiure. +-spec ptty_alloc(pid(), channel_id(), proplists:proplist()) -> + success | failiure | {error, closed}. +-spec ptty_alloc(pid(), channel_id(), proplists:proplist(), timeout()) -> + success | failiure | {error, timeout} | {error, closed}. + %% %% %% Description: Sends a ssh connection protocol pty_req. diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl index fdb9d3b3e6..915060c426 100644 --- a/lib/ssh/src/ssh_connection_handler.erl +++ b/lib/ssh/src/ssh_connection_handler.erl @@ -289,8 +289,13 @@ renegotiate_data(ConnectionHandler) -> -spec close(pid(), channel_id()) -> ok. %%-------------------------------------------------------------------- close(ConnectionHandler, ChannelId) -> - sync_send_all_state_event(ConnectionHandler, {close, ChannelId}). - + case sync_send_all_state_event(ConnectionHandler, {close, ChannelId}) of + ok -> + ok; + {error, closed} -> + ok + end. + %%-------------------------------------------------------------------- -spec stop(pid()) -> ok | {error, term()}. %%-------------------------------------------------------------------- @@ -1204,7 +1209,11 @@ sync_send_all_state_event(FsmPid, Event) -> sync_send_all_state_event(FsmPid, Event, infinity). sync_send_all_state_event(FsmPid, Event, Timeout) -> - try gen_fsm:sync_send_all_state_event(FsmPid, Event, Timeout) + try gen_fsm:sync_send_all_state_event(FsmPid, Event, Timeout) of + {closed, _Channel} -> + {error, closed}; + Result -> + Result catch exit:{noproc, _} -> {error, closed}; diff --git a/lib/ssh/test/ssh_connection_SUITE.erl b/lib/ssh/test/ssh_connection_SUITE.erl index a73573e7fe..e3871b3feb 100644 --- a/lib/ssh/test/ssh_connection_SUITE.erl +++ b/lib/ssh/test/ssh_connection_SUITE.erl @@ -45,7 +45,8 @@ all() -> gracefull_invalid_start, gracefull_invalid_long_start, gracefull_invalid_long_start_no_nl, - stop_listener + stop_listener, + start_subsystem_on_closed_channel ]. groups() -> [{openssh, [], payload() ++ ptty()}]. @@ -575,6 +576,31 @@ stop_listener(Config) when is_list(Config) -> ct:fail({unexpected, Error}) end. +start_subsystem_on_closed_channel(Config) -> + PrivDir = ?config(priv_dir, Config), + UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth + file:make_dir(UserDir), + SysDir = ?config(data_dir, Config), + {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir}, + {user_dir, UserDir}, + {password, "morot"}, + {subsystems, [{"echo_n", {ssh_echo_server, [4000000]}}]}]), + + ConnectionRef = ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true}, + {user, "foo"}, + {password, "morot"}, + {user_interaction, false}, + {user_dir, UserDir}]), + + {ok, ChannelId} = ssh_connection:session_channel(ConnectionRef, infinity), + + ok = ssh_connection:close(ConnectionRef, ChannelId), + + {error, closed} = ssh_connection:subsystem(ConnectionRef, ChannelId, "echo_n", infinity), + + ssh:close(ConnectionRef), + ssh:stop_daemon(Pid). + %%-------------------------------------------------------------------- %% Internal functions ------------------------------------------------ %%-------------------------------------------------------------------- -- cgit v1.2.3