Skip to content

Commit 5687863

Browse files
encukouencukou
authored
Merge pull request python-ldap#90 – Fix set_option() of timeout to handle -1 as infinity
python-ldap#90
2 parents c5ad802 + 2aa8bca commit 5687863

5 files changed

Lines changed: 129 additions & 25 deletions

File tree

‎CHANGES‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ Modules/
3636
* Fix memory leak in whoami
3737
* Fix internal error handling of LDAPControl_to_List()
3838
* Fix two memory leaks and release GIL in encode_assertion_control
39+
* Allow set_option() to set timeouts to infinity
3940
and, thanks to Michael Ströder:
4041
* removed unused code schema.c
4142
* moved code from version.c to ldapmodule.c

‎Doc/reference/ldap.rst‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,9 @@ following option identifiers are defined as constants:
156156
157157
.. py:data:: OPT_NETWORK_TIMEOUT
158158
159+
.. versionchanged:: 3.0
160+
A timeout of ``-1`` resets timeout to infinity.
161+
159162
.. py:data:: OPT_PROTOCOL_VERSION
160163
161164
Sets the LDAP protocol version used for a connection. This is mapped to
@@ -180,6 +183,9 @@ following option identifiers are defined as constants:
180183
181184
.. py:data:: OPT_TIMEOUT
182185
186+
.. versionchanged:: 3.0
187+
A timeout of ``-1`` resets timeout to infinity.
188+
183189
.. py:data:: OPT_URI
184190
185191
.. _ldap-sasl-options:

‎Modules/options.c‎

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,20 @@ LDAP_set_option(LDAPObject *self, int option, PyObject *value)
140140
if (!PyArg_Parse(value, "d:set_option", &doubleval))
141141
return 0;
142142
if (doubleval >= 0) {
143-
set_timeval_from_double( &tv, doubleval );
143+
set_timeval_from_double( &tv, doubleval );
144+
ptr = &tv;
145+
} else if (doubleval == -1) {
146+
/* -1 is infinity timeout */
147+
tv.tv_sec = -1;
148+
tv.tv_usec = 0;
144149
ptr = &tv;
145150
} else {
146-
ptr = NULL;
151+
PyErr_Format(
152+
PyExc_ValueError,
153+
"timeout must be >= 0 or -1 for infinity, got %d",
154+
option
155+
);
156+
return 0;
147157
}
148158
break;
149159
case LDAP_OPT_SERVER_CONTROLS:
@@ -180,10 +190,9 @@ LDAP_get_option(LDAPObject *self, int option)
180190
struct timeval *tv;
181191
LDAPAPIInfo apiinfo;
182192
LDAPControl **lcs;
183-
LDAPControl *lc;
184193
char *strval;
185-
PyObject *extensions, *v, *tup;
186-
Py_ssize_t i, num_extensions, num_controls;
194+
PyObject *extensions, *v;
195+
Py_ssize_t i, num_extensions;
187196
LDAP *ld;
188197

189198
ld = self ? self->ldap : NULL;
@@ -352,27 +361,8 @@ LDAP_get_option(LDAPObject *self, int option)
352361
if (res != LDAP_OPT_SUCCESS)
353362
return option_error(res, "ldap_get_option");
354363

355-
if (lcs == NULL)
356-
return PyList_New(0);
357-
358-
/* Get the number of controls */
359-
num_controls = 0;
360-
while (lcs[num_controls])
361-
num_controls++;
362-
363-
/* We'll build a list of controls, with each control a tuple */
364-
v = PyList_New(num_controls);
365-
for (i = 0; i < num_controls; i++) {
366-
lc = lcs[i];
367-
tup = Py_BuildValue("(sbs)",
368-
lc->ldctl_oid,
369-
lc->ldctl_iscritical,
370-
lc->ldctl_value.bv_val);
371-
PyList_SET_ITEM(v, i, tup);
372-
}
373-
364+
v = LDAPControls_to_List(lcs);
374365
ldap_controls_free(lcs);
375-
376366
return v;
377367

378368
default:

‎Tests/__init__.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,4 @@
2222
from . import t_ldap_schema_subentry
2323
from . import t_untested_mods
2424
from . import t_ldap_controls_libldap
25+
from . import t_ldap_options

