[erlang-bugs] SSH Bugs

[Ingela Anderton Andin]

Hi!

On 07/01/2014 03:14 PM, Michael.K.Schmidt@REDACTED wrote:
> Hi Angela,

     ^^^^^^^^

     It is a Swedish name starts with I ;)	

>> Ingela Anderton Andin <Ingela.Anderton.Andin@REDACTED>
>> Sent by: erlang-bugs-bounces@REDACTED
>>
>> 07/01/2014 03:50 AM
>>
>> To
>>
>> <erlang-bugs@REDACTED>,
>>
>> cc
>>
>> Subject
>>
>> Re: [erlang-bugs] SSH Bugs
>>
>> Hi!
>>
>> On 06/30/2014 10:37 PM, Michael.K.Schmidt@REDACTED wrote:
>>>
>>> Hi,
>>>
>>> Does anyone know what the final resolution was with this bug?
>>> http://erlang.org/pipermail/erlang-patches/2013-December/004499.html
>>
>> Humm I can not remeber seeing this patch before! Maybe it got lost in
>> the process some how :(
>
> No worries! When I didn't see it in 17.1, I thought I would check to see if
> that was the case.
>
>>
>>
>>> The patch given by Anton Ryabkov seems to address the immediate bug.
> Is
>>> this fix ok?  Or is there a better way?
>>>
>>> With Anton's patch I am able to get this example to run:
>>>
> https://github.com/erlang/otp/blob/maint/lib/ssh/examples/ssh_sample_cli.erl
>
>>>
>>> It works with Tera Term as the client, but when I try to use PuTTY I
> get
>>> the following error:
>>> "Disconnected:  Server protocol violation: unexpected
>>> SSH2_MSG_CHANNEL_SUCCESS packet"
>>>
>>> I believe this worked with PuTTY before R16B03.  Did an edge case get
>>> missed with this refactor?
>>> https://github.com/erlang/otp/commit/
>> 68263a48bfbdac4dc219a91f06af3d535d881850
>>
>> Possible!
>>
>> Have you tried it with 17.1? I think the 17.1 code semantically seems to
>> be doing the same as the patch.
>
> I'm still having trouble with 17.1.  When I make Anton's changes, TeraTerm
> seems to work.
>
> As far as PuTTY, I assume the root cause is somewhere in
> ssh_connection.erl:handle_cli_msg() :
>
> handle_cli_msg(#connection{channel_cache = Cache} = Connection,
> 	       #channel{user = undefined,
> 			remote_id = RemoteId,
> 			local_id = ChannelId} = Channel0, Reply0) ->
>
>      case (catch start_cli(Connection, ChannelId)) of
> 	{ok, Pid} ->
> 	    erlang:monitor(process, Pid),
> 	    Channel = Channel0#channel{user = Pid},
> 	    ssh_channel:cache_update(Cache, Channel),
> 	    Reply = {connection_reply,
> 		     channel_success_msg(RemoteId)},
> 	    {{replies, [{channel_data, Pid, Reply0}, Reply]}, Connection};
> 	_Other ->
> 	    Reply = {connection_reply,
> 		     channel_failure_msg(RemoteId)},
> 	    {{replies, [Reply]}, Connection}
>      end;
>
> We start_cli() and send channel_success_msg().  Prior to the refactor I
> believe this was reply_msg() instead.  If I change
> 	    Reply = {connection_reply,
> 		     channel_success_msg(RemoteId)},
> 	    {{replies, [{channel_data, Pid, Reply0}, Reply]}, Connection};
> To:
> 	    {Reply, Connection1} = reply_msg(Channel, Connection, Reply0),
>   	    {{replies, [Reply]}, Connection1};
>
> Then PuTTY seems happy again. Does this seem OK?  Or am I messing something
> else up by "fixing" this.

Humm ... looks like the refactor went wrong, when I make this change 
which also conform with the next clause, the ssh test SUITE still 
passes. It would be great if you could make a pull-request and add a
test case for this.  I will make a ticket for this issue, but it is 
vacation times now so it will take a while for someone to look into the 
ticket.

Regards Ingela Erlang/OTP team - Ericsson AB