From a8b2748dfdb6e74d708667255adb6f4bfccadf78 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Tue, 5 Dec 2017 13:45:38 +0100 Subject: [PATCH 1/2] Support None for set_option(OPT_NETWORK_TIMEOUT) The setter for network timeout and timeout options now support None as value. None is mapped to infinity. Closes: https://github.com/python-ldap/python-ldap/issues/95 Signed-off-by: Christian Heimes --- Doc/reference/ldap.rst | 6 ++++-- Modules/options.c | 48 +++++++++++++++++++++++++++-------------- Tests/t_ldap_options.py | 46 +++++++++++++++++++++++++++++++-------- 3 files changed, 73 insertions(+), 27 deletions(-) diff --git a/Doc/reference/ldap.rst b/Doc/reference/ldap.rst index 1f7ae5b..82bc31c 100644 --- 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 @@ -1125,6 +1125,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 index 647f859..ea841a3 100644 --- a/Modules/options.c +++ b/Modules/options.c @@ -136,26 +136,42 @@ 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 { + /* Float valued timeval options, 'd' also handles int/long */ + if (!PyArg_Parse(value, "d:set_option", &doubleval)) { + /* clear error and try again with None */ + PyErr_Clear(); + if (PyNone_Check(value)) { + /* None is mapped to infinity timeout */ + doubleval = -1; + } + else { PyErr_Format( - PyExc_ValueError, - "timeout must be >= 0 or -1 for infinity, got %d", - option + 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 index f0bcc21..1e682f2 100644 --- 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,20 @@ 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) + 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 +153,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 From 12f193efaeae7c82a62f4b723a5e221dac26adef Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 7 Dec 2017 21:02:29 +0100 Subject: [PATCH 2/2] In timeout options, reword the message only of TypeError --- Modules/options.c | 30 ++++++++++++++++-------------- Tests/t_ldap_options.py | 2 ++ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/Modules/options.c b/Modules/options.c index ea841a3..ee606d4 100644 --- a/Modules/options.c +++ b/Modules/options.c @@ -136,20 +136,22 @@ LDAP_set_option(LDAPObject *self, int option, PyObject *value) break; case LDAP_OPT_TIMEOUT: case LDAP_OPT_NETWORK_TIMEOUT: - /* Float valued timeval options, 'd' also handles int/long */ - if (!PyArg_Parse(value, "d:set_option", &doubleval)) { - /* clear error and try again with None */ - PyErr_Clear(); - if (PyNone_Check(value)) { - /* None is mapped to infinity timeout */ - doubleval = -1; - } - else { - PyErr_Format( - PyExc_TypeError, - "A float or None is expected for timeout, got %.100s", - Py_TYPE(value)->tp_name - ); + /* 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; } } diff --git a/Tests/t_ldap_options.py b/Tests/t_ldap_options.py index 1e682f2..a681a9b 100644 --- a/Tests/t_ldap_options.py +++ b/Tests/t_ldap_options.py @@ -64,6 +64,8 @@ def _test_timeout(self, option): 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)