‎Tests/t_ldap_options.py‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
import os
2+
import unittest
3+
4+
# Switch off processing .ldaprc or ldap.conf before importing _ldap
5+
os.environ['LDAPNOINIT'] = '1'
6+
7+
import ldap
8+
from ldap.controls import RequestControlTuples
9+
from ldap.controls.pagedresults import SimplePagedResultsControl
10+
from ldap.controls.openldap import SearchNoOpControl
11+
from slapdtest import requires_tls
12+
13+
14+
SENTINEL = object()
15+
16+
TEST_CTRL = RequestControlTuples([
17+
# with BER data
18+
SimplePagedResultsControl(criticality=0, size=5, cookie=b'cookie'),
19+
# value-less
20+
SearchNoOpControl(criticality=1),
21+
])
22+
TEST_CTRL_EXPECTED = [
23+
TEST_CTRL[0],
24+
# get_option returns empty bytes
25+
(TEST_CTRL[1][0], TEST_CTRL[1][1], b''),
26+
]
27+
28+
29+
class TestGlobalOptions(unittest.TestCase):
30+
def _check_option(self, option, value, expected=SENTINEL,
31+
nonevalue=None):
32+
old = ldap.get_option(option)
33+
try:
34+
ldap.set_option(option, value)
35+
new = ldap.get_option(option)
36+
if expected is SENTINEL:
37+
self.assertEqual(new, value)
38+
else:
39+
self.assertEqual(new, expected)
40+
finally:
41+
ldap.set_option(option, old if old is not None else nonevalue)
42+
self.assertEqual(ldap.get_option(option), old)
43+
44+
def test_invalid(self):
45+
with self.assertRaises(ValueError):
46+
ldap.get_option(-1)
47+
with self.assertRaises(ValueError):
48+
ldap.set_option(-1, '')
49+
50+
def test_timeout(self):
51+
self._check_option(ldap.OPT_TIMEOUT, 0, nonevalue=-1)
52+
self._check_option(ldap.OPT_TIMEOUT, 10.5, nonevalue=-1)
53+
with self.assertRaises(ValueError):
54+
self._check_option(ldap.OPT_TIMEOUT, -5, nonevalue=-1)
55+
with self.assertRaises(TypeError):
56+
ldap.set_option(ldap.OPT_TIMEOUT, object)
57+
58+
def test_network_timeout(self):
59+
self._check_option(ldap.OPT_NETWORK_TIMEOUT, 0, nonevalue=-1)
60+
self._check_option(ldap.OPT_NETWORK_TIMEOUT, 10.5, nonevalue=-1)
61+
with self.assertRaises(ValueError):
62+
self._check_option(ldap.OPT_NETWORK_TIMEOUT, -5, nonevalue=-1)
63+
64+
def _test_controls(self, option):
65+
self._check_option(option, [])
66+
self._check_option(option, TEST_CTRL, TEST_CTRL_EXPECTED)
67+
self._check_option(option, tuple(TEST_CTRL), TEST_CTRL_EXPECTED)
68+
with self.assertRaises(TypeError):
69+
ldap.set_option(option, object)
70+
71+
with self.assertRaises(TypeError):
72+
# must contain a tuple
73+
ldap.set_option(option, [list(TEST_CTRL[0])])
74+
with self.assertRaises(TypeError):
75+
# data must be bytes or None
76+
ldap.set_option(
77+
option,
78+
[TEST_CTRL[0][0], TEST_CTRL[0][1], u'data']
79+
)
80+
81+
def test_client_controls(self):
82+
self._test_controls(ldap.OPT_CLIENT_CONTROLS)
83+
84+
def test_server_controls(self):
85+
self._test_controls(ldap.OPT_SERVER_CONTROLS)
86+
87+
def test_uri(self):
88+
self._check_option(ldap.OPT_URI, "ldapi:///path/to/socket")
89+
with self.assertRaises(TypeError):
90+
ldap.set_option(ldap.OPT_URI, object)
91+
92+
@requires_tls()
93+
def test_cafile(self):
94+
# None or a distribution or OS-specific path
95+
ldap.get_option(ldap.OPT_X_TLS_CACERTFILE)
96+
97+
def test_readonly(self):
98+
value = ldap.get_option(ldap.OPT_API_INFO)
99+
self.assertIsInstance(value, dict)
100+
with self.assertRaises(ValueError) as e:
101+
ldap.set_option(ldap.OPT_API_INFO, value)
102+
self.assertIn('read-only', str(e.exception))
103+
104+
105+
if __name__ == '__main__':
106+
unittest.main()

0 commit comments

Comments
 (0)