From 5232979de8394944dccae848207aae65c1a65ec1 Mon Sep 17 00:00:00 2001 From: Noel Power Date: Fri, 9 Nov 2018 16:47:00 +0000 Subject: [PATCH 1/8] python: Fix memory leak with ParseTuple (using 'es' format) Signed-off-by: Noel Power Reviewed-by: Douglas Bagnall --- python/pyglue.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/python/pyglue.c b/python/pyglue.c index 22ac53f3c66..e4f961fdaf7 100644 --- a/python/pyglue.c +++ b/python/pyglue.c @@ -300,28 +300,36 @@ static PyObject *py_strcasecmp_m(PyObject *self, PyObject *args) { const char *s1 = NULL; const char *s2 = NULL; - + long cmp_result = 0; if (!PyArg_ParseTuple(args, "eses", "utf8", &s1, "utf8", &s2)) { return NULL; } - return PyInt_FromLong(strcasecmp_m(s1, s2)); + cmp_result = strcasecmp_m(s1, s2); + PyMem_Free(discard_const_p(char, s1)); + PyMem_Free(discard_const_p(char, s2)); + return PyInt_FromLong(cmp_result); } static PyObject *py_strstr_m(PyObject *self, PyObject *args) { const char *s1 = NULL; const char *s2 = NULL; - char *ret = NULL; - + char *strstr_ret = NULL; + PyObject *result = NULL; if (!PyArg_ParseTuple(args, "eses", "utf8", &s1, "utf8", &s2)) return NULL; - ret = strstr_m(s1, s2); - if (!ret) { + strstr_ret = strstr_m(s1, s2); + if (!strstr_ret) { + PyMem_Free(discard_const_p(char, s1)); + PyMem_Free(discard_const_p(char, s2)); Py_RETURN_NONE; } - return PyUnicode_FromString(ret); + result = PyUnicode_FromString(strstr_ret); + PyMem_Free(discard_const_p(char, s1)); + PyMem_Free(discard_const_p(char, s2)); + return result; } static PyMethodDef py_misc_methods[] = { -- 2.16.4 From 253af8b85450c2830a442084e98734ca338c1b2f Mon Sep 17 00:00:00 2001 From: Noel Power Date: Tue, 11 Dec 2018 15:18:10 +0000 Subject: [PATCH 2/8] python: Add new compat PYARG_STR_UNI format In python2 PYARG_STR_UNI evaluates to et which allows str type (e.g bytes) pass through unencoded and accepts unicode objects encoded as utf8 In python3 PYARG_STR_UNI evaluates to es which allows str type encoded as named/specified encoding Signed-off-by: Noel Power Reviewed-by: Douglas Bagnall --- python/py3compat.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/python/py3compat.h b/python/py3compat.h index 5fa57f323d5..89b7552c791 100644 --- a/python/py3compat.h +++ b/python/py3compat.h @@ -118,6 +118,7 @@ /* PyArg_ParseTuple/Py_BuildValue argument */ #define PYARG_BYTES_LEN "y#" +#define PYARG_STR_UNI "es" #else @@ -179,6 +180,15 @@ /* PyArg_ParseTuple/Py_BuildValue argument */ #define PYARG_BYTES_LEN "s#" +/* + * We want a format that will ensure unicode is encoded using the + * specified encoding 'utf8' (to obtain the char* array) + * In python3 we use "es" but in python2 the specifiying 'es' will + * result in the any incomming 'str' type being decoded first to ascii + * then encoded to the specified 'utf8' encoding. In order to avoid that + * we use format 'et' in python2 instead. + */ +#define PYARG_STR_UNI "et" /* Module init */ -- 2.16.4 From efc4570d6ff1f1b0f12869d65ccf4aed92773ee2 Mon Sep 17 00:00:00 2001 From: Noel Power Date: Tue, 11 Dec 2018 15:32:11 +0000 Subject: [PATCH 3/8] auth/credentials: use 'et' as format for ParseTuple with python2 Signed-off-by: Noel Power Reviewed-by: Douglas Bagnall --- auth/credentials/pycredentials.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auth/credentials/pycredentials.c b/auth/credentials/pycredentials.c index fa8ac2bcb9f..7f9bc38af8e 100644 --- a/auth/credentials/pycredentials.c +++ b/auth/credentials/pycredentials.c @@ -175,7 +175,7 @@ static PyObject *py_creds_set_password(PyObject *self, PyObject *args) enum credentials_obtained obt = CRED_SPECIFIED; int _obt = obt; PyObject *result = NULL; - if (!PyArg_ParseTuple(args, "es|i", "utf8", &newval, &_obt)) { + if (!PyArg_ParseTuple(args, PYARG_STR_UNI"|i", "utf8", &newval, &_obt)) { return NULL; } obt = _obt; -- 2.16.4 From a799377029077a1749cb9cddbb81962c8479b3df Mon Sep 17 00:00:00 2001 From: Noel Power Date: Tue, 11 Dec 2018 15:58:07 +0000 Subject: [PATCH 4/8] python: use 'et' as format for ParseTuple with python2 Signed-off-by: Noel Power Reviewed-by: Douglas Bagnall --- python/pyglue.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/python/pyglue.c b/python/pyglue.c index e4f961fdaf7..70e211606ff 100644 --- a/python/pyglue.c +++ b/python/pyglue.c @@ -301,7 +301,9 @@ static PyObject *py_strcasecmp_m(PyObject *self, PyObject *args) const char *s1 = NULL; const char *s2 = NULL; long cmp_result = 0; - if (!PyArg_ParseTuple(args, "eses", "utf8", &s1, "utf8", &s2)) { + if (!PyArg_ParseTuple(args, PYARG_STR_UNI + PYARG_STR_UNI, + "utf8", &s1, "utf8", &s2)) { return NULL; } @@ -317,7 +319,9 @@ static PyObject *py_strstr_m(PyObject *self, PyObject *args) const char *s2 = NULL; char *strstr_ret = NULL; PyObject *result = NULL; - if (!PyArg_ParseTuple(args, "eses", "utf8", &s1, "utf8", &s2)) + if (!PyArg_ParseTuple(args, PYARG_STR_UNI + PYARG_STR_UNI, + "utf8", &s1, "utf8", &s2)) return NULL; strstr_ret = strstr_m(s1, s2); -- 2.16.4 From b6c8ef5fb70c65c04c8269ff95e661e219968767 Mon Sep 17 00:00:00 2001 From: Noel Power Date: Tue, 11 Dec 2018 15:58:44 +0000 Subject: [PATCH 5/8] s4/libnet: use 'et' as format for ParseTuple with python2 Signed-off-by: Noel Power Reviewed-by: Douglas Bagnall --- source4/libnet/py_net.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source4/libnet/py_net.c b/source4/libnet/py_net.c index 65060d53091..b50f7abfb96 100644 --- a/source4/libnet/py_net.c +++ b/source4/libnet/py_net.c @@ -161,7 +161,8 @@ static PyObject *py_net_change_password(py_net_Object *self, PyObject *args, PyO const char *newpass = NULL; const char *oldpass = NULL; ZERO_STRUCT(r); - if (!PyArg_ParseTupleAndKeywords(args, kwargs, "es|esss:change_password", + if (!PyArg_ParseTupleAndKeywords(args, kwargs, PYARG_STR_UNI + "|"PYARG_STR_UNI"ss:change_password", discard_const_p(char *, kwnames), "utf8", &newpass, -- 2.16.4 From f8758b3b1f98476469501dd45a7c898950294e05 Mon Sep 17 00:00:00 2001 From: Noel Power Date: Mon, 12 Nov 2018 17:42:51 +0000 Subject: [PATCH 6/8] lib/ldb/tests/python: Add test to pass utf8 encoded bytes to ldb.Dn This test should demonstrate an error with the 'es' format in python where a 'str' byte-string is passed (containing utf8 encoded bytes) with some characters that cannot be decoded as ascii. The same code if run in python3 should generate an error (needs string not bytes) Also Add knownfail for ldb.Dn passed utf8 encoded byte string Signed-off-by: Noel Power Reviewed-by: Douglas Bagnall --- lib/ldb/tests/python/api.py | 15 +++++++++++++++ selftest/knownfail | 3 +++ 2 files changed, 18 insertions(+) diff --git a/lib/ldb/tests/python/api.py b/lib/ldb/tests/python/api.py index 7b6418154e6..1d9f33f8f73 100755 --- a/lib/ldb/tests/python/api.py +++ b/lib/ldb/tests/python/api.py @@ -142,6 +142,21 @@ class SimpleLdb(LdbBaseTest): l = ldb.Ldb(self.url(), flags=self.flags()) dn = ldb.Dn(l, (b'a=' + b'\xc4\x85\xc4\x87\xc4\x99\xc5\x82\xc5\x84\xc3\xb3\xc5\x9b\xc5\xba\xc5\xbc').decode('utf8')) + def test_utf8_encoded_ldb_Dn(self): + l = ldb.Ldb(self.url(), flags=self.flags()) + dn_encoded_utf8 = b'a=' + b'\xc4\x85\xc4\x87\xc4\x99\xc5\x82\xc5\x84\xc3\xb3\xc5\x9b\xc5\xba\xc5\xbc' + try: + dn = ldb.Dn(l, dn_encoded_utf8) + except UnicodeDecodeError as e: + raise + except TypeError as te: + if PY3: + p3errors = ["argument 2 must be str, not bytes", + "Can't convert 'bytes' object to str implicitly"] + self.assertIn(str(te), p3errors) + else: + raise + def test_search_attrs(self): l = ldb.Ldb(self.url(), flags=self.flags()) self.assertEqual(len(l.search(ldb.Dn(l, ""), ldb.SCOPE_SUBTREE, "(dc=*)", ["dc"])), 0) diff --git a/selftest/knownfail b/selftest/knownfail index abbbd889c71..254b61edf5e 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -363,3 +363,6 @@ ^samba.tests.ntlmdisabled.python\(ktest\).python2.ntlmdisabled.NtlmDisabledTests.test_samr_change_password\(ktest\) ^samba.tests.ntlmdisabled.python\(ad_dc_no_ntlm\).python3.ntlmdisabled.NtlmDisabledTests.test_ntlm_connection\(ad_dc_no_ntlm\) ^samba.tests.ntlmdisabled.python\(ad_dc_no_ntlm\).python2.ntlmdisabled.NtlmDisabledTests.test_ntlm_connection\(ad_dc_no_ntlm\) +# Ldb test api.SimpleLdb.test_utf8_ldb_Dn is expected to fail +^ldb.python.api.SimpleLdb.test_utf8_encoded_ldb_Dn(none) + -- 2.16.4 From 45f5337f9f02faa9bc8f5501601df399c229732e Mon Sep 17 00:00:00 2001 From: Noel Power Date: Mon, 12 Nov 2018 17:56:46 +0000 Subject: [PATCH 7/8] selftest: Enable ldb.python for PY3 Signed-off-by: Noel Power Reviewed-by: Douglas Bagnall --- selftest/tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selftest/tests.py b/selftest/tests.py index 9143a17ef33..89e5ff43507 100644 --- a/selftest/tests.py +++ b/selftest/tests.py @@ -59,7 +59,7 @@ else: planpythontestsuite("none", "subunit.tests.test_suite") planpythontestsuite("none", "samba.tests.blackbox.ndrdump", py3_compatible=True) planpythontestsuite("none", "samba.tests.blackbox.check_output", py3_compatible=True) -planpythontestsuite("none", "api", name="ldb.python", extra_path=['lib/ldb/tests/python']) +planpythontestsuite("none", "api", name="ldb.python", extra_path=['lib/ldb/tests/python'], py3_compatible=True) planpythontestsuite("none", "samba.tests.credentials", py3_compatible=True) planpythontestsuite("none", "samba.tests.registry", py3_compatible=True) planpythontestsuite("ad_dc_ntvfs:local", "samba.tests.auth", py3_compatible=True) -- 2.16.4 From 8900e0b4cb05613df9cbeeb8b8253273b06b3c17 Mon Sep 17 00:00:00 2001 From: Noel Power Date: Mon, 12 Nov 2018 16:06:10 +0000 Subject: [PATCH 8/8] lib/ldb: Use new PYARG_ES format for parseTuple While 'es' format works great for unicode (in python2) and str (in python3) The behaviour with str (in python2) is unexpected. In python2 the str type is (re-encoded) with the specified encoding. In python2 the 'et' type would be a better match, that ensures 'str' type is treated like it was with 's' (no reencoding) and unicode is encoded with the specified encoding. However in python3 'et' allows byte (or bytearray) params to be accepted (with no reencoding), we don't want this. This patch adds a new PYARG_STR_UNI format code which is a hybrid, in python2 it evaluates to 'et' and in python3 'es' and so gives the desired behaviour for each python version. Additionally remove the associated known fail. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13616 Signed-off-by: Noel Power Reviewed-by: Douglas Bagnall Autobuild-User(master): Douglas Bagnall Autobuild-Date(master): Sun Jan 13 03:53:00 CET 2019 on sn-devel-144 --- lib/ldb/pyldb.c | 6 +++++- selftest/knownfail | 3 --- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/ldb/pyldb.c b/lib/ldb/pyldb.c index b417d4cf754..c98ce5d1b2b 100644 --- a/lib/ldb/pyldb.c +++ b/lib/ldb/pyldb.c @@ -91,6 +91,8 @@ static PyTypeObject PyLdbBytesType; #define PyStr_AsUTF8AndSize PyUnicode_AsUTF8AndSize #define PyInt_FromLong PyLong_FromLong +#define PYARG_STR_UNI "es" + static PyObject *PyLdbBytes_FromStringAndSize(const char *msg, int size) { PyObject* result = NULL; @@ -109,6 +111,8 @@ static PyObject *PyLdbBytes_FromStringAndSize(const char *msg, int size) #define PyStr_AsUTF8 PyString_AsString #define PyLdbBytes_FromStringAndSize PyString_FromStringAndSize +#define PYARG_STR_UNI "et" + const char *PyStr_AsUTF8AndSize(PyObject *pystr, Py_ssize_t *sizeptr); const char * PyStr_AsUTF8AndSize(PyObject *pystr, Py_ssize_t *sizeptr) @@ -901,7 +905,7 @@ static PyObject *py_ldb_dn_new(PyTypeObject *type, PyObject *args, PyObject *kwa PyLdbDnObject *py_ret = NULL; const char * const kwnames[] = { "ldb", "dn", NULL }; - if (!PyArg_ParseTupleAndKeywords(args, kwargs, "Oes", + if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O"PYARG_STR_UNI, discard_const_p(char *, kwnames), &py_ldb, "utf8", &str)) goto out; diff --git a/selftest/knownfail b/selftest/knownfail index 254b61edf5e..abbbd889c71 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -363,6 +363,3 @@ ^samba.tests.ntlmdisabled.python\(ktest\).python2.ntlmdisabled.NtlmDisabledTests.test_samr_change_password\(ktest\) ^samba.tests.ntlmdisabled.python\(ad_dc_no_ntlm\).python3.ntlmdisabled.NtlmDisabledTests.test_ntlm_connection\(ad_dc_no_ntlm\) ^samba.tests.ntlmdisabled.python\(ad_dc_no_ntlm\).python2.ntlmdisabled.NtlmDisabledTests.test_ntlm_connection\(ad_dc_no_ntlm\) -# Ldb test api.SimpleLdb.test_utf8_ldb_Dn is expected to fail -^ldb.python.api.SimpleLdb.test_utf8_encoded_ldb_Dn(none) - -- 2.16.4