Attachment 14768 Details for Bug 13616 – follow patch (master version)

[patch] follow patch (master version)

master-et-es-13616-followup-patch (text/plain), 13.37 KB, created by Noel Power on 2019-01-14 12:15:55 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Noel Power

Created: 2019-01-14 12:15:55 UTC

Size: 13.37 KB

patch

obsolete

>From 5232979de8394944dccae848207aae65c1a65ec1 Mon Sep 17 00:00:00 2001
>From: Noel Power <noel.power@suse.com>
>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 <noel.power@suse.com>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>---
> 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 <noel.power@suse.com>
>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 <noel.power@suse.com>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>---
> 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 <noel.power@suse.com>
>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 <noel.power@suse.com>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>---
> 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 <noel.power@suse.com>
>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 <noel.power@suse.com>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>---
> 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 <noel.power@suse.com>
>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 <noel.power@suse.com>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>---
> 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 <noel.power@suse.com>
>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 <noel.power@suse.com>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>---
> 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 <noel.power@suse.com>
>Date: Mon, 12 Nov 2018 17:56:46 +0000
>Subject: [PATCH 7/8] selftest: Enable ldb.python for PY3
>
>Signed-off-by: Noel Power <noel.power@suse.com>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>---
> 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 <noel.power@suse.com>
>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 <noel.power@suse.com>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>
>Autobuild-User(master): Douglas Bagnall <dbagnall@samba.org>
>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
>

Actions: View

Attachments on bug 13616: 14493 | 14499 | 14500 | 14501 | 14565 | 14650 | 14651 | 14652 | 14653 | 14768 | 14769 | 14786 | 14906