File 1251-ssh-Use-connect_timeout-as-default-for-neg... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 1251-ssh-Use-connect_timeout-as-default-for-negotiation-t.patch of Package erlang (Revision 3e585b5159e575cdaf8b197320891957)

Currently displaying revision 3e585b5159e575cdaf8b197320891957 , Show latest

From e6712b804184c0937b1e3a3c36fb477410ca9071 Mon Sep 17 00:00:00 2001
From: Hans Nilsson <hans@erlang.org>
Date: Thu, 14 Oct 2021 09:44:04 +0200
Subject: [PATCH] ssh: Use connect_timeout as default for negotiation timeout
 instead of infinity

---
 lib/ssh/doc/src/ssh.xml               |  4 ++-
 lib/ssh/src/ssh.erl                   |  3 ++-
 lib/ssh/src/ssh_sftp.erl              | 36 +++++++++++++++++----------
 lib/ssh/test/ssh_connection_SUITE.erl | 11 ++++++++
 4 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/lib/ssh/doc/src/ssh.xml b/lib/ssh/doc/src/ssh.xml
index 377643e69e..a9db9f418b 100644
--- a/lib/ssh/doc/src/ssh.xml
+++ b/lib/ssh/doc/src/ssh.xml
@@ -1274,7 +1274,9 @@
 No channel is started. This is done by calling <seemfa marker="ssh_connection#session_channel/2">
 ssh_connection:session_channel/[2, 4]</seemfa>.
 
- The <c>NegotiationTimeout</c> is in milli-seconds. The default value is <c>infinity</c>.
+ The <c>NegotiationTimeout</c> is in milli-seconds. The default value is <c>infinity</c> or
+ the value of the <seetype marker="#connect_timeout_client_option"><c>connect_timeout</c></seetype>
+ option, if present.
 For connection timeout, use the option
 <seetype marker="#connect_timeout_client_option"><c>connect_timeout</c></seetype>.
 
diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl
index 467e429755..f655f0db45 100644
--- a/lib/ssh/src/ssh.erl
+++ b/lib/ssh/src/ssh.erl
@@ -151,7 +151,8 @@ connect(_OpenTcpSocket, Options) ->
 
 connect(Host, Port, Options) when ?IS_VALID_PORT(Port),
 ?IS_VALID_OPTIONS(Options) ->
- connect(Host, Port, Options, infinity);
+ Timeout = proplists:get_value(connect_timeout, Options, infinity),
+ connect(Host, Port, Options, Timeout);
 connect(Socket, UserOptions, NegotiationTimeout)
 when ?IS_VALID_OPTIONS(UserOptions),
 ?IS_VALID_TIMEOUT(NegotiationTimeout) ->
diff --git a/lib/ssh/src/ssh_sftp.erl b/lib/ssh/src/ssh_sftp.erl
index 22997d863a..015b2f2a30 100644
--- a/lib/ssh/src/ssh_sftp.erl
+++ b/lib/ssh/src/ssh_sftp.erl
@@ -135,7 +135,8 @@ start_channel(Dest) ->
 )
 -> {ok,pid(),ssh:connection_ref()} | {error,reason()} .
 
-start_channel(Cm, UserOptions) when is_pid(Cm) ->
+start_channel(Cm, UserOptions0) when is_pid(Cm) ->
+ UserOptions = legacy_timeout(UserOptions0),
 Timeout = proplists:get_value(timeout, UserOptions, infinity),
 {_SshOpts, ChanOpts, SftpOpts} = handle_options(UserOptions),
 WindowSize = proplists:get_value(window_size, ChanOpts, ?XFER_WINDOW_SIZE),
@@ -158,7 +159,8 @@ start_channel(Cm, UserOptions) when is_pid(Cm) ->
 	 Error
 end;
 
-start_channel(Dest, UserOptions) ->
+start_channel(Dest, UserOptions0) ->
+ UserOptions = legacy_timeout(UserOptions0),
 {SshOpts, ChanOpts, SftpOpts} = handle_options(UserOptions),
 case ssh:is_host(Dest, SshOpts) of
 true ->
