Skip to content

Commit

Permalink
Improve interoperability with SSH clients
Browse files Browse the repository at this point in the history
This patch fixes a few problems of the SSH shell server that affect the
interoperability with SSH clients in widespread use.

First problem is that, whenever a channel_request message is
received with want_reply=true, the reply ends up being sent to the
servers channel id, not the clients channel id. This causes the client
to terminate the connection. The easiest solution to the problem appears
to be a new function in ssh_connection_manager.erl that translates the
servers channel id before sending the reply (in the same manner as
other functions do it).

Second problem is in ssh_cli.erl. When an SSH client sends a
window_change request between PTY allocation and starting the shell
(which appears to happen with some clients), ssh_cli.erl crashes
because #state.buf is yet 'undefined'. Allocating an empty buffer at PTY
allocation time solves the problem.

Affected SSH clients:
 - all clients based on SSH-2.0-TrileadSSH2Java_213 (problem #1)
 - SSH Term Pro (problem #2)
  • Loading branch information
Stefan Zegenhagen authored and proxyles committed Jul 24, 2012
1 parent 92eb89b commit 9a73dd6
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 3 deletions.
3 changes: 2 additions & 1 deletion lib/ssh/src/ssh_cli.erl
Expand Up @@ -81,7 +81,8 @@ handle_ssh_msg({ssh_cm, ConnectionManager,
height = not_zero(Height, 24),
pixel_width = PixWidth,
pixel_height = PixHeight,
modes = Modes}},
modes = Modes},
buf = empty_buf()},
set_echo(State),
ssh_connection:reply_request(ConnectionManager, WantReply,
success, ChannelId),
Expand Down
2 changes: 1 addition & 1 deletion lib/ssh/src/ssh_connection.erl
Expand Up @@ -177,7 +177,7 @@ close(ConnectionManager, ChannelId) ->
%% Description: Send status replies to requests that want such replies.
%%--------------------------------------------------------------------
reply_request(ConnectionManager, true, Status, ChannelId) ->
ConnectionManager ! {ssh_cm, self(), {Status, ChannelId}},
ssh_connection_manager:reply_request(ConnectionManager, Status, ChannelId),
ok;
reply_request(_,false, _, _) ->
ok.
Expand Down
15 changes: 14 additions & 1 deletion lib/ssh/src/ssh_connection_manager.erl
Expand Up @@ -40,7 +40,7 @@
close/2, stop/1, send/5,
send_eof/2]).

-export([open_channel/6, request/6, request/7, global_request/4, event/2,
-export([open_channel/6, reply_request/3, request/6, request/7, global_request/4, event/2,
cast/2]).

%% Internal application API and spawn
Expand Down Expand Up @@ -95,6 +95,9 @@ request(ConnectionManager, ChannelId, Type, true, Data, Timeout) ->
request(ConnectionManager, ChannelId, Type, false, Data, _) ->
cast(ConnectionManager, {request, ChannelId, Type, Data}).

reply_request(ConnectionManager, Status, ChannelId) ->
cast(ConnectionManager, {reply_request, Status, ChannelId}).

global_request(ConnectionManager, Type, true = Reply, Data) ->
case call(ConnectionManager,
{global_request, self(), Type, Reply, Data}) of
Expand Down Expand Up @@ -431,6 +434,16 @@ handle_cast({request, ChannelId, Type, Data}, State0) ->
lists:foreach(fun send_msg/1, Replies),
{noreply, State};

handle_cast({reply_request, Status, ChannelId}, #state{connection_state =
#connection{channel_cache = Cache}} = State0) ->
State = case ssh_channel:cache_lookup(Cache, ChannelId) of
#channel{remote_id = RemoteId} ->
cm_message({Status, RemoteId}, State0);
undefined ->
State0
end,
{noreply, State};

handle_cast({global_request, _, _, _, _} = Request, State0) ->
State = handle_global_request(Request, State0),
{noreply, State};
Expand Down

0 comments on commit 9a73dd6

Please sign in to comment.