From bdfeefeb4fd6fa71eadd1462a1de43ae449d01f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Barrois?= Date: Mon, 9 Dec 2013 11:42:45 +0100 Subject: [PATCH] [Py3] Always use *bytes* for values. Keep unicode for everything else. --- Lib/ldap/compat.py | 9 +++++- Modules/LDAPObject.c | 18 +++++------- Modules/berval.c | 22 ++++++++++++++ Modules/berval.h | 1 + Modules/ldapcontrol.c | 9 +++--- Modules/message.c | 1 + Tests/t_cext.py | 68 +++++++++++++++++++++---------------------- Tests/t_search.py | 18 ++++++------ 8 files changed, 87 insertions(+), 59 deletions(-) diff --git a/Lib/ldap/compat.py b/Lib/ldap/compat.py index 7fbd115..1ee0b86 100644 --- a/Lib/ldap/compat.py +++ b/Lib/ldap/compat.py @@ -4,7 +4,14 @@ import sys if sys.version_info[0] < 3: from UserDict import UserDict - from urllib import quote, unquote + from urllib import quote + from urllib import unquote as urllib_unquote + + def unquote(uri): + """Specialized unquote that uses UTF-8 for parsing.""" + uri = uri.encode('ascii') + unquoted = urllib_unquote(uri) + return unquoted.decode('utf-8') else: from collections import UserDict from urllib.parse import quote, unquote diff --git a/Modules/LDAPObject.c b/Modules/LDAPObject.c index c5b3267..72b56ea 100644 --- a/Modules/LDAPObject.c +++ b/Modules/LDAPObject.c @@ -105,7 +105,7 @@ Tuple_to_LDAPMod( PyObject* tup, int no_op ) { int op; char *type; - PyObject *list, *item, *bytes; + PyObject *list, *item; LDAPMod *lm = NULL; Py_ssize_t i, len, nstrs; @@ -139,7 +139,7 @@ Tuple_to_LDAPMod( PyObject* tup, int no_op ) if (list == Py_None) { /* None indicates a NULL mod_bvals */ - } else if (PyUnicode_Check(list)) { + } else if (PyBytes_Check(list)) { /* Single string is a singleton list */ lm->mod_bvalues = PyMem_NEW(struct berval *, 2); if (lm->mod_bvalues == NULL) @@ -147,10 +147,9 @@ Tuple_to_LDAPMod( PyObject* tup, int no_op ) lm->mod_bvalues[0] = PyMem_NEW(struct berval, 1); if (lm->mod_bvalues[0] == NULL) goto nomem; - bytes = PyUnicode_AsUTF8String(list); lm->mod_bvalues[1] = NULL; - lm->mod_bvalues[0]->bv_len = PyBytes_Size(bytes); - lm->mod_bvalues[0]->bv_val = PyBytes_AsString(bytes); + lm->mod_bvalues[0]->bv_len = PyBytes_Size(list); + lm->mod_bvalues[0]->bv_val = PyBytes_AsString(list); } else if (PySequence_Check(list)) { nstrs = PySequence_Length(list); lm->mod_bvalues = PyMem_NEW(struct berval *, nstrs + 1); @@ -164,15 +163,14 @@ Tuple_to_LDAPMod( PyObject* tup, int no_op ) item = PySequence_GetItem(list, i); if (item == NULL) goto error; - if (!PyUnicode_Check(item)) { + if (!PyBytes_Check(item)) { PyErr_SetObject( PyExc_TypeError, Py_BuildValue( "sO", "expected a string in the list", item)); Py_DECREF(item); goto error; } - bytes = PyUnicode_AsUTF8String(item); - lm->mod_bvalues[i]->bv_len = PyBytes_Size(bytes); - lm->mod_bvalues[i]->bv_val = PyBytes_AsString(bytes); + lm->mod_bvalues[i]->bv_len = PyBytes_Size(item); + lm->mod_bvalues[i]->bv_val = PyBytes_AsString(item); Py_DECREF(item); } if (nstrs == 0) @@ -1201,7 +1199,7 @@ l_ldap_whoami_s( LDAPObject* self, PyObject* args ) if ( ldaperror!=LDAP_SUCCESS ) return LDAPerror( self->ldap, "ldap_whoami_s" ); - result = LDAPberval_to_object(bvalue); + result = LDAPberval_to_unicode_object(bvalue); return result; } diff --git a/Modules/berval.c b/Modules/berval.c index f39b7fe..501b865 100644 --- a/Modules/berval.c +++ b/Modules/berval.c @@ -85,6 +85,28 @@ LDAPberval_to_object(const struct berval *bv) { PyObject *ret = NULL; + if (!bv) { + ret = Py_None; + Py_INCREF(ret); + } + else { + ret = PyBytes_FromStringAndSize(bv->bv_val, bv->bv_len); + } + + return ret; +} + +/* + * Same as LDAPberval_to_object, but returns a Unicode PyObject. + * Use when the value is known to be text (for instance a distinguishedName). + * + * Returns a new Python object on success, or NULL on failure. + */ +PyObject * +LDAPberval_to_unicode_object(const struct berval *bv) +{ + PyObject *ret = NULL; + if (!bv) { ret = Py_None; Py_INCREF(ret); diff --git a/Modules/berval.h b/Modules/berval.h index 0b32438..d0b7206 100644 --- a/Modules/berval.h +++ b/Modules/berval.h @@ -11,5 +11,6 @@ int LDAPberval_from_object(PyObject *obj, struct berval *bv); int LDAPberval_from_object_check(PyObject *obj); void LDAPberval_release(struct berval *bv); PyObject *LDAPberval_to_object(const struct berval *bv); +PyObject *LDAPberval_to_unicode_object(const struct berval *bv); #endif /* __h_berval_ */ diff --git a/Modules/ldapcontrol.c b/Modules/ldapcontrol.c index f663d6d..b8a8593 100644 --- a/Modules/ldapcontrol.c +++ b/Modules/ldapcontrol.c @@ -104,14 +104,13 @@ Tuple_to_LDAPControl( PyObject* tup ) berbytes.bv_len = 0; berbytes.bv_val = NULL; } - else if (PyUnicode_Check(bytes)) { - bytes_utf8 = PyUnicode_AsUTF8String(bytes); - berbytes.bv_len = PyBytes_Size(bytes_utf8); - berbytes.bv_val = PyBytes_AsString(bytes_utf8); + else if (PyBytes_Check(bytes)) { + berbytes.bv_len = PyBytes_Size(bytes); + berbytes.bv_val = PyBytes_AsString(bytes); } else { PyErr_SetObject(PyExc_TypeError, Py_BuildValue("sO", - "expected a string", bytes)); + "expected bytes", bytes)); LDAPControl_DEL(lc); return NULL; } diff --git a/Modules/message.c b/Modules/message.c index 37c1f1d..a159e10 100644 --- a/Modules/message.c +++ b/Modules/message.c @@ -192,6 +192,7 @@ LDAPmessage_to_python(LDAP *ld, LDAPMessage *m, int add_ctrls, int add_intermedi if (refs) { Py_ssize_t i; for (i=0; refs[i] != NULL; i++) { + /* A referal is a distinguishedName => unicode */ PyObject *refstr = PyUnicode_FromString(refs[i]); PyList_Append(reflist, refstr); Py_DECREF(refstr); diff --git a/Tests/t_cext.py b/Tests/t_cext.py index b63f89a..00bcaf5 100644 --- a/Tests/t_cext.py +++ b/Tests/t_cext.py @@ -191,8 +191,8 @@ class TestLdapCExtension(unittest.TestCase): self.assertEquals(len(pmsg[0]), 2) self.assertEquals(pmsg[0][0], self.base) self.assertEquals(pmsg[0][0], self.base) - self.assertTrue('dcObject' in pmsg[0][1]['objectClass']) - self.assertTrue('organization' in pmsg[0][1]['objectClass']) + self.assertTrue(b'dcObject' in pmsg[0][1]['objectClass']) + self.assertTrue(b'organization' in pmsg[0][1]['objectClass']) self.assertEquals(msgid, m) self.assertEquals(ctrls, []) @@ -233,9 +233,9 @@ class TestLdapCExtension(unittest.TestCase): l = self._init() m = l.add_ext("cn=Foo," + self.base, [ - ('objectClass','organizationalRole'), - ('cn', 'Foo'), - ('description', 'testing'), + ('objectClass', b'organizationalRole'), + ('cn', b'Foo'), + ('description', b'testing'), ]) result,pmsg,msgid,ctrls = l.result4(m, _ldap.MSG_ALL, self.timeout) @@ -254,10 +254,10 @@ class TestLdapCExtension(unittest.TestCase): self.assertEquals(msgid, m) self.assertEquals(ctrls, []) - self.assertEquals(pmsg[0], ('cn=Foo,'+self.base, - { 'objectClass': ['organizationalRole'], - 'cn': ['Foo'], - 'description': ['testing'] })) + self.assertEqual(pmsg[0], ('cn=Foo,'+self.base, + { 'objectClass': [b'organizationalRole'], + 'cn': [b'Foo'], + 'description': [b'testing'] })) def test_compare(self): l = self._init() @@ -265,10 +265,10 @@ class TestLdapCExtension(unittest.TestCase): # first, add an object with a field we can compare on dn = "cn=CompareTest," + self.base m = l.add_ext(dn, [ - ('objectClass','person'), - ('sn', 'CompareTest'), - ('cn', 'CompareTest'), - ('userPassword', 'the_password'), + ('objectClass', b'person'), + ('sn', b'CompareTest'), + ('cn', b'CompareTest'), + ('userPassword', b'the_password'), ]) result,pmsg,msgid,ctrls = l.result4(m, _ldap.MSG_ALL, self.timeout) self.assertEquals(result, _ldap.RES_ADD) @@ -320,8 +320,8 @@ class TestLdapCExtension(unittest.TestCase): # first, add an object we will delete dn = "cn=Deleteme,"+self.base m = l.add_ext(dn, [ - ('objectClass','organizationalRole'), - ('cn', 'Deleteme'), + ('objectClass', b'organizationalRole'), + ('cn', b'Deleteme'), ]) result,pmsg,msgid,ctrls = l.result4(m, _ldap.MSG_ALL, self.timeout) self.assertEquals(result, _ldap.RES_ADD) @@ -339,7 +339,7 @@ class TestLdapCExtension(unittest.TestCase): # try deleting an object that doesn't exist not_found = False m = l.modify_ext("cn=DoesNotExist,"+self.base, [ - (_ldap.MOD_ADD, 'description', ['blah']), + (_ldap.MOD_ADD, 'description', [b'blah']), ]) try: r = l.result4(m, _ldap.MSG_ALL, self.timeout) @@ -366,16 +366,16 @@ class TestLdapCExtension(unittest.TestCase): # first, add an object we will delete dn = "cn=AddToMe,"+self.base m = l.add_ext(dn, [ - ('objectClass','person'), - ('cn', 'AddToMe'), - ('sn', 'Modify'), - ('description', 'a description'), + ('objectClass', b'person'), + ('cn', b'AddToMe'), + ('sn', b'Modify'), + ('description', b'a description'), ]) result,pmsg,msgid,ctrls = l.result4(m, _ldap.MSG_ALL, self.timeout) self.assertEquals(result, _ldap.RES_ADD) m = l.modify_ext(dn, [ - (_ldap.MOD_ADD, 'description', ['b desc', 'c desc']), + (_ldap.MOD_ADD, 'description', [b'b desc', b'c desc']), ]) result,pmsg,msgid,ctrls = l.result4(m, _ldap.MSG_ALL, self.timeout) self.assertEquals(result, _ldap.RES_MODIFY) @@ -396,14 +396,14 @@ class TestLdapCExtension(unittest.TestCase): self.assertEquals(pmsg[0][0], dn) d = list(pmsg[0][1]['description']) d.sort() - self.assertEquals(d, ['a description', 'b desc', 'c desc']) + self.assertEqual(d, [b'a description', b'b desc', b'c desc']) def test_rename(self): l = self._init() dn = "cn=RenameMe,"+self.base m = l.add_ext(dn, [ - ('objectClass','organizationalRole'), - ('cn', 'RenameMe'), + ('objectClass', b'organizationalRole'), + ('cn', b'RenameMe'), ]) result,pmsg,msgid,ctrls = l.result4(m, _ldap.MSG_ALL, self.timeout) self.assertEquals(result, _ldap.RES_ADD) @@ -433,13 +433,13 @@ class TestLdapCExtension(unittest.TestCase): self.assertEquals(ctrls, []) self.assertEquals(len(pmsg), 1) self.assertEquals(pmsg[0][0], dn2) - self.assertEquals(pmsg[0][1]['cn'], ['IAmRenamed']) + self.assertEqual(pmsg[0][1]['cn'], [b'IAmRenamed']) # create the container containerDn = "ou=RenameContainer,"+self.base m = l.add_ext(containerDn, [ - ('objectClass','organizationalUnit'), - ('ou', 'RenameContainer'), + ('objectClass', b'organizationalUnit'), + ('ou', b'RenameContainer'), ]) result,pmsg,msgid,ctrls = l.result4(m, _ldap.MSG_ALL, self.timeout) self.assertEquals(result, _ldap.RES_ADD) @@ -449,8 +449,8 @@ class TestLdapCExtension(unittest.TestCase): # the renamed object appears to be deleted, not moved.) # see http://www.openldap.org/its/index.cgi/Software%20Bugs?id=5408 m = l.add_ext("cn=Bogus," + containerDn, [ - ('objectClass','organizationalRole'), - ('cn', 'Bogus'), + ('objectClass', b'organizationalRole'), + ('cn', b'Bogus'), ]) result,pmsg,msgid,ctrls = l.result4(m, _ldap.MSG_ALL, self.timeout) self.assertEquals(result, _ldap.RES_ADD) @@ -485,7 +485,7 @@ class TestLdapCExtension(unittest.TestCase): self.assertEquals(ctrls, []) self.assertEquals(len(pmsg), 1) self.assertEquals(pmsg[0][0], dn3) - self.assertEquals(pmsg[0][1]['cn'], ['IAmRenamedAgain']) + self.assertEqual(pmsg[0][1]['cn'], [b'IAmRenamedAgain']) def test_whoami(self): @@ -517,10 +517,10 @@ class TestLdapCExtension(unittest.TestCase): # first, create a user to change password on dn = "cn=PasswordTest," + self.base m = l.add_ext(dn, [ - ('objectClass','person'), - ('sn', 'PasswordTest'), - ('cn', 'PasswordTest'), - ('userPassword', 'initial'), + ('objectClass', b'person'), + ('sn', b'PasswordTest'), + ('cn', b'PasswordTest'), + ('userPassword', b'initial'), ]) result,pmsg,msgid,ctrls = l.result4(m, _ldap.MSG_ALL, self.timeout) self.assertEquals(result, _ldap.RES_ADD) diff --git a/Tests/t_search.py b/Tests/t_search.py index 8b602aa..55da21f 100644 --- a/Tests/t_search.py +++ b/Tests/t_search.py @@ -56,13 +56,13 @@ class TestSearch(unittest.TestCase): result.sort() self.assertEquals(result, [('cn=Foo1,'+base, - {'cn': ['Foo1'], 'objectClass': ['organizationalRole']}), + {'cn': [b'Foo1'], 'objectClass': [b'organizationalRole']}), ('cn=Foo2,'+base, - {'cn': ['Foo2'], 'objectClass': ['organizationalRole']}), + {'cn': [b'Foo2'], 'objectClass': [b'organizationalRole']}), ('cn=Foo3,'+base, - {'cn': ['Foo3'], 'objectClass': ['organizationalRole']}), + {'cn': [b'Foo3'], 'objectClass': [b'organizationalRole']}), ('cn=Foo4,ou=Container,'+base, - {'cn': ['Foo4'], 'objectClass': ['organizationalRole']}), + {'cn': [b'Foo4'], 'objectClass': [b'organizationalRole']}), ] ) @@ -74,11 +74,11 @@ class TestSearch(unittest.TestCase): result.sort() self.assertEquals(result, [('cn=Foo1,'+base, - {'cn': ['Foo1'], 'objectClass': ['organizationalRole']}), + {'cn': [b'Foo1'], 'objectClass': [b'organizationalRole']}), ('cn=Foo2,'+base, - {'cn': ['Foo2'], 'objectClass': ['organizationalRole']}), + {'cn': [b'Foo2'], 'objectClass': [b'organizationalRole']}), ('cn=Foo3,'+base, - {'cn': ['Foo3'], 'objectClass': ['organizationalRole']}), + {'cn': [b'Foo3'], 'objectClass': [b'organizationalRole']}), ] ) @@ -88,8 +88,8 @@ class TestSearch(unittest.TestCase): result = l.search_s(base, ldap.SCOPE_SUBTREE, '(cn=Foo4)', ['cn']) result.sort() - self.assertEquals(result, - [('cn=Foo4,ou=Container,'+base, {'cn': ['Foo4']})] + self.assertEqual(result, + [('cn=Foo4,ou=Container,'+base, {'cn': [b'Foo4']})] )