[Py3] Always use *bytes* for values.

Keep unicode for everything else.

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

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_len = PyBytes_Size(list);
-        lm->mod_bvalues[0]->bv_val = PyBytes_AsString(bytes);
+        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(item);
-          lm->mod_bvalues[i]->bv_len = PyBytes_Size(bytes);
+          lm->mod_bvalues[i]->bv_val = PyBytes_AsString(item);
-          lm->mod_bvalues[i]->bv_val = PyBytes_AsString(bytes);
           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;
 }

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);

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_ */

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)) {
+    else if (PyBytes_Check(bytes)) {
-        bytes_utf8 = PyUnicode_AsUTF8String(bytes);
+        berbytes.bv_len = PyBytes_Size(bytes);
-        berbytes.bv_len = PyBytes_Size(bytes_utf8);
+        berbytes.bv_val = PyBytes_AsString(bytes);
-        berbytes.bv_val = PyBytes_AsString(bytes_utf8);
     }
     else {
 	PyErr_SetObject(PyExc_TypeError, Py_BuildValue("sO",
-            "expected a string", bytes));
+            "expected bytes", bytes));
         LDAPControl_DEL(lc);
         return NULL;
     }

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);

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(b'dcObject' in pmsg[0][1]['objectClass'])
-        self.assertTrue('organization' 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'), 
+               ('objectClass', b'organizationalRole'),
-               ('cn', 'Foo'),
+               ('cn', b'Foo'),
-               ('description', 'testing'),
+               ('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,
+        self.assertEqual(pmsg[0], ('cn=Foo,'+self.base,
-                { 'objectClass': ['organizationalRole'],
+                { 'objectClass': [b'organizationalRole'],
-                  'cn': ['Foo'],
+                  'cn': [b'Foo'],
-                  'description': ['testing'] }))
+                  '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'), 
+               ('objectClass', b'person'),
-               ('sn', 'CompareTest'),
+               ('sn', b'CompareTest'),
-               ('cn', 'CompareTest'),
+               ('cn', b'CompareTest'),
-               ('userPassword', 'the_password'),
+               ('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'), 
+               ('objectClass', b'organizationalRole'),
-               ('cn', 'Deleteme'),
+               ('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'), 
+               ('objectClass', b'person'),
-               ('cn', 'AddToMe'),
+               ('cn', b'AddToMe'),
-               ('sn', 'Modify'),
+               ('sn', b'Modify'),
-               ('description', 'a description'),
+               ('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'), 
+               ('objectClass', b'organizationalRole'),
-               ('cn', 'RenameMe'),
+               ('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'), 
+               ('objectClass', b'organizationalUnit'),
-               ('ou', 'RenameContainer'),
+               ('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'), 
+               ('objectClass', b'organizationalRole'),
-               ('cn', 'Bogus'),
+               ('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'), 
+               ('objectClass', b'person'),
-               ('sn', 'PasswordTest'),
+               ('sn', b'PasswordTest'),
-               ('cn', 'PasswordTest'),
+               ('cn', b'PasswordTest'),
-               ('userPassword', 'initial'),
+               ('userPassword', b'initial'),
             ])
         result,pmsg,msgid,ctrls = l.result4(m, _ldap.MSG_ALL, self.timeout)
         self.assertEquals(result, _ldap.RES_ADD)

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,
+        self.assertEqual(result,
-            [('cn=Foo4,ou=Container,'+base, {'cn': ['Foo4']})]
+            [('cn=Foo4,ou=Container,'+base, {'cn': [b'Foo4']})]
         )