Skip to content

Commit

Permalink
Fix macOS SDK builds without ldap_init_fd
Browse files Browse the repository at this point in the history
Fix macOS SDK builds without ldap_init_fd

macOS system libldap 2.4.28 does not have ldap_init_fd symbol. Disable
initialize_fd when Apple libldap 2.4.28 is detected.

Also run some macOS tests on Travis CI. Since the SDK does not ship
slapd, testing is rather limited.

https://github.com/python-ldap/python-ldap/pull/360
Fixes: https://github.com/python-ldap/python-ldap/issues/359
Signed-off-by: Christian Heimes <cheimes@redhat.com>
  • Loading branch information
Christian Heimes authored and GitHub committed Jun 29, 2020
1 parent 6f06059 commit 605a34b
Show file tree
Hide file tree
Showing 13 changed files with 75 additions and 2 deletions.
9 changes: 9 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
language: python
group: travis_latest

sudo: false

Expand All @@ -14,6 +15,13 @@ addons:
# Note: when updating Python versions, also change setup.py and tox.ini
matrix:
include:
- os: osx
osx_image: xcode11.4
language: minimal
env:
- TOXENV=macos
- CFLAGS_warnings="-Wall -Werror=declaration-after-statement"
- CFLAGS_std="-std=c99"
- python: 3.6
env:
- TOXENV=py36
Expand All @@ -25,6 +33,7 @@ matrix:
- python: 3.7
env:
- TOXENV=py37
- CFLAGS_std="-std=c99"
- WITH_GCOV=1
dist: xenial
sudo: true
Expand Down
8 changes: 8 additions & 0 deletions Doc/reference/ldap.rst
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ This module defines the following functions:
and explicitly closed after the :class:`~ldap.ldapobject.LDAPObject` is
unbound. The internal connection type is determined from the URI, ``TCP``
for ``ldap://`` / ``ldaps://``, ``IPC`` (``AF_UNIX``) for ``ldapi://``.
The parameter is not available on macOS when python-ldap is compiled with system
libldap, see :py:const:`INIT_FD_AVAIL`.

Note that internally the OpenLDAP function
`ldap_initialize(3) <https://www.openldap.org/software/man.cgi?query=ldap_init&sektion=3>`_
Expand Down Expand Up @@ -139,6 +141,12 @@ General
Integer where a non-zero value indicates that python-ldap was built with
support for SSL/TLS (OpenSSL or similar libs).

.. py:data:: INIT_FD_AVAIL
Integer where a non-zero value indicates that python-ldap supports
:py:func:`initialize` from a file descriptor. The feature is generally
available except on macOS when python-ldap is compiled with system libldap.


.. _ldap-options:

Expand Down
1 change: 1 addition & 0 deletions Lib/ldap/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ class Str(Constant):
Feature('LIBLDAP_R', 'HAVE_LIBLDAP_R'),
Feature('SASL_AVAIL', 'HAVE_SASL'),
Feature('TLS_AVAIL', 'HAVE_TLS'),
Feature('INIT_FD_AVAIL', 'HAVE_LDAP_INIT_FD'),

Str("CONTROL_MANAGEDSAIT"),
Str("CONTROL_PROXY_AUTHZ"),
Expand Down
2 changes: 2 additions & 0 deletions Lib/ldap/ldapobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ def __init__(
self._uri = uri
self._ldap_object_lock = self._ldap_lock('opcall')
if fileno is not None:
if not hasattr(_ldap, "initialize_fd"):
raise ValueError("libldap does not support initialize_fd")
if hasattr(fileno, "fileno"):
fileno = fileno.fileno()
self._l = ldap.functions._ldap_function_call(
Expand Down
1 change: 1 addition & 0 deletions Lib/slapdtest/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@

from slapdtest._slapdtest import SlapdObject, SlapdTestCase, SysLogHandler
from slapdtest._slapdtest import requires_ldapi, requires_sasl, requires_tls
from slapdtest._slapdtest import requires_init_fd
from slapdtest._slapdtest import skip_unless_ci
8 changes: 8 additions & 0 deletions Lib/slapdtest/_slapdtest.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,14 @@ def requires_ldapi():
else:
return identity

def requires_init_fd():
if not ldap.INIT_FD_AVAIL:
return skip_unless_ci(
"test needs ldap.INIT_FD", feature='INIT_FD')
else:
return identity


def _add_sbin(path):
"""Add /sbin and related directories to a command search path"""
directories = path.split(os.pathsep)
Expand Down
5 changes: 5 additions & 0 deletions Modules/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,16 @@
/* openldap.h with ldap_init_fd() was introduced in 2.4.48
* see https://bugs.openldap.org/show_bug.cgi?id=8671
*/
#define HAVE_LDAP_INIT_FD 1
#include <openldap.h>
#elif (defined(__APPLE__) && (LDAP_VENDOR_VERSION == 20428))
/* macOS system libldap 2.4.28 does not have ldap_init_fd symbol */
#undef HAVE_LDAP_INIT_FD
#else
/* ldap_init_fd() has been around for a very long time
* SSSD has been defining the function for a while, so it's probably OK.
*/
#define HAVE_LDAP_INIT_FD 1
#define LDAP_PROTO_TCP 1
#define LDAP_PROTO_UDP 2
#define LDAP_PROTO_IPC 3
Expand Down
8 changes: 8 additions & 0 deletions Modules/constants_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,14 @@ if (PyModule_AddIntConstant(m, "TLS_AVAIL", 0) != 0)
return -1;
#endif

#ifdef HAVE_LDAP_INIT_FD
if (PyModule_AddIntConstant(m, "INIT_FD_AVAIL", 1) != 0)
return -1;
#else
if (PyModule_AddIntConstant(m, "INIT_FD_AVAIL", 0) != 0)
return -1;
#endif

add_string(CONTROL_MANAGEDSAIT);
add_string(CONTROL_PROXY_AUTHZ);
add_string(CONTROL_SUBENTRIES);
Expand Down
4 changes: 4 additions & 0 deletions Modules/functions.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ l_ldap_initialize(PyObject *unused, PyObject *args)
return (PyObject *)newLDAPObject(ld);
}

#ifdef HAVE_LDAP_INIT_FD
/* initialize_fd(fileno, url) */

static PyObject *
Expand Down Expand Up @@ -82,6 +83,7 @@ l_ldap_initialize_fd(PyObject *unused, PyObject *args)

return (PyObject *)newLDAPObject(ld);
}
#endif

