Merge pull request #104 – Support None for set_option(OPT_NETWORK_TIM…

…EOUT) https://github.com/python-ldap/python-ldap/pull/104
ECN · Dec 11, 2017 · 4d3cd7d · 4d3cd7d
2 parents 31abc2e + 12f193e
commit 4d3cd7d
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 29 deletions.
diff --git a/Doc/reference/ldap.rst b/Doc/reference/ldap.rst
@@ -157,7 +157,7 @@ following option identifiers are defined as constants:
 .. py:data:: OPT_NETWORK_TIMEOUT
 
    .. versionchanged:: 3.0
-      A timeout of ``-1`` resets timeout to infinity.
+      A timeout of ``-1`` or ``None`` resets timeout to infinity.
 
 .. py:data:: OPT_PROTOCOL_VERSION
 
@@ -184,7 +184,7 @@ following option identifiers are defined as constants:
 .. py:data:: OPT_TIMEOUT
 
    .. versionchanged:: 3.0
-      A timeout of ``-1`` resets timeout to infinity.
+      A timeout of ``-1`` or ``None`` resets timeout to infinity.
 
 .. py:data:: OPT_URI
 
@@ -1137,6 +1137,8 @@ These attributes are mutable unless described as read-only.
    This option is mapped to option constant :py:const:`OPT_NETWORK_TIMEOUT`
    and used in the underlying OpenLDAP client lib.
 
+   .. versionchanged:: 3.0.0
+      A timeout of ``-1`` or ``None`` resets timeout to infinity.
 
 .. py:attribute:: LDAPObject.protocol_version -> int
 

diff --git a/Modules/options.c b/Modules/options.c
@@ -136,26 +136,44 @@ LDAP_set_option(LDAPObject *self, int option, PyObject *value)
 	    break;
     case LDAP_OPT_TIMEOUT:
     case LDAP_OPT_NETWORK_TIMEOUT:
-	    /* Float valued timeval options */
-	    if (!PyArg_Parse(value, "d:set_option", &doubleval))
-		return 0;
-            if (doubleval >= 0) {
-                set_timeval_from_double( &tv, doubleval );
-                ptr = &tv;
-            } else if (doubleval == -1) {
-                /* -1 is infinity timeout */
-                tv.tv_sec = -1;
-                tv.tv_usec = 0;
-                ptr = &tv;
-            } else {
-                PyErr_Format(
-                    PyExc_ValueError,
-                    "timeout must be >= 0 or -1 for infinity, got %d",
-                    option
-                );
+        /* Float valued timeval options */
+        if (value == Py_None) {
+            /* None is mapped to infinity timeout */
+            doubleval = -1;
+        } else {
+            /* 'd' handles int/long */
+            if (!PyArg_Parse(value, "d:set_option", &doubleval)) {
+                if (PyErr_ExceptionMatches(PyExc_TypeError)) {
+                    /* TypeError: mention either float or None is expected */
+                    PyErr_Clear();
+                    PyErr_Format(
+                        PyExc_TypeError,
+                        "A float or None is expected for timeout, got %.100s",
+                        Py_TYPE(value)->tp_name
+                    );
+                }
                 return 0;
             }
-	    break;
+        }
+
+        if (doubleval >= 0) {
+            set_timeval_from_double( &tv, doubleval );
+            ptr = &tv;
+        } else if (doubleval == -1) {
+            /* -1 is infinity timeout */
+            tv.tv_sec = -1;
+            tv.tv_usec = 0;
+            ptr = &tv;
+        } else {
+            PyErr_Format(
+                PyExc_ValueError,
+                "timeout must be >= 0 or -1/None for infinity, got %d",
+                option
+            );
+            return 0;
+        }
+        break;
+
     case LDAP_OPT_SERVER_CONTROLS:
     case LDAP_OPT_CLIENT_CONTROLS:
             if (!LDAPControls_from_object(value, &controls))

diff --git a/Tests/t_ldap_options.py b/Tests/t_ldap_options.py
@@ -38,8 +38,7 @@ def get_option(self, option):
     def set_option(self, option, value):
         raise NotImplementedError()
 
-    def _check_option(self, option, value, expected=SENTINEL,
-                      nonevalue=None):
+    def _check_option(self, option, value, expected=SENTINEL):
         old = self.get_option(option)
         try:
             self.set_option(option, value)
@@ -49,10 +48,7 @@ def _check_option(self, option, value, expected=SENTINEL,
             else:
                 self.assertEqual(new, expected)
         finally:
-            self.set_option(
-                option,
-                old if old is not None else nonevalue
-            )
+            self.set_option(option, old)
             self.assertEqual(self.get_option(option), old)
 
     def test_invalid(self):
@@ -62,12 +58,22 @@ def test_invalid(self):
             self.set_option(-1, '')
 
     def _test_timeout(self, option):
-        self._check_option(option, 10.5, nonevalue=-1)
-        self._check_option(option, 0, nonevalue=-1)
+        self._check_option(option, 10.5)
+        self._check_option(option, 0)
         with self.assertRaises(ValueError):
-            self._check_option(option, -5, nonevalue=-1)
+            self._check_option(option, -5)
         with self.assertRaises(TypeError):
             self.set_option(option, object)
+        with self.assertRaises(OverflowError):
+            self._check_option(option, 10**1000)
+        old = self.get_option(option)
+        try:
+            self.set_option(option, None)
+            self.assertEqual(self.get_option(option), None)
+            self.set_option(option, -1)
+            self.assertEqual(self.get_option(option), None)
+        finally:
+            self.set_option(option, old)
 
     def test_timeout(self):
         self._test_timeout(ldap.OPT_TIMEOUT)
@@ -149,6 +155,30 @@ def get_option(self, option):
     def set_option(self, option, value):
         return self.conn.set_option(option, value)
 
+    def test_network_timeout_attribute(self):
+        option = ldap.OPT_NETWORK_TIMEOUT
+        old = self.get_option(option)
+        try:
+            self.assertEqual(self.conn.network_timeout, old)
+
+            self.conn.network_timeout = 5
+            self.assertEqual(self.conn.network_timeout, 5)
+            self.assertEqual(self.get_option(option), 5)
+
+            self.conn.network_timeout = -1
+            self.assertEqual(self.conn.network_timeout, None)
+            self.assertEqual(self.get_option(option), None)
+
+            self.conn.network_timeout = 10.5
+            self.assertEqual(self.conn.network_timeout, 10.5)
+            self.assertEqual(self.get_option(option), 10.5)
+
+            self.conn.network_timeout = None
+            self.assertEqual(self.conn.network_timeout, None)
+            self.assertEqual(self.get_option(option), None)
+        finally:
+            self.set_option(option, old)
+
     # test is failing with:
     # pyasn1.error.SubstrateUnderrunError: Short octet stream on tag decoding
     @unittest.expectedFailure