@@ -167,9 +169,7 @@ start_channel(Dest, UserOptions) ->
 false ->
 %% No, it is probably not a Host, must be a socket
 Socket = Dest,
- Timeout = % A mixture of ssh:connect and ssh_sftp:start_channel:
- proplists:get_value(connect_timeout, SshOpts,
- proplists:get_value(timeout, SftpOpts, infinity)),
+ Timeout = proplists:get_value(connect_timeout, SshOpts, infinity),
 case ssh:connect(Socket, SshOpts, Timeout) of
 {ok,Cm} ->
 case start_channel(Cm, ChanOpts ++ SftpOpts) of
@@ -193,15 +193,10 @@ start_channel(Dest, UserOptions) ->
 )
 -> {ok,pid(),ssh:connection_ref()} | {error,reason()}.
 
-start_channel(Host, Port, UserOptions) ->
+start_channel(Host, Port, UserOptions0) ->
+ UserOptions = legacy_timeout(UserOptions0),
+ Timeout = proplists:get_value(connect_timeout, UserOptions, infinity),
 {SshOpts, _ChanOpts, _SftpOpts} = handle_options(UserOptions),
- Timeout = % A mixture of ssh:connect and ssh_sftp:start_channel:
- case proplists:get_value(connect_timeout, UserOptions) of
- undefined ->
- proplists:get_value(timeout, UserOptions, infinity);
- TO ->
- TO
- end,
 case ssh:connect(Host, Port, SshOpts, Timeout) of
 	{ok, Cm} ->
 case start_channel(Cm, UserOptions) of
@@ -1228,6 +1223,21 @@ terminate(_Reason, State) ->
 %%====================================================================
 %% Internal functions
 %%====================================================================
+legacy_timeout(UserOptions) ->
+ %% Make both connect_timeout and timeout defined if exaclty one of them is defined:
+ case {proplists:get_value(connect_timeout, UserOptions),
+ proplists:get_value(timeout, UserOptions)} of
+ {undefined, undefined} ->
+ UserOptions;
+ {undefined, TO} ->
+ [{connect_timeout,TO} | UserOptions];
+ {TO, undefined} ->
+ [{timeout,TO} | UserOptions];
+ {_, _} ->
+ UserOptions
+ end.
+
+
 handle_options(UserOptions) ->
 handle_options(UserOptions, [], [], []).
 
diff --git a/lib/ssh/test/ssh_connection_SUITE.erl b/lib/ssh/test/ssh_connection_SUITE.erl
index cda08d47a9..1e79f0a622 100644
--- a/lib/ssh/test/ssh_connection_SUITE.erl
+++ b/lib/ssh/test/ssh_connection_SUITE.erl
@@ -57,6 +57,7 @@
 connect4_invalid_two_1/1,
 connect4_invalid_two_2/1,
 connect4_invalid_three/1,
+ connect_timeout/1,
 daemon_sock_not_passive/1,
 daemon_sock_not_tcp/1,
 do_interrupted_send/3,
@@ -160,6 +161,7 @@ all() ->
 connect4_invalid_two_1,
 connect4_invalid_two_2,
 connect4_invalid_three,
+ connect_timeout,
 daemon_sock_not_tcp,
 gracefull_invalid_version,
 gracefull_invalid_start,
@@ -408,6 +410,15 @@ connect_sock_not_tcp(_Config) ->
 {user_interaction, true}]),
 gen_udp:close(Sock).
 
+%%--------------------------------------------------------------------
+connect_timeout(_Config) ->
+ {ok,Sl} = gen_tcp:listen(0, []),
+ {ok, {_,Port}} = inet:sockname(Sl),
+ {error,timeout} = ssh:connect(loopback, Port, [{connect_timeout,2000},
+ {save_accepted_host, false},
+ {silently_accept_hosts, true}]),
+ gen_tcp:close(Sl).
+
 %%--------------------------------------------------------------------
 daemon_sock_not_tcp(_Config) ->
 {ok,Sock} = gen_udp:open(0, []), 
-- 
2.31.1

Places

File 1251-ssh-Use-connect_timeout-as-default-for-negotiation-t.patch of Package erlang (Revision 3e585b5159e575cdaf8b197320891957)

Places