/* ldap_str2dn */

Expand Down Expand Up @@ -190,7 +192,9 @@ l_ldap_get_option(PyObject *self, PyObject *args)

static PyMethodDef methods[] = {
{"initialize", (PyCFunction)l_ldap_initialize, METH_VARARGS},
#ifdef HAVE_LDAP_INIT_FD
{"initialize_fd", (PyCFunction)l_ldap_initialize_fd, METH_VARARGS},
#endif
{"str2dn", (PyCFunction)l_ldap_str2dn, METH_VARARGS},
{"set_option", (PyCFunction)l_ldap_set_option, METH_VARARGS},
{"get_option", (PyCFunction)l_ldap_get_option, METH_VARARGS},
Expand Down
5 changes: 4 additions & 1 deletion Tests/t_cext.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

# import the plain C wrapper module
import _ldap
from slapdtest import SlapdTestCase, requires_tls
from slapdtest import SlapdTestCase, requires_tls, requires_init_fd


class TestLdapCExtension(SlapdTestCase):
Expand Down Expand Up @@ -248,19 +248,22 @@ def test_simple_bind_fileno(self):
with self._open_conn_fd() as (sock, l):
self.assertEqual(l.whoami_s(), "dn:" + self.server.root_dn)

@requires_init_fd()
def test_simple_bind_fileno_invalid(self):
with open(os.devnull) as f:
l = _ldap.initialize_fd(f.fileno(), self.server.ldap_uri)
with self.assertRaises(_ldap.SERVER_DOWN):
self._bind_conn(l)

@requires_init_fd()
def test_simple_bind_fileno_closed(self):
with self._open_conn_fd() as (sock, l):
self.assertEqual(l.whoami_s(), "dn:" + self.server.root_dn)
sock.close()
with self.assertRaises(_ldap.SERVER_DOWN):
l.whoami_s()

@requires_init_fd()
def test_simple_bind_fileno_rebind(self):
with self._open_conn_fd() as (sock, l):
self.assertEqual(l.whoami_s(), "dn:" + self.server.root_dn)
Expand Down
2 changes: 2 additions & 0 deletions Tests/t_ldapobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

from slapdtest import SlapdTestCase
from slapdtest import requires_ldapi, requires_sasl, requires_tls
from slapdtest import requires_init_fd


LDIF_TEMPLATE = """dn: %(suffix)s
Expand Down Expand Up @@ -543,6 +544,7 @@ def test105_reconnect_restore(self):
self.assertEqual(l1.whoami_s(), 'dn:'+bind_dn)


@requires_init_fd()
class Test03_SimpleLDAPObjectWithFileno(Test00_SimpleLDAPObject):
def _open_ldap_conn(self, who=None, cred=None, **kwargs):
if hasattr(self, '_sock'):
Expand Down
4 changes: 3 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
from setuptools import setup, Extension

if sys.version_info < (3, 6):
raise RuntimeError('The C API from Python 3.6+ is required.')
raise RuntimeError(
'The C API from Python 3.6+ is required, found %s' % sys.version_info
)

from configparser import ConfigParser

Expand Down
20 changes: 20 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,26 @@ setenv =
PYTHON_LDAP_TRACE_FILE={envtmpdir}/trace.log
commands = {[testenv]commands}

[testenv:macos]
# Travis CI macOS image does not have slapd
# SDK libldap does not support ldap_init_fd
basepython = python3
deps = {[testenv]deps}
passenv = {[testenv]passenv}
setenv =
CI_DISABLED=INIT_FD
commands =
{envpython} -m unittest -v \
Tests/t_cidict.py \
Tests/t_ldap_dn.py \
Tests/t_ldap_filter.py \
Tests/t_ldap_functions.py \
Tests/t_ldap_modlist.py \
Tests/t_ldap_schema_tokenizer.py \
Tests/t_ldapurl.py \
Tests/t_ldif.py \
Tests/t_untested_mods.py

[testenv:pypy3]
basepython = pypy3
deps = pytest
Expand Down

0 comments on commit 605a34b

Please sign in to comment.