[#623] hard-limit large connection timeouts

Also, make sure that hard-limiting and error checks to happen at session
creation time.

Author: d-w-moore

irods/__init__.py

@@ -41,6 +41,8 @@ def client_logging(flag=True,handler=None):
 CHALLENGE_LEN = 64
 MAX_SQL_ROWS = 256
 DEFAULT_CONNECTION_TIMEOUT = 120
+# https://stackoverflow.com/questions/45704243/value-of-c-pytime-t-in-python
+MAXIMUM_CONNECTION_TIMEOUT = 9223372036
 
 AUTH_SCHEME_KEY = 'a_scheme'
 AUTH_USER_KEY = 'a_user'

irods/session.py

@@ -25,14 +25,13 @@
 from irods.exception import (NetworkException, NotImplementedInIRODSServer)
 from irods.password_obfuscation import decode
 from irods import NATIVE_AUTH_SCHEME, PAM_AUTH_SCHEMES
-from . import DEFAULT_CONNECTION_TIMEOUT
+from . import (DEFAULT_CONNECTION_TIMEOUT, MAXIMUM_CONNECTION_TIMEOUT)
 
 _fds = None
 _fds_lock = threading.Lock()
 _sessions = None
 _sessions_lock = threading.Lock()
 
-
 def _cleanup_remaining_sessions():
     for fd in list(_fds.keys()):
         if not fd.closed:
@@ -59,6 +58,7 @@ def _weakly_reference(ses):
 
 class NonAnonymousLoginWithoutPassword(RuntimeError): pass
 
+
 class iRODSSession(object):
 
     def library_features(self):
@@ -141,7 +141,8 @@ def __init__(self, configure = True, auto_cleanup = True, **kwargs):
         self._env_file = ''
         self._auth_file = ''
         self.do_configure = (kwargs if configure else {})
-        self._cached_connection_timeout = kwargs.pop('connection_timeout', DEFAULT_CONNECTION_TIMEOUT)
+        self._cached_connection_timeout = None
+        self.connection_timeout = kwargs.pop('connection_timeout', DEFAULT_CONNECTION_TIMEOUT)
         self.__configured = None
         if configure:
             self.__configured = self.configure(**kwargs)
@@ -366,16 +367,22 @@ def connection_timeout(self, seconds):
             exc = ValueError("Setting an iRODS connection_timeout to 0 seconds would make it non-blocking.")
             raise exc
         elif isinstance(seconds, Number):
-            if seconds < 0 or str(seconds) == 'nan' or str(abs(seconds)) == 'inf':
-                exc = ValueError("The iRODS connection_timeout may not be assigned a negative or otherwise rogue value (eg: NaN, Inf).")
+            # Note: We can handle infinities because -Inf < 0 and Inf > MAXIMUM_CONNECTION_TIMEOUT.
+            if seconds < 0 or str(seconds) == 'nan':
+                exc = ValueError("The iRODS connection_timeout may not be assigned a negative, out-of-bounds, or otherwise rogue value (eg: NaN, -Inf).")
                 raise exc
+            elif seconds > MAXIMUM_CONNECTION_TIMEOUT:
+                logging.getLogger(__name__).warning('Hard limiting connection timeout of %g to the maximum allowable value of %g',
+                    seconds, MAXIMUM_CONNECTION_TIMEOUT)
+                seconds = MAXIMUM_CONNECTION_TIMEOUT
         elif seconds is None:
             pass
         else:
             exc = ValueError("The iRODS connection_timeout must be assigned a positive int, positive float, or None.")
             raise exc
         self._cached_connection_timeout = seconds
-        self.pool.connection_timeout = seconds
+        if self.pool:
+            self.pool.connection_timeout = seconds
 
     @staticmethod
     def get_irods_password_file():

irods/test/connection_test.py

@@ -5,6 +5,7 @@
 import tempfile
 import unittest
 from irods.exception import NetworkException
+from irods import MAXIMUM_CONNECTION_TIMEOUT
 import irods.test.helpers as helpers
 from irods.test.helpers import (server_side_sleep, temporarily_assign_attribute as temp_setter)
 
@@ -159,8 +160,24 @@ def test_assigning_session_connection_timeout_to_invalid_values__issue_569(self)
             with self.assertRaises(ValueError):
                 sess.connection_timeout = value
 
-        # Test previously set value is unaffected
-        self.assertEqual(sess.connection_timeout, DESIRED_TIMEOUT)
+    def test_assigning_session_connection_timeout_to_large_values__issue_623(self):
+        # Test use of a too-large timeout in iRODSSession constructor as well as on assignment to the
+        # iRODSSession property 'connection_timeout'.  In both cases, error checking and hard-limiting
+        # should be immediate.
+        sess = helpers.make_session(connection_timeout = MAXIMUM_CONNECTION_TIMEOUT + 1)
+        # The session attribute '_cached_connection_timeout' is where the session timeout value is kept
+        # safe for whenever a Pool sub-object is initialized (or re-initialized).
+        self.assertEqual(sess._cached_connection_timeout, MAXIMUM_CONNECTION_TIMEOUT)
+
+        # Make (and check) a change of the connection_timeout value so that second of the surrounding
+        # equality assertions does not accidentally succeed due to the value remaining untouched.
+        sess.connection_timeout = 1
+        self.assertEqual(sess._cached_connection_timeout, 1)
+
+        sess.connection_timeout = MAXIMUM_CONNECTION_TIMEOUT + 1
+        self.assertEqual(sess._cached_connection_timeout, MAXIMUM_CONNECTION_TIMEOUT)
+
+        self.assertEqual(sess.pool.connection_timeout, MAXIMUM_CONNECTION_TIMEOUT)
 
     def test_assigning_session_connection_timeout__issue_569(self):
         sess = helpers.make_session()