[PATCH] Make s3 SMB Python bindings equivalent to s4

Tim Beale timbeale at catalyst.net.nz
Fri Jan 4 01:40:15 UTC 2019


Attached is an updated patch. Some small changes were made after some
feedback from Andrew on the gitlab merge request:
https://gitlab.com/samba-team/samba/merge_requests/155

I'll push these changes next week unless anyone else has feedback on the
patches?

Thanks,
Tim

On 12/12/18 5:17 PM, Tim Beale via samba-technical wrote:
> The attached patch* adds additional APIs to the s3 SMB Python bindings
> to make them equivalent to the current s4 bindings. The tests for the s4
> bindings have been converted over to use the s3 bindings instead. This
> means the tests can now run against a DC with SMBv1 disabled (and
> tests.py has been updated to do just that).
>
> If you haven't been following the various threads, the reason I'm doing
> all this is because the s4 bindings don't support SMBv2. The s3 SMB
> library does, but the s3 python bindings don't support all the s4 APIs
> that things like samba-tool currently use.
>
> The next steps are:
> - adding one more API (.get_acl()) that the NTACL backup code uses, then
> convert that code to use the s3 bindings.
> - replace the SMB connections used by samba-tool with the s3 bindings.
> - rename 'libsmb_samba_internal' to something more appropriate ('libsmb'?).
> - remove the s4 bindings (i.e. pysmb.c), as everything should have been
> updated to use the s3 bindings.
>
> CI link: https://gitlab.com/catalyst-samba/samba/pipelines/39807293
> Gitlab branch:
> https://gitlab.com/catalyst-samba/samba/commits/timb-pysmb-latest
>
> Review/feedback appreciated. Thanks.
>
> *Note the attached patch applies on top of other changes currently
> pending delivery, i.e.
> https://lists.samba.org/archive/samba-technical/2018-December/131513.html
>
-------------- next part --------------
From 6d154ba450374e25fb0b542194fee155a71e838c Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 12 Dec 2018 10:25:35 +1300
Subject: [PATCH 01/19] s3:pylibsmb: Make lp a mandatory param for the SMB
 connection

Currently establishing the SMB connection relies on having initialized
the global source3 loadparm.

This patch makes the lp param mandatory, so that you always have to pass
the parameter in when establishing the SMB connection.

It also makes the source3 API more consistent with the current source4
API, which will make it easier to replace the source4 version later.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/libsmb_samba_internal.py | 2 +-
 source3/libsmb/pylibsmb.c                   | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/python/samba/tests/libsmb_samba_internal.py b/python/samba/tests/libsmb_samba_internal.py
index 8918d84..ebfed94 100644
--- a/python/samba/tests/libsmb_samba_internal.py
+++ b/python/samba/tests/libsmb_samba_internal.py
@@ -60,7 +60,7 @@ class LibsmbTestCase(samba.tests.TestCase):
         creds.set_password(os.getenv("PASSWORD"))
 
         c = libsmb_samba_internal.Conn(os.getenv("SERVER_IP"), "tmp",
-                                       creds, multi_threaded=True,
+                                       lp, creds, multi_threaded=True,
                                        force_smb1=True)
 
         mythreads = []
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 041d408..c1be26b 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -423,6 +423,7 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 	char *host, *share;
 	PyObject *creds = NULL;
 	struct cli_credentials *cli_creds;
+	PyObject *py_lp = Py_None;
 	PyObject *py_multi_threaded = Py_False;
 	bool multi_threaded = false;
 	PyObject *py_sign = Py_False;
@@ -435,7 +436,7 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 	int flags = 0;
 
 	static const char *kwlist[] = {
-		"host", "share", "credentials",
+		"host", "share", "lp", "credentials",
 		"multi_threaded", "sign", "force_smb1",
 		NULL
 	};
@@ -447,8 +448,8 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 	}
 
 	ret = ParseTupleAndKeywords(
-		args, kwds, "ss|O!OOO", kwlist,
-		&host, &share,
+		args, kwds, "ssO|O!OOO", kwlist,
+		&host, &share, &py_lp,
 		py_type_Credentials, &creds,
 		&py_multi_threaded,
 		&py_sign,
-- 
2.7.4


From 9263f60fdd01235d6f62772fe611844eea48f620 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 3 Dec 2018 10:50:19 +1300
Subject: [PATCH 02/19] s3:pylibsmb: Add .unlink() API to SMB Py bindings

Add a basic .unlink() API to the source3 bindings. This is based on the
source4 python bindings, but uses the source3 client library APIs.
(We use a helper function to do most of the work, because we will need
to reuse it in order to support the deltree API).

Update the source4 test to use the source3 API. We will gradually
convert it over, and then delete the source4 python bindings.

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/smb.py | 11 ++++++++++-
 source3/libsmb/pylibsmb.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index 6893647..a74db3b 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -23,6 +23,8 @@ from samba import smb
 from samba import NTSTATUSError
 from samba.ntstatus import (NT_STATUS_OBJECT_NAME_NOT_FOUND,
                             NT_STATUS_OBJECT_PATH_NOT_FOUND)
+from samba.samba3 import libsmb_samba_internal
+from samba.samba3 import param as s3param
 
 PY3 = sys.version_info[0] == 3
 addom = 'addom.samba.example.com/'
@@ -44,6 +46,13 @@ class SMBTests(samba.tests.TestCase):
                             "sysvol",
                             lp=self.get_loadparm(),
                             creds=creds)
+
+        # temporarily create a 2nd SMB connection for migrating the py-bindings
+        lp = s3param.get_context()
+        lp.load(os.getenv("SMB_CONF_PATH"))
+        self.smb_conn = libsmb_samba_internal.Conn(self.server, "sysvol",
+                                                   lp, creds)
+
         self.conn.mkdir(test_dir)
 
     def tearDown(self):
@@ -161,7 +170,7 @@ class SMBTests(samba.tests.TestCase):
         self.assertTrue(self.file_exists(test_file))
 
         # delete it and check that it's gone
-        self.conn.unlink(test_file)
+        self.smb_conn.unlink(test_file)
         self.assertFalse(self.file_exists(test_file))
 
     def test_chkpath(self):
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index c1be26b..ab7400a 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -965,6 +965,46 @@ static PyObject *py_cli_list(struct py_cli_state *self,
 	return result;
 }
 
+/*
+ * Deletes a file
+ */
+static NTSTATUS unlink_file(struct py_cli_state *self, const char *filename)
+{
+	NTSTATUS status;
+	uint16_t attrs = (FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN);
+
+	if (self->is_smb1) {
+		struct tevent_req *req = NULL;
+
+		req = cli_unlink_send(NULL, self->ev, self->cli, filename,
+				      attrs);
+		if (!py_tevent_req_wait_exc(self, req)) {
+			return NT_STATUS_INTERNAL_ERROR;
+		}
+		status = cli_unlink_recv(req);
+		TALLOC_FREE(req);
+	} else {
+		status = cli_unlink(self->cli, filename, attrs);
+	}
+
+	return status;
+}
+
+static PyObject *py_smb_unlink(struct py_cli_state *self, PyObject *args)
+{
+	NTSTATUS status;
+	const char *filename = NULL;
+
+	if (!PyArg_ParseTuple(args, "s:unlink", &filename)) {
+		return NULL;
+	}
+
+	status = unlink_file(self, filename);
+	PyErr_NTSTATUS_NOT_OK_RAISE(status);
+
+	Py_RETURN_NONE;
+}
+
 static PyMethodDef py_cli_state_methods[] = {
 	{ "settimeout", (PyCFunction)py_cli_settimeout, METH_VARARGS,
 	  "settimeout(new_timeout_msecs) => return old_timeout_msecs" },
@@ -987,6 +1027,9 @@ static PyMethodDef py_cli_state_methods[] = {
 	  "List a directory" },
 	{ "get_oplock_break", (PyCFunction)py_cli_get_oplock_break,
 	  METH_VARARGS, "Wait for an oplock break" },
+	{ "unlink", (PyCFunction)py_smb_unlink,
+	  METH_VARARGS,
+	  "unlink(path) -> None\n\n \t\tDelete a file." },
 	{ NULL, NULL, 0, NULL }
 };
 
-- 
2.7.4


From b95197e37b6eea7b697dda76487f48d72def0e74 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 3 Dec 2018 16:27:07 +1300
Subject: [PATCH 03/19] s3:pylibsmb: Add .mkdir(), .rmdir() APIS to SMB py
 bindings

I kept these separate from the chkpath API because it's a nice way to
use the old s4 API in the tests to verify the new s3 API works
correctly.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/smb.py | 10 +++----
 source3/libsmb/pylibsmb.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index a74db3b..25cca33 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -53,7 +53,7 @@ class SMBTests(samba.tests.TestCase):
         self.smb_conn = libsmb_samba_internal.Conn(self.server, "sysvol",
                                                    lp, creds)
 
-        self.conn.mkdir(test_dir)
+        self.smb_conn.mkdir(test_dir)
 
     def tearDown(self):
         super(SMBTests, self).tearDown()
@@ -98,13 +98,13 @@ class SMBTests(samba.tests.TestCase):
         cur_dir = test_dir
         for subdir in ["subdir-X", "subdir-Y", "subdir-Z"]:
             path = self.make_sysvol_path(cur_dir, subdir)
-            self.conn.mkdir(path)
+            self.smb_conn.mkdir(path)
             dirpaths.append(path)
             cur_dir = path
 
             # create another empty dir just for kicks
             path = self.make_sysvol_path(cur_dir, "another")
-            self.conn.mkdir(path)
+            self.smb_conn.mkdir(path)
             empty_dirs.append(path)
 
         # create some files in these directories
@@ -188,9 +188,9 @@ class SMBTests(samba.tests.TestCase):
 
         # check correct result after creating and then deleting a new dir
         new_dir = self.make_sysvol_path(test_dir, 'test-new')
-        self.conn.mkdir(new_dir)
+        self.smb_conn.mkdir(new_dir)
         self.assertTrue(self.conn.chkpath(new_dir))
-        self.conn.rmdir(new_dir)
+        self.smb_conn.rmdir(new_dir)
         self.assertFalse(self.conn.chkpath(new_dir))
 
     def test_save_load_text(self):
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index ab7400a..ecd2acf 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -1005,6 +1005,72 @@ static PyObject *py_smb_unlink(struct py_cli_state *self, PyObject *args)
 	Py_RETURN_NONE;
 }
 
+/*
+ * Delete an empty directory
+ */
+static NTSTATUS remove_dir(struct py_cli_state *self, const char *dirname)
+{
+	NTSTATUS status;
+
+	if (self->is_smb1) {
+		struct tevent_req *req = NULL;
+
+		req = cli_rmdir_send(NULL, self->ev, self->cli, dirname);
+		if (!py_tevent_req_wait_exc(self, req)) {
+			return NT_STATUS_INTERNAL_ERROR;
+		}
+		status = cli_rmdir_recv(req);
+		TALLOC_FREE(req);
+	} else {
+		status = cli_rmdir(self->cli, dirname);
+	}
+	return status;
+}
+
+static PyObject *py_smb_rmdir(struct py_cli_state *self, PyObject *args)
+{
+	NTSTATUS status;
+	const char *dirname;
+
+	if (!PyArg_ParseTuple(args, "s:rmdir", &dirname)) {
+		return NULL;
+	}
+
+	status = remove_dir(self, dirname);
+	PyErr_NTSTATUS_IS_ERR_RAISE(status);
+
+	Py_RETURN_NONE;
+}
+
+/*
+ * Create a directory
+ */
+static PyObject *py_smb_mkdir(struct py_cli_state *self, PyObject *args)
+{
+	NTSTATUS status;
+	const char *dirname;
+
+	if (!PyArg_ParseTuple(args, "s:mkdir", &dirname)) {
+		return NULL;
+	}
+
+	if (self->is_smb1) {
+		struct tevent_req *req = NULL;
+
+		req = cli_mkdir_send(NULL, self->ev, self->cli, dirname);
+		if (!py_tevent_req_wait_exc(self, req)) {
+			return NULL;
+		}
+		status = cli_mkdir_recv(req);
+		TALLOC_FREE(req);
+	} else {
+		status = cli_mkdir(self->cli, dirname);
+	}
+	PyErr_NTSTATUS_IS_ERR_RAISE(status);
+
+	Py_RETURN_NONE;
+}
+
 static PyMethodDef py_cli_state_methods[] = {
 	{ "settimeout", (PyCFunction)py_cli_settimeout, METH_VARARGS,
 	  "settimeout(new_timeout_msecs) => return old_timeout_msecs" },
@@ -1030,6 +1096,10 @@ static PyMethodDef py_cli_state_methods[] = {
 	{ "unlink", (PyCFunction)py_smb_unlink,
 	  METH_VARARGS,
 	  "unlink(path) -> None\n\n \t\tDelete a file." },
+	{ "mkdir", (PyCFunction)py_smb_mkdir, METH_VARARGS,
+	  "mkdir(path) -> None\n\n \t\tCreate a directory." },
+	{ "rmdir", (PyCFunction)py_smb_rmdir, METH_VARARGS,
+	  "rmdir(path) -> None\n\n \t\tDelete a directory." },
 	{ NULL, NULL, 0, NULL }
 };
 
-- 
2.7.4


From 9759ae4142ba345c46f019a47a30513fd89f6057 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 3 Dec 2018 16:33:19 +1300
Subject: [PATCH 04/19] s3:pylibsmb: Add .chkpath() API to SMB py bindings

Note that this is checking the existence of *directories*, not *files*.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/smb.py | 16 ++++++++--------
 source3/libsmb/pylibsmb.c | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index 25cca33..497f57d 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -118,7 +118,7 @@ class SMBTests(samba.tests.TestCase):
 
         # sanity-check these dirs/files exist
         for subdir in dirpaths + empty_dirs:
-            self.assertTrue(self.conn.chkpath(subdir),
+            self.assertTrue(self.smb_conn.chkpath(subdir),
                             "Failed to create {0}".format(subdir))
         for path in filepaths:
             self.assertTrue(self.file_exists(path),
@@ -127,7 +127,7 @@ class SMBTests(samba.tests.TestCase):
         # try using deltree to remove a single empty directory
         path = empty_dirs.pop(0)
         self.conn.deltree(path)
-        self.assertFalse(self.conn.chkpath(path),
+        self.assertFalse(self.smb_conn.chkpath(path),
                          "Failed to delete {0}".format(path))
 
         # try using deltree to remove a single file
@@ -141,7 +141,7 @@ class SMBTests(samba.tests.TestCase):
 
         # now check that all the dirs/files are no longer there
         for subdir in dirpaths + empty_dirs:
-            self.assertFalse(self.conn.chkpath(subdir),
+            self.assertFalse(self.smb_conn.chkpath(subdir),
                              "Failed to delete {0}".format(subdir))
         for path in filepaths:
             self.assertFalse(self.file_exists(path),
@@ -176,22 +176,22 @@ class SMBTests(samba.tests.TestCase):
     def test_chkpath(self):
         """Tests .chkpath determines whether or not a directory exists"""
 
-        self.assertTrue(self.conn.chkpath(test_dir))
+        self.assertTrue(self.smb_conn.chkpath(test_dir))
 
         # should return False for a non-existent directory
         bad_dir = self.make_sysvol_path(test_dir, 'dont_exist')
-        self.assertFalse(self.conn.chkpath(bad_dir))
+        self.assertFalse(self.smb_conn.chkpath(bad_dir))
 
         # should return False for files (because they're not directories)
         self.conn.savefile(test_file, binary_contents)
-        self.assertFalse(self.conn.chkpath(test_file))
+        self.assertFalse(self.smb_conn.chkpath(test_file))
 
         # check correct result after creating and then deleting a new dir
         new_dir = self.make_sysvol_path(test_dir, 'test-new')
         self.smb_conn.mkdir(new_dir)
-        self.assertTrue(self.conn.chkpath(new_dir))
+        self.assertTrue(self.smb_conn.chkpath(new_dir))
         self.smb_conn.rmdir(new_dir)
-        self.assertFalse(self.conn.chkpath(new_dir))
+        self.assertFalse(self.smb_conn.chkpath(new_dir))
 
     def test_save_load_text(self):
 
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index ecd2acf..61f256b 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -1071,6 +1071,42 @@ static PyObject *py_smb_mkdir(struct py_cli_state *self, PyObject *args)
 	Py_RETURN_NONE;
 }
 
+/*
+ * Checks existence of a directory
+ */
+static bool check_dir_path(struct py_cli_state *self, const char *path)
+{
+	NTSTATUS status;
+
+	if (self->is_smb1) {
+		struct tevent_req *req = NULL;
+
+		req = cli_chkpath_send(NULL, self->ev, self->cli, path);
+		if (!py_tevent_req_wait_exc(self, req)) {
+			return false;
+		}
+		status = cli_chkpath_recv(req);
+		TALLOC_FREE(req);
+	} else {
+		status = cli_chkpath(self->cli, path);
+	}
+
+	return NT_STATUS_IS_OK(status);
+}
+
+static PyObject *py_smb_chkpath(struct py_cli_state *self, PyObject *args)
+{
+	const char *path;
+	bool dir_exists;
+
+	if (!PyArg_ParseTuple(args, "s:chkpath", &path)) {
+		return NULL;
+	}
+
+	dir_exists = check_dir_path(self, path);
+	return PyBool_FromLong(dir_exists);
+}
+
 static PyMethodDef py_cli_state_methods[] = {
 	{ "settimeout", (PyCFunction)py_cli_settimeout, METH_VARARGS,
 	  "settimeout(new_timeout_msecs) => return old_timeout_msecs" },
@@ -1100,6 +1136,9 @@ static PyMethodDef py_cli_state_methods[] = {
 	  "mkdir(path) -> None\n\n \t\tCreate a directory." },
 	{ "rmdir", (PyCFunction)py_smb_rmdir, METH_VARARGS,
 	  "rmdir(path) -> None\n\n \t\tDelete a directory." },
+	{ "chkpath", (PyCFunction)py_smb_chkpath, METH_VARARGS,
+	  "chkpath(dir_path) -> True or False\n\n"
+	  "\t\tReturn true if directory exists, false otherwise." },
 	{ NULL, NULL, 0, NULL }
 };
 
-- 
2.7.4


From 28154fdb91373a1a49bb91503872fd106f5e60a9 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 4 Dec 2018 09:15:30 +1300
Subject: [PATCH 05/19] s3:pylibsmb: Consolidate .readdir()/.list() py bindings
 API

Rename the .readdir API to .list and make the parameters match up with
the s4 python bindings, i.e. 'mask' is optional and separate from the
directory parameter.

Note that nothing uses the .readdir source3 API currently, so it's safe
to rename.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 source3/libsmb/pylibsmb.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 61f256b..9423fd2 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -902,6 +902,8 @@ static PyObject *py_cli_list(struct py_cli_state *self,
 			     PyObject *kwds)
 {
 	char *mask;
+	char *base_dir;
+	char *user_mask = NULL;
 	unsigned attribute =
 		FILE_ATTRIBUTE_DIRECTORY |
 		FILE_ATTRIBUTE_SYSTEM |
@@ -913,16 +915,24 @@ static PyObject *py_cli_list(struct py_cli_state *self,
 	size_t i, num_finfos;
 	PyObject *result;
 
-	const char *kwlist[] = {
-		"mask", "attribute", "info_level", NULL
-	};
+	const char *kwlist[] = { "directory", "mask", "attribs", NULL };
 
-	if (!ParseTupleAndKeywords(
-		    args, kwds, "s|II", kwlist,
-		    &mask, &attribute, &info_level)) {
+	if (!ParseTupleAndKeywords(args, kwds, "z|sH:list", kwlist,
+				   &base_dir, &user_mask, &attribute)) {
 		return NULL;
 	}
 
+	if (user_mask == NULL) {
+		mask = talloc_asprintf(NULL, "%s\\*", base_dir);
+	} else {
+		mask = talloc_asprintf(NULL, "%s\\%s", base_dir, user_mask);
+	}
+	if (mask == NULL) {
+		PyErr_NoMemory();
+		return NULL;
+	}
+	dos_format(mask);
+
 	req = cli_list_send(NULL, self->ev, self->cli, mask, attribute,
 			    info_level);
 	if (!py_tevent_req_wait_exc(self, req)) {
@@ -930,6 +940,7 @@ static PyObject *py_cli_list(struct py_cli_state *self,
 	}
 	status = cli_list_recv(req, NULL, &finfos, &num_finfos);
 	TALLOC_FREE(req);
+	TALLOC_FREE(mask);
 
 	if (!NT_STATUS_IS_OK(status)) {
 		PyErr_SetNTSTATUS(status);
@@ -1124,9 +1135,12 @@ static PyMethodDef py_cli_state_methods[] = {
 	{ "delete_on_close", (PyCFunction)py_cli_delete_on_close,
 	  METH_VARARGS|METH_KEYWORDS,
 	  "Set/Reset the delete on close flag" },
-	{ "readdir", (PyCFunction)py_cli_list,
-	  METH_VARARGS|METH_KEYWORDS,
-	  "List a directory" },
+	{ "list", (PyCFunction)py_cli_list, METH_VARARGS|METH_KEYWORDS,
+	  "list(directory, mask='*', attribs=DEFAULT_ATTRS) -> "
+	  "directory contents as a dictionary\n"
+	  "\t\tDEFAULT_ATTRS: FILE_ATTRIBUTE_SYSTEM | "
+	  "FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_ARCHIVE\n\n"
+	  "\t\tList contents of a directory." },
 	{ "get_oplock_break", (PyCFunction)py_cli_get_oplock_break,
 	  METH_VARARGS, "Wait for an oplock break" },
 	{ "unlink", (PyCFunction)py_smb_unlink,
-- 
2.7.4


From f003a5d3b526769fb74e3ce6b5b0e42533fb388e Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 4 Dec 2018 15:43:53 +1300
Subject: [PATCH 06/19] s3:pylibsmb: Split out code into list_helper()

Refactor out the code so we can re-use it for the synchronous API as
well.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 source3/libsmb/pylibsmb.c | 42 +++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 9423fd2..b5cb573 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -897,6 +897,31 @@ static PyObject *py_cli_delete_on_close(struct py_cli_state *self,
 	Py_RETURN_NONE;
 }
 
+/*
+ * Helper to add directory listing entries to an overall Python list
+ */
+static NTSTATUS list_helper(const char *mntpoint, struct file_info *finfo,
+			    const char *mask, void *state)
+{
+	PyObject *result = (PyObject *)state;
+	PyObject *file = NULL;
+	int ret;
+
+	file = Py_BuildValue("{s:s,s:i}",
+			     "name", finfo->name,
+			     "mode", (int)finfo->mode);
+	if (file == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	ret = PyList_Append(result, file);
+	if (ret == -1) {
+		return NT_STATUS_INTERNAL_ERROR;
+	}
+
+	return NT_STATUS_OK;
+}
+
 static PyObject *py_cli_list(struct py_cli_state *self,
 			     PyObject *args,
 			     PyObject *kwds)
@@ -953,21 +978,8 @@ static PyObject *py_cli_list(struct py_cli_state *self,
 	}
 
 	for (i=0; i<num_finfos; i++) {
-		struct file_info *finfo = &finfos[i];
-		PyObject *file;
-		int ret;
-
-		file = Py_BuildValue(
-			"{s:s,s:i}",
-			"name", finfo->name,
-			"mode", (int)finfo->mode);
-		if (file == NULL) {
-			Py_XDECREF(result);
-			return NULL;
-		}
-
-		ret = PyList_Append(result, file);
-		if (ret == -1) {
+		status = list_helper(base_dir, &finfos[i], user_mask, result);
+		if (!NT_STATUS_IS_OK(status)) {
 			Py_XDECREF(result);
 			return NULL;
 		}
-- 
2.7.4


From 232d9e8e4ac6f9ad99824850c91d86edfe65864d Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 12 Dec 2018 11:28:22 +1300
Subject: [PATCH 07/19] s3:pylibsmb: Split code out into do_listing() helper

We'll want to reuse the directory listing function for the deltree API.
This patch splits the bulk of the work out into a do_listing() helper
function.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 source3/libsmb/pylibsmb.c | 81 +++++++++++++++++++++++++++++------------------
 1 file changed, 50 insertions(+), 31 deletions(-)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index b5cb573..7c06350 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -922,53 +922,71 @@ static NTSTATUS list_helper(const char *mntpoint, struct file_info *finfo,
 	return NT_STATUS_OK;
 }
 
-static PyObject *py_cli_list(struct py_cli_state *self,
-			     PyObject *args,
-			     PyObject *kwds)
+static NTSTATUS do_listing(struct py_cli_state *self,
+			   const char *base_dir, const char *user_mask,
+			   uint16_t attribute,
+			   NTSTATUS (*callback_fn)(const char *,
+						   struct file_info *,
+						   const char *, void *),
+			   void *priv)
 {
-	char *mask;
-	char *base_dir;
-	char *user_mask = NULL;
-	unsigned attribute =
-		FILE_ATTRIBUTE_DIRECTORY |
-		FILE_ATTRIBUTE_SYSTEM |
-		FILE_ATTRIBUTE_HIDDEN;
+	char *mask = NULL;
 	unsigned info_level = SMB_FIND_FILE_BOTH_DIRECTORY_INFO;
-	struct tevent_req *req;
-	NTSTATUS status;
-	struct file_info *finfos;
+	struct file_info *finfos = NULL;
 	size_t i, num_finfos;
-	PyObject *result;
-
-	const char *kwlist[] = { "directory", "mask", "attribs", NULL };
-
-	if (!ParseTupleAndKeywords(args, kwds, "z|sH:list", kwlist,
-				   &base_dir, &user_mask, &attribute)) {
-		return NULL;
-	}
+	NTSTATUS status;
+	struct tevent_req *req = NULL;
 
 	if (user_mask == NULL) {
 		mask = talloc_asprintf(NULL, "%s\\*", base_dir);
 	} else {
 		mask = talloc_asprintf(NULL, "%s\\%s", base_dir, user_mask);
 	}
+
 	if (mask == NULL) {
-		PyErr_NoMemory();
-		return NULL;
+		return NT_STATUS_NO_MEMORY;
 	}
 	dos_format(mask);
 
 	req = cli_list_send(NULL, self->ev, self->cli, mask, attribute,
 			    info_level);
 	if (!py_tevent_req_wait_exc(self, req)) {
-		return NULL;
+		return NT_STATUS_INTERNAL_ERROR;
 	}
 	status = cli_list_recv(req, NULL, &finfos, &num_finfos);
 	TALLOC_FREE(req);
 	TALLOC_FREE(mask);
 
 	if (!NT_STATUS_IS_OK(status)) {
-		PyErr_SetNTSTATUS(status);
+		return status;
+	}
+
+	for (i = 0; i < num_finfos; i++) {
+		status = callback_fn(base_dir, &finfos[i], user_mask,
+				     priv);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
+		}
+	}
+	return status;
+}
+
+static PyObject *py_cli_list(struct py_cli_state *self,
+			     PyObject *args,
+			     PyObject *kwds)
+{
+	char *base_dir;
+	char *user_mask = NULL;
+	unsigned attribute =
+		FILE_ATTRIBUTE_DIRECTORY |
+		FILE_ATTRIBUTE_SYSTEM |
+		FILE_ATTRIBUTE_HIDDEN;
+	NTSTATUS status;
+	PyObject *result;
+	const char *kwlist[] = { "directory", "mask", "attribs", NULL };
+
+	if (!ParseTupleAndKeywords(args, kwds, "z|sH:list", kwlist,
+				   &base_dir, &user_mask, &attribute)) {
 		return NULL;
 	}
 
@@ -977,12 +995,13 @@ static PyObject *py_cli_list(struct py_cli_state *self,
 		return NULL;
 	}
 
-	for (i=0; i<num_finfos; i++) {
-		status = list_helper(base_dir, &finfos[i], user_mask, result);
-		if (!NT_STATUS_IS_OK(status)) {
-			Py_XDECREF(result);
-			return NULL;
-		}
+	status = do_listing(self, base_dir, user_mask, attribute,
+			    list_helper, result);
+
+	if (!NT_STATUS_IS_OK(status)) {
+		Py_XDECREF(result);
+		PyErr_SetNTSTATUS(status);
+		return NULL;
 	}
 
 	return result;
-- 
2.7.4


From be31a6d975cfdd61f251dc3de6276d082571bc2b Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 12 Dec 2018 12:08:24 +1300
Subject: [PATCH 08/19] s3:pylibsmb: Make .list() work for SMBv2

In order for .list() to work on an SMBv2 connection we need to
use the synchronous API.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 source3/libsmb/pylibsmb.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 7c06350..2b5c8dc 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -933,9 +933,9 @@ static NTSTATUS do_listing(struct py_cli_state *self,
 	char *mask = NULL;
 	unsigned info_level = SMB_FIND_FILE_BOTH_DIRECTORY_INFO;
 	struct file_info *finfos = NULL;
-	size_t i, num_finfos;
+	size_t i;
+	size_t num_finfos = 0;
 	NTSTATUS status;
-	struct tevent_req *req = NULL;
 
 	if (user_mask == NULL) {
 		mask = talloc_asprintf(NULL, "%s\\*", base_dir);
@@ -948,19 +948,27 @@ static NTSTATUS do_listing(struct py_cli_state *self,
 	}
 	dos_format(mask);
 
-	req = cli_list_send(NULL, self->ev, self->cli, mask, attribute,
-			    info_level);
-	if (!py_tevent_req_wait_exc(self, req)) {
-		return NT_STATUS_INTERNAL_ERROR;
+	if (self->is_smb1) {
+		struct tevent_req *req = NULL;
+
+		req = cli_list_send(NULL, self->ev, self->cli, mask, attribute,
+				    info_level);
+		if (!py_tevent_req_wait_exc(self, req)) {
+			return NT_STATUS_INTERNAL_ERROR;
+		}
+		status = cli_list_recv(req, NULL, &finfos, &num_finfos);
+		TALLOC_FREE(req);
+	} else {
+		status = cli_list(self->cli, mask, attribute, callback_fn,
+				  priv);
 	}
-	status = cli_list_recv(req, NULL, &finfos, &num_finfos);
-	TALLOC_FREE(req);
 	TALLOC_FREE(mask);
 
 	if (!NT_STATUS_IS_OK(status)) {
 		return status;
 	}
 
+	/* invoke the callback for the async results (SMBv1 connections) */
 	for (i = 0; i < num_finfos; i++) {
 		status = callback_fn(base_dir, &finfos[i], user_mask,
 				     priv);
-- 
2.7.4


From ea01461400514302e6e5464f0c35d43d31ddd0b9 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 4 Dec 2018 16:03:35 +1300
Subject: [PATCH 09/19] s3:pylibsmb: Don't return '.'/'..' in .list()

The source4 .list() API wasn't doing this. This makes source3 and source4
have *almost* equivalent functionality, so we can start usign the
source3 API in the tests.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/smb.py | 6 +++---
 source3/libsmb/pylibsmb.c | 5 +++++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index 497f57d..b951a4b 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -64,7 +64,7 @@ class SMBTests(samba.tests.TestCase):
 
     def test_list(self):
         # check a basic listing returns the items we expect
-        ls = [f['name'] for f in self.conn.list(addom)]
+        ls = [f['name'] for f in self.smb_conn.list(addom)]
         self.assertIn('scripts', ls,
                       msg='"scripts" directory not found in sysvol')
         self.assertIn('Policies', ls,
@@ -75,11 +75,11 @@ class SMBTests(samba.tests.TestCase):
                          msg='Current dir (.) found in directory listing')
 
         # using a '*' mask should be the same as using no mask
-        ls_wildcard = [f['name'] for f in self.conn.list(addom, "*")]
+        ls_wildcard = [f['name'] for f in self.smb_conn.list(addom, "*")]
         self.assertEqual(ls, ls_wildcard)
 
         # applying a mask should only return items that match that mask
-        ls_pol = [f['name'] for f in self.conn.list(addom, "Pol*")]
+        ls_pol = [f['name'] for f in self.smb_conn.list(addom, "Pol*")]
         expected = ["Policies"]
         self.assertEqual(ls_pol, expected)
 
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 2b5c8dc..15c051f 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -907,6 +907,11 @@ static NTSTATUS list_helper(const char *mntpoint, struct file_info *finfo,
 	PyObject *file = NULL;
 	int ret;
 
+	/* suppress '.' and '..' in the results we return */
+	if (ISDOT(finfo->name) || ISDOTDOT(finfo->name)) {
+		return NT_STATUS_OK;
+	}
+
 	file = Py_BuildValue("{s:s,s:i}",
 			     "name", finfo->name,
 			     "mode", (int)finfo->mode);
-- 
2.7.4


From 218ec694e677d2dee69c495718f95a6692c60774 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 4 Dec 2018 16:25:10 +1300
Subject: [PATCH 10/19] s3:pylibsmb: Make s3 and s4 listings return the same
 dict

Make the python dictionary generated by the s3 .list() use the same keys
as the current source4 dict. The reason for using the source4 dict is
that other python code depends on these keys (e.g. ntacls.py), whereas
the source3 API is currently unused.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 python/samba/tests/smb.py |  2 +-
 source3/libsmb/pylibsmb.c | 20 +++++++++++++++++---
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index b951a4b..28560ca 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -85,7 +85,7 @@ class SMBTests(samba.tests.TestCase):
 
         # each item in the listing is a has with expected keys
         expected_keys = ['attrib', 'mtime', 'name', 'short_name', 'size']
-        for item in self.conn.list(addom):
+        for item in self.smb_conn.list(addom):
             for key in expected_keys:
                 self.assertIn(key, item,
                               msg="Key '%s' not in listing '%s'" % (key, item))
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 15c051f..64da389 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -912,9 +912,18 @@ static NTSTATUS list_helper(const char *mntpoint, struct file_info *finfo,
 		return NT_STATUS_OK;
 	}
 
-	file = Py_BuildValue("{s:s,s:i}",
+	/*
+	 * Build a dictionary representing the file info.
+	 * Note: Windows does not always return short_name (so it may be None)
+	 */
+	file = Py_BuildValue("{s:s,s:i,s:s,s:O,s:l}",
 			     "name", finfo->name,
-			     "mode", (int)finfo->mode);
+			     "attrib", (int)finfo->mode,
+			     "short_name", finfo->short_name,
+			     "size", PyLong_FromUnsignedLongLong(finfo->size),
+			     "mtime",
+			     convert_timespec_to_time_t(finfo->mtime_ts));
+
 	if (file == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
@@ -1184,7 +1193,12 @@ static PyMethodDef py_cli_state_methods[] = {
 	  "directory contents as a dictionary\n"
 	  "\t\tDEFAULT_ATTRS: FILE_ATTRIBUTE_SYSTEM | "
 	  "FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_ARCHIVE\n\n"
-	  "\t\tList contents of a directory." },
+	  "\t\tList contents of a directory. The keys are, \n"
+	  "\t\t\tname: Long name of the directory item\n"
+	  "\t\t\tshort_name: Short name of the directory item\n"
+	  "\t\t\tsize: File size in bytes\n"
+	  "\t\t\tattrib: Attributes\n"
+	  "\t\t\tmtime: Modification time\n" },
 	{ "get_oplock_break", (PyCFunction)py_cli_get_oplock_break,
 	  METH_VARARGS, "Wait for an oplock break" },
 	{ "unlink", (PyCFunction)py_smb_unlink,
-- 
2.7.4


From 51acd1f36f20ec8c235405c4e6f497c1e1873f63 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 12 Dec 2018 12:23:42 +1300
Subject: [PATCH 11/19] s3:pylibsmb: Free async .list() memory

finfos was being allocated but never freed.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 source3/libsmb/pylibsmb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 64da389..156352d 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -987,9 +987,12 @@ static NTSTATUS do_listing(struct py_cli_state *self,
 		status = callback_fn(base_dir, &finfos[i], user_mask,
 				     priv);
 		if (!NT_STATUS_IS_OK(status)) {
+			TALLOC_FREE(finfos);
 			return status;
 		}
 	}
+
+	TALLOC_FREE(finfos);
 	return status;
 }
 
-- 
2.7.4


From 48ab831725ebad48513986f5e6fc8b0198704507 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 11 Dec 2018 09:34:42 +1300
Subject: [PATCH 12/19] s3:pylibsmb: Add .savefile() API to SMB py bindings

This provides a simple API for writing a file's contents and makes the
s3 API consistent with the s4 API.

All the async APIs here support SMBv2 so we don't need to use the sync
APIs at all.

Note that we have the choice here of using either cli_write_send() or
cli_push_send(). I chose the latter, because that's what smbclient uses.
It also appears to handle writing a large file better (i.e. one that
exceeds the max write size of the underlying connection).

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 python/samba/tests/smb.py | 16 ++++-----
 selftest/knownfail.d/smb  |  2 --
 source3/libsmb/pylibsmb.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 97 insertions(+), 10 deletions(-)
 delete mode 100644 selftest/knownfail.d/smb

diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index 28560ca..df590ff 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -113,7 +113,7 @@ class SMBTests(samba.tests.TestCase):
             for i in range(1, 4):
                 contents = "I'm file {0} in dir {1}!".format(i, subdir)
                 path = self.make_sysvol_path(subdir, "file-{0}.txt".format(i))
-                self.conn.savefile(path, test_contents.encode('utf8'))
+                self.smb_conn.savefile(path, test_contents.encode('utf8'))
                 filepaths.append(path)
 
         # sanity-check these dirs/files exist
@@ -166,7 +166,7 @@ class SMBTests(samba.tests.TestCase):
         """
         # create the test file
         self.assertFalse(self.file_exists(test_file))
-        self.conn.savefile(test_file, binary_contents)
+        self.smb_conn.savefile(test_file, binary_contents)
         self.assertTrue(self.file_exists(test_file))
 
         # delete it and check that it's gone
@@ -183,7 +183,7 @@ class SMBTests(samba.tests.TestCase):
         self.assertFalse(self.smb_conn.chkpath(bad_dir))
 
         # should return False for files (because they're not directories)
-        self.conn.savefile(test_file, binary_contents)
+        self.smb_conn.savefile(test_file, binary_contents)
         self.assertFalse(self.smb_conn.chkpath(test_file))
 
         # check correct result after creating and then deleting a new dir
@@ -195,7 +195,7 @@ class SMBTests(samba.tests.TestCase):
 
     def test_save_load_text(self):
 
-        self.conn.savefile(test_file, test_contents.encode('utf8'))
+        self.smb_conn.savefile(test_file, test_contents.encode('utf8'))
 
         contents = self.conn.loadfile(test_file)
         self.assertEquals(contents.decode('utf8'), test_contents,
@@ -203,7 +203,7 @@ class SMBTests(samba.tests.TestCase):
 
         # check we can overwrite the file with new contents
         new_contents = 'wxyz' * 128
-        self.conn.savefile(test_file, new_contents.encode('utf8'))
+        self.smb_conn.savefile(test_file, new_contents.encode('utf8'))
         contents = self.conn.loadfile(test_file)
         self.assertEquals(contents.decode('utf8'), new_contents,
                           msg='contents of test file did not match what was written')
@@ -211,7 +211,7 @@ class SMBTests(samba.tests.TestCase):
     # with python2 this will save/load str type (with embedded nulls)
     # with python3 this will save/load bytes type
     def test_save_load_string_bytes(self):
-        self.conn.savefile(test_file, test_literal_bytes_embed_nulls)
+        self.smb_conn.savefile(test_file, test_literal_bytes_embed_nulls)
 
         contents = self.conn.loadfile(test_file)
         self.assertEquals(contents, test_literal_bytes_embed_nulls,
@@ -220,7 +220,7 @@ class SMBTests(samba.tests.TestCase):
     # python3 only this will save/load unicode
     def test_save_load_utfcontents(self):
         if PY3:
-            self.conn.savefile(test_file, utf_contents.encode('utf8'))
+            self.smb_conn.savefile(test_file, utf_contents.encode('utf8'))
 
             contents = self.conn.loadfile(test_file)
             self.assertEquals(contents.decode('utf8'), utf_contents,
@@ -229,7 +229,7 @@ class SMBTests(samba.tests.TestCase):
     # with python2 this will save/load str type
     # with python3 this will save/load bytes type
     def test_save_binary_contents(self):
-        self.conn.savefile(test_file, binary_contents)
+        self.smb_conn.savefile(test_file, binary_contents)
 
         contents = self.conn.loadfile(test_file)
         self.assertEquals(contents, binary_contents,
diff --git a/selftest/knownfail.d/smb b/selftest/knownfail.d/smb
deleted file mode 100644
index 3c6324a..0000000
--- a/selftest/knownfail.d/smb
+++ /dev/null
@@ -1,2 +0,0 @@
-# currently savefile appends rather than overwriting
-samba.tests.smb.*samba.tests.smb.SMBTests.test_save_load_text\(ad_dc:local\)
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 156352d..2a1ae29 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -741,6 +741,92 @@ static PyObject *py_cli_close(struct py_cli_state *self, PyObject *args)
 	Py_RETURN_NONE;
 }
 
+struct push_state {
+	char *data;
+	off_t nread;
+	off_t total_data;
+};
+
+/*
+ * cli_push() helper to write a chunk of data to a remote file
+ */
+static size_t push_data(uint8_t *buf, size_t n, void *priv)
+{
+	struct push_state *state = (struct push_state *)priv;
+	char *curr_ptr = NULL;
+	off_t remaining;
+	size_t copied_bytes;
+
+	if (state->nread >= state->total_data) {
+		return 0;
+	}
+
+	curr_ptr = state->data + state->nread;
+	remaining = state->total_data - state->nread;
+	copied_bytes = MIN(remaining, n);
+
+	memcpy(buf, curr_ptr, copied_bytes);
+	state->nread += copied_bytes;
+	return copied_bytes;
+}
+
+/*
+ * Writes a file with the contents specified
+ */
+static PyObject *py_smb_savefile(struct py_cli_state *self, PyObject *args,
+				 PyObject *kwargs)
+{
+	uint16_t fnum;
+	const char *filename = NULL;
+	char *data = NULL;
+	Py_ssize_t size = 0;
+	NTSTATUS status;
+	struct tevent_req *req = NULL;
+	struct push_state state;
+
+	if (!PyArg_ParseTuple(args, "s"PYARG_BYTES_LEN":savefile", &filename,
+			      &data, &size)) {
+		return NULL;
+	}
+
+	/* create a new file handle for writing to */
+	req = cli_ntcreate_send(NULL, self->ev, self->cli, filename, 0,
+				FILE_WRITE_DATA, FILE_ATTRIBUTE_NORMAL,
+				FILE_SHARE_READ|FILE_SHARE_WRITE,
+				FILE_OVERWRITE_IF, FILE_NON_DIRECTORY_FILE,
+				SMB2_IMPERSONATION_IMPERSONATION, 0);
+	if (!py_tevent_req_wait_exc(self, req)) {
+		return NULL;
+	}
+	status = cli_ntcreate_recv(req, &fnum, NULL);
+	TALLOC_FREE(req);
+	PyErr_NTSTATUS_IS_ERR_RAISE(status);
+
+	/* write the new file contents */
+	state.data = data;
+	state.nread = 0;
+	state.total_data = size;
+
+	req = cli_push_send(NULL, self->ev, self->cli, fnum, 0, 0, 0,
+			    push_data, &state);
+	if (!py_tevent_req_wait_exc(self, req)) {
+		return NULL;
+	}
+	status = cli_push_recv(req);
+	TALLOC_FREE(req);
+	PyErr_NTSTATUS_IS_ERR_RAISE(status);
+
+	/* close the file handle */
+	req = cli_close_send(NULL, self->ev, self->cli, fnum);
+	if (!py_tevent_req_wait_exc(self, req)) {
+		return NULL;
+	}
+	status = cli_close_recv(req);
+	PyErr_NTSTATUS_IS_ERR_RAISE(status);
+
+	Py_RETURN_NONE;
+}
+
 static PyObject *py_cli_write(struct py_cli_state *self, PyObject *args,
 			      PyObject *kwds)
 {
@@ -1214,6 +1300,9 @@ static PyMethodDef py_cli_state_methods[] = {
 	{ "chkpath", (PyCFunction)py_smb_chkpath, METH_VARARGS,
 	  "chkpath(dir_path) -> True or False\n\n"
 	  "\t\tReturn true if directory exists, false otherwise." },
+	{ "savefile", (PyCFunction)py_smb_savefile, METH_VARARGS,
+	  "savefile(path, str) -> None\n\n"
+	  "\t\tWrite " PY_DESC_PY3_BYTES " str to file." },
 	{ NULL, NULL, 0, NULL }
 };
 
-- 
2.7.4


From 6d9f83e071e82efe1856d9c0fe0ef837553e809a Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 5 Dec 2018 15:08:09 +1300
Subject: [PATCH 13/19] s3:pylibsmb: Add .loadfile() API to SMB py bindings

Add a .loadfile API to read a file's contents. This provides a
convenient way to read a file and is consistent with the existing
source4 API, which is used by things like the GPO python code and the
ntacls backup.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 python/samba/tests/smb.py |  12 ++---
 source3/libsmb/pylibsmb.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 137 insertions(+), 6 deletions(-)

diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index df590ff..bddadc5 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -150,7 +150,7 @@ class SMBTests(samba.tests.TestCase):
     def file_exists(self, filepath):
         """Returns whether a regular file exists (by trying to open it)"""
         try:
-            self.conn.loadfile(filepath)
+            self.smb_conn.loadfile(filepath)
             exists = True;
         except NTSTATUSError as err:
             if (err.args[0] == NT_STATUS_OBJECT_NAME_NOT_FOUND or
@@ -197,14 +197,14 @@ class SMBTests(samba.tests.TestCase):
 
         self.smb_conn.savefile(test_file, test_contents.encode('utf8'))
 
-        contents = self.conn.loadfile(test_file)
+        contents = self.smb_conn.loadfile(test_file)
         self.assertEquals(contents.decode('utf8'), test_contents,
                           msg='contents of test file did not match what was written')
 
         # check we can overwrite the file with new contents
         new_contents = 'wxyz' * 128
         self.smb_conn.savefile(test_file, new_contents.encode('utf8'))
-        contents = self.conn.loadfile(test_file)
+        contents = self.smb_conn.loadfile(test_file)
         self.assertEquals(contents.decode('utf8'), new_contents,
                           msg='contents of test file did not match what was written')
 
@@ -213,7 +213,7 @@ class SMBTests(samba.tests.TestCase):
     def test_save_load_string_bytes(self):
         self.smb_conn.savefile(test_file, test_literal_bytes_embed_nulls)
 
-        contents = self.conn.loadfile(test_file)
+        contents = self.smb_conn.loadfile(test_file)
         self.assertEquals(contents, test_literal_bytes_embed_nulls,
                           msg='contents of test file did not match what was written')
 
@@ -222,7 +222,7 @@ class SMBTests(samba.tests.TestCase):
         if PY3:
             self.smb_conn.savefile(test_file, utf_contents.encode('utf8'))
 
-            contents = self.conn.loadfile(test_file)
+            contents = self.smb_conn.loadfile(test_file)
             self.assertEquals(contents.decode('utf8'), utf_contents,
                               msg='contents of test file did not match what was written')
 
@@ -231,7 +231,7 @@ class SMBTests(samba.tests.TestCase):
     def test_save_binary_contents(self):
         self.smb_conn.savefile(test_file, binary_contents)
 
-        contents = self.conn.loadfile(test_file)
+        contents = self.smb_conn.loadfile(test_file)
         self.assertEquals(contents, binary_contents,
                           msg='contents of test file did not match what was written')
 
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 2a1ae29..2190b7c 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -28,6 +28,7 @@
 #include "source4/libcli/util/pyerrors.h"
 #include "auth/credentials/pycredentials.h"
 #include "trans2.h"
+#include "libsmb/clirap.h"
 
 static PyTypeObject *get_pytype(const char *module, const char *type)
 {
@@ -863,6 +864,133 @@ static PyObject *py_cli_write(struct py_cli_state *self, PyObject *args,
 	return Py_BuildValue("K", (unsigned long long)written);
 }
 
+/*
+ * Returns the size of the given file
+ */
+static NTSTATUS py_smb_filesize(struct py_cli_state *self, uint16_t fnum,
+				off_t *size)
+{
+	NTSTATUS status;
+
+	if (self->is_smb1) {
+		uint8_t *rdata = NULL;
+		struct tevent_req *req = NULL;
+
+		req = cli_qfileinfo_send(NULL, self->ev, self->cli, fnum,
+					 SMB_QUERY_FILE_ALL_INFO, 68,
+					 CLI_BUFFER_SIZE);
+		if (!py_tevent_req_wait_exc(self, req)) {
+			return NT_STATUS_INTERNAL_ERROR;
+		}
+		status = cli_qfileinfo_recv(req, NULL, NULL, &rdata, NULL);
+		if (NT_STATUS_IS_OK(status)) {
+			*size = IVAL2_TO_SMB_BIG_UINT(rdata, 48);
+		}
+		TALLOC_FREE(req);
+		TALLOC_FREE(rdata);
+	} else {
+		status = cli_qfileinfo_basic(self->cli, fnum, NULL, size,
+					     NULL, NULL, NULL, NULL, NULL);
+	}
+	return status;
+}
+
+static NTSTATUS pull_helper(char *buf, size_t n, void *priv)
+{
+	char **dest_buf = (char **)priv;
+	memcpy(*dest_buf, buf, n);
+	*dest_buf += n;
+	return NT_STATUS_OK;
+}
+
+/*
+ * Loads the specified file's contents and returns it
+ */
+static PyObject *py_smb_loadfile(struct py_cli_state *self, PyObject *args,
+				 PyObject *kwargs)
+{
+	NTSTATUS status;
+	const char *filename = NULL;
+	struct tevent_req *req = NULL;
+	uint16_t fnum;
+	off_t size;
+	char *buf = NULL;
+	off_t nread = 0;
+	PyObject *result = NULL;
+
+	if (!PyArg_ParseTuple(args, "s:loadfile", &filename)) {
+		return NULL;
+	}
+
+	/* get a read file handle */
+	req = cli_ntcreate_send(NULL, self->ev, self->cli, filename, 0,
+				FILE_READ_DATA, FILE_ATTRIBUTE_NORMAL,
+				FILE_SHARE_READ, FILE_OPEN, 0,
+				SMB2_IMPERSONATION_IMPERSONATION, 0);
+	if (!py_tevent_req_wait_exc(self, req)) {
+		return NULL;
+	}
+	status = cli_ntcreate_recv(req, &fnum, NULL);
+	TALLOC_FREE(req);
+	PyErr_NTSTATUS_IS_ERR_RAISE(status);
+
+	/* get a buffer to hold the file contents */
+	status = py_smb_filesize(self, fnum, &size);
+	PyErr_NTSTATUS_IS_ERR_RAISE(status);
+
+	result = PyBytes_FromStringAndSize(NULL, size);
+	if (result == NULL) {
+		return NULL;
+	}
+
+	/* read the file contents */
+	buf = PyBytes_AS_STRING(result);
+	req = cli_pull_send(NULL, self->ev, self->cli, fnum, 0, size,
+			    size, pull_helper, &buf);
+	if (!py_tevent_req_wait_exc(self, req)) {
+		Py_XDECREF(result);
+		return NULL;
+	}
+	status = cli_pull_recv(req, &nread);
+	TALLOC_FREE(req);
+	if (!NT_STATUS_IS_OK(status)) {
+		Py_XDECREF(result);
+		PyErr_SetNTSTATUS(status);
+		return NULL;
+	}
+
+	/* close the file handle */
+	req = cli_close_send(NULL, self->ev, self->cli, fnum);
+	if (!py_tevent_req_wait_exc(self, req)) {
+		Py_XDECREF(result);
+		return NULL;
+	}
+	status = cli_close_recv(req);
+	TALLOC_FREE(req);
+	if (!NT_STATUS_IS_OK(status)) {
+		Py_XDECREF(result);
+		PyErr_SetNTSTATUS(status);
+		return NULL;
+	}
+
+	/* sanity-check we read the expected number of bytes */
+	if (nread > size) {
+		Py_XDECREF(result);
+		PyErr_Format(PyExc_IOError,
+			     "read invalid - got %zu requested %zu",
+			     nread, size);
+		return NULL;
+	}
+
+	if (nread < size) {
+		if (_PyBytes_Resize(&result, nread) < 0) {
+			return NULL;
+		}
+	}
+
+	return result;
+}
+
 static PyObject *py_cli_read(struct py_cli_state *self, PyObject *args,
 			     PyObject *kwds)
 {
@@ -1303,6 +1431,9 @@ static PyMethodDef py_cli_state_methods[] = {
 	{ "savefile", (PyCFunction)py_smb_savefile, METH_VARARGS,
 	  "savefile(path, str) -> None\n\n"
 	  "\t\tWrite " PY_DESC_PY3_BYTES " str to file." },
+	{ "loadfile", (PyCFunction)py_smb_loadfile, METH_VARARGS,
+	  "loadfile(path) -> file contents as a " PY_DESC_PY3_BYTES
+	  "\n\n\t\tRead contents of a file." },
 	{ NULL, NULL, 0, NULL }
 };
 
-- 
2.7.4


From a963308f24933d980a4e936acb8a2ceea9285523 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 3 Jan 2019 17:48:39 +1300
Subject: [PATCH 14/19] s3:libsmb: Avoid duplicated code by making
 cli_read_sink() public

cli_read_sink() and pull_helper() were essentially identical. By making
cli_read_sink() non-static, we can delete the latter.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 source3/libsmb/clireadwrite.c |  6 +++++-
 source3/libsmb/proto.h        |  1 +
 source3/libsmb/pylibsmb.c     | 10 +---------
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/source3/libsmb/clireadwrite.c b/source3/libsmb/clireadwrite.c
index e953fa5..da188db 100644
--- a/source3/libsmb/clireadwrite.c
+++ b/source3/libsmb/clireadwrite.c
@@ -822,7 +822,11 @@ NTSTATUS cli_read_recv(struct tevent_req *req, size_t *received)
 	return NT_STATUS_OK;
 }
 
-static NTSTATUS cli_read_sink(char *buf, size_t n, void *priv)
+/*
+ * Helper function for cli_pull(). This takes a chunk of data (buf) read from
+ * a remote file and copies it into the return buffer (priv).
+ */
+NTSTATUS cli_read_sink(char *buf, size_t n, void *priv)
 {
 	char **pbuf = (char **)priv;
 	memcpy(*pbuf, buf, n);
diff --git a/source3/libsmb/proto.h b/source3/libsmb/proto.h
index bfad4dc..b0cfcb5 100644
--- a/source3/libsmb/proto.h
+++ b/source3/libsmb/proto.h
@@ -840,6 +840,7 @@ NTSTATUS cli_pull(struct cli_state *cli, uint16_t fnum,
 		  off_t start_offset, off_t size, size_t window_size,
 		  NTSTATUS (*sink)(char *buf, size_t n, void *priv),
 		  void *priv, off_t *received);
+NTSTATUS cli_read_sink(char *buf, size_t n, void *priv);
 struct tevent_req *cli_read_send(
 	TALLOC_CTX *mem_ctx,
 	struct tevent_context *ev,
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 2190b7c..08de5d0 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -895,14 +895,6 @@ static NTSTATUS py_smb_filesize(struct py_cli_state *self, uint16_t fnum,
 	return status;
 }
 
-static NTSTATUS pull_helper(char *buf, size_t n, void *priv)
-{
-	char **dest_buf = (char **)priv;
-	memcpy(*dest_buf, buf, n);
-	*dest_buf += n;
-	return NT_STATUS_OK;
-}
-
 /*
  * Loads the specified file's contents and returns it
  */
@@ -946,7 +938,7 @@ static PyObject *py_smb_loadfile(struct py_cli_state *self, PyObject *args,
 	/* read the file contents */
 	buf = PyBytes_AS_STRING(result);
 	req = cli_pull_send(NULL, self->ev, self->cli, fnum, 0, size,
-			    size, pull_helper, &buf);
+			    size, cli_read_sink, &buf);
 	if (!py_tevent_req_wait_exc(self, req)) {
 		Py_XDECREF(result);
 		return NULL;
-- 
2.7.4


From 94ad24b720763191558e7f2af210c4238672b9dc Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 12 Dec 2018 13:47:46 +1300
Subject: [PATCH 15/19] s3:pylibsmb: Minor refactor to py_cli_list() variables

Add a define for the file attribute mask (we'll reuse it in a subsequent
patch), and make the variable type explicit.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 source3/libsmb/pylibsmb.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 08de5d0..4438945 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -30,6 +30,9 @@
 #include "trans2.h"
 #include "libsmb/clirap.h"
 
+#define LIST_ATTRIBUTE_MASK \
+	(FILE_ATTRIBUTE_DIRECTORY|FILE_ATTRIBUTE_SYSTEM|FILE_ATTRIBUTE_HIDDEN)
+
 static PyTypeObject *get_pytype(const char *module, const char *type)
 {
 	PyObject *mod;
@@ -1151,7 +1154,7 @@ static NTSTATUS do_listing(struct py_cli_state *self,
 			   void *priv)
 {
 	char *mask = NULL;
-	unsigned info_level = SMB_FIND_FILE_BOTH_DIRECTORY_INFO;
+	unsigned int info_level = SMB_FIND_FILE_BOTH_DIRECTORY_INFO;
 	struct file_info *finfos = NULL;
 	size_t i;
 	size_t num_finfos = 0;
@@ -1208,10 +1211,7 @@ static PyObject *py_cli_list(struct py_cli_state *self,
 {
 	char *base_dir;
 	char *user_mask = NULL;
-	unsigned attribute =
-		FILE_ATTRIBUTE_DIRECTORY |
-		FILE_ATTRIBUTE_SYSTEM |
-		FILE_ATTRIBUTE_HIDDEN;
+	unsigned int attribute = LIST_ATTRIBUTE_MASK;
 	NTSTATUS status;
 	PyObject *result;
 	const char *kwlist[] = { "directory", "mask", "attribs", NULL };
-- 
2.7.4


From 3a65d8650b1e8f684b130e1d8c89c348161c14d4 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 12 Dec 2018 13:45:46 +1300
Subject: [PATCH 16/19] s3:pylibsmb: Add .deltree() API to SMB py bindings

This basically re-uses the underlying functionality of existing APIs in
order to support a .deltree() API, i.e.
- we use the .list() functionality (i.e. do_listing()) to traverse every
  item in the given directory.
- we then use either .unlink() (i.e. unlink_file()) or .rmdir() (i.e.
  remove_dir()) to delete the individual item.
- sub-directories are handled recursively, by repeating the process.

Note that the .deltree() API is currently only really used for testing
(and deleting GPO files). So the recursion is never going to be
excessive.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/smb.py |  9 +++--
 source3/libsmb/pylibsmb.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 100 insertions(+), 4 deletions(-)

diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index bddadc5..df3edd0 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -58,7 +58,7 @@ class SMBTests(samba.tests.TestCase):
     def tearDown(self):
         super(SMBTests, self).tearDown()
         try:
-            self.conn.deltree(test_dir)
+            self.smb_conn.deltree(test_dir)
         except:
             pass
 
@@ -96,6 +96,7 @@ class SMBTests(samba.tests.TestCase):
         dirpaths = []
         empty_dirs = []
         cur_dir = test_dir
+
         for subdir in ["subdir-X", "subdir-Y", "subdir-Z"]:
             path = self.make_sysvol_path(cur_dir, subdir)
             self.smb_conn.mkdir(path)
@@ -126,18 +127,18 @@ class SMBTests(samba.tests.TestCase):
 
         # try using deltree to remove a single empty directory
         path = empty_dirs.pop(0)
-        self.conn.deltree(path)
+        self.smb_conn.deltree(path)
         self.assertFalse(self.smb_conn.chkpath(path),
                          "Failed to delete {0}".format(path))
 
         # try using deltree to remove a single file
         path = filepaths.pop(0)
-        self.conn.deltree(path)
+        self.smb_conn.deltree(path)
         self.assertFalse(self.file_exists(path),
                          "Failed to delete {0}".format(path))
 
         # delete the top-level dir
-        self.conn.deltree(test_dir)
+        self.smb_conn.deltree(test_dir)
 
         # now check that all the dirs/files are no longer there
         for subdir in dirpaths + empty_dirs:
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 4438945..452c30e 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -1380,6 +1380,98 @@ static PyObject *py_smb_chkpath(struct py_cli_state *self, PyObject *args)
 	return PyBool_FromLong(dir_exists);
 }
 
+struct deltree_state {
+	struct py_cli_state *self;
+	const char *full_dirpath;
+};
+
+static NTSTATUS delete_dir_tree(struct py_cli_state *self,
+				const char *dirpath);
+
+/*
+ * Deletes a single item in the directory tree. This could be either a file
+ * or a directory. This function gets invoked as a callback for every item in
+ * the given directory's listings.
+ */
+static NTSTATUS delete_tree_callback(const char *mntpoint,
+				     struct file_info *finfo,
+				     const char *mask, void *priv)
+{
+	char *filepath = NULL;
+	struct deltree_state *state = priv;
+	NTSTATUS status;
+
+	/* skip '.' or '..' directory listings */
+	if (ISDOT(finfo->name) || ISDOTDOT(finfo->name)) {
+		return NT_STATUS_OK;
+	}
+
+	/* get the absolute filepath */
+	filepath = talloc_asprintf(NULL, "%s\\%s", state->full_dirpath,
+				   finfo->name);
+	if (filepath == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	if (finfo->mode & FILE_ATTRIBUTE_DIRECTORY) {
+
+		/* recursively delete the sub-directory and its contents */
+		status = delete_dir_tree(state->self, filepath);
+	} else {
+		status = unlink_file(state->self, filepath);
+	}
+
+	TALLOC_FREE(filepath);
+	return status;
+}
+
+/*
+ * Removes a directory and all its contents
+ */
+static NTSTATUS delete_dir_tree(struct py_cli_state *self,
+				const char *filepath)
+{
+	NTSTATUS status;
+	const char *mask = "*";
+	struct deltree_state state = { 0 };
+
+	/* go through the directory's contents, deleting each item */
+	state.self = self;
+	state.full_dirpath = filepath;
+	status = do_listing(self, filepath, mask, LIST_ATTRIBUTE_MASK,
+			    delete_tree_callback, &state);
+
+	/* remove the directory itself */
+	if (NT_STATUS_IS_OK(status)) {
+		status = remove_dir(self, filepath);
+	}
+	return status;
+}
+
+static PyObject *py_smb_deltree(struct py_cli_state *self, PyObject *args)
+{
+	NTSTATUS status;
+	const char *filepath = NULL;
+	bool dir_exists;
+
+	if (!PyArg_ParseTuple(args, "s:deltree", &filepath)) {
+		return NULL;
+	}
+
+	/* check whether we're removing a directory or a file */
+	dir_exists = check_dir_path(self, filepath);
+
+	if (dir_exists) {
+		status = delete_dir_tree(self, filepath);
+	} else {
+		status = unlink_file(self, filepath);
+	}
+
+	PyErr_NTSTATUS_IS_ERR_RAISE(status);
+
+	Py_RETURN_NONE;
+}
+
 static PyMethodDef py_cli_state_methods[] = {
 	{ "settimeout", (PyCFunction)py_cli_settimeout, METH_VARARGS,
 	  "settimeout(new_timeout_msecs) => return old_timeout_msecs" },
@@ -1426,6 +1518,9 @@ static PyMethodDef py_cli_state_methods[] = {
 	{ "loadfile", (PyCFunction)py_smb_loadfile, METH_VARARGS,
 	  "loadfile(path) -> file contents as a " PY_DESC_PY3_BYTES
 	  "\n\n\t\tRead contents of a file." },
+	{ "deltree", (PyCFunction)py_smb_deltree, METH_VARARGS,
+	  "deltree(path) -> None\n\n"
+	  "\t\tDelete a directory and all its contents." },
 	{ NULL, NULL, 0, NULL }
 };
 
-- 
2.7.4


From ba79ceca2f9a1f1d6812a761d3c6658826b405e3 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 12 Dec 2018 14:42:30 +1300
Subject: [PATCH 17/19] tests: Completely replace s4 connection in smb tests

This test now uses the s3 python bindings completely, so we can remove
the s4 connection.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/smb.py | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index df3edd0..98e669c 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -42,12 +42,8 @@ class SMBTests(samba.tests.TestCase):
         super(SMBTests, self).setUp()
         self.server = os.environ["SERVER"]
         creds = self.insta_creds(template=self.get_credentials())
-        self.conn = smb.SMB(self.server,
-                            "sysvol",
-                            lp=self.get_loadparm(),
-                            creds=creds)
 
-        # temporarily create a 2nd SMB connection for migrating the py-bindings
+        # create an SMB connection to the server
         lp = s3param.get_context()
         lp.load(os.getenv("SMB_CONF_PATH"))
         self.smb_conn = libsmb_samba_internal.Conn(self.server, "sysvol",
-- 
2.7.4


From 29aceee5bb6f70605f91a7ccc3463766071481f6 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 12 Dec 2018 10:04:09 +1300
Subject: [PATCH 18/19] tests: Avoid hardcoding domain in test

Currently the sysvol domain directory is hard-coded, so the tests can
only ever run on the ad_dc.

This patch makes things marginally better by using the REALM
environmental variable instead. This allows us to run it against other
testenvs (like the SMBv2-only restoredc).

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/smb.py | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index 98e669c..9700953 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -27,13 +27,14 @@ from samba.samba3 import libsmb_samba_internal
 from samba.samba3 import param as s3param
 
 PY3 = sys.version_info[0] == 3
-addom = 'addom.samba.example.com/'
+realm = os.environ.get('REALM')
+domain_dir = realm.lower() + '/'
 test_contents = 'abcd' * 256
 utf_contents = u'Süßigkeiten Äpfel ' * 128
 test_literal_bytes_embed_nulls = b'\xff\xfe\x14\x61\x00\x00\x62\x63\x64' * 256
 binary_contents = b'\xff\xfe'
 binary_contents = binary_contents + "Hello cruel world of python3".encode('utf8') * 128
-test_dir = os.path.join(addom, 'testing_%d' % random.randint(0, 0xFFFF))
+test_dir = os.path.join(domain_dir, 'testing_%d' % random.randint(0, 0xFFFF))
 test_file = os.path.join(test_dir, 'testing').replace('/', '\\')
 
 
@@ -60,7 +61,7 @@ class SMBTests(samba.tests.TestCase):
 
     def test_list(self):
         # check a basic listing returns the items we expect
-        ls = [f['name'] for f in self.smb_conn.list(addom)]
+        ls = [f['name'] for f in self.smb_conn.list(domain_dir)]
         self.assertIn('scripts', ls,
                       msg='"scripts" directory not found in sysvol')
         self.assertIn('Policies', ls,
@@ -71,17 +72,17 @@ class SMBTests(samba.tests.TestCase):
                          msg='Current dir (.) found in directory listing')
 
         # using a '*' mask should be the same as using no mask
-        ls_wildcard = [f['name'] for f in self.smb_conn.list(addom, "*")]
+        ls_wildcard = [f['name'] for f in self.smb_conn.list(domain_dir, "*")]
         self.assertEqual(ls, ls_wildcard)
 
         # applying a mask should only return items that match that mask
-        ls_pol = [f['name'] for f in self.smb_conn.list(addom, "Pol*")]
+        ls_pol = [f['name'] for f in self.smb_conn.list(domain_dir, "Pol*")]
         expected = ["Policies"]
         self.assertEqual(ls_pol, expected)
 
         # each item in the listing is a has with expected keys
         expected_keys = ['attrib', 'mtime', 'name', 'short_name', 'size']
-        for item in self.smb_conn.list(addom):
+        for item in self.smb_conn.list(domain_dir):
             for key in expected_keys:
                 self.assertIn(key, item,
                               msg="Key '%s' not in listing '%s'" % (key, item))
-- 
2.7.4


From 4aa19c73763415c2cbf02325dfe7b2a99076f7ad Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 12 Dec 2018 14:50:53 +1300
Subject: [PATCH 19/19] tests: Run SMB Py bindings tests against testenv with
 SMBv1-disabled

Sanity-check that the SMBv2 connection actually works by running it
against a testenv with SMBv1 disabled.

I've dropped 'local' from the ad_dc target, because it shouldn't be
needed. We're trying to test the client-side SMB connection, so running
it without 'local' is probably a better test.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/selftest/tests.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 5218c83..dd3b0ae 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -689,7 +689,9 @@ planpythontestsuite("chgdcpass:local", "samba.tests.dcerpc.rpcecho", py3_compati
 planoldpythontestsuite("nt4_dc", "samba.tests.netbios", extra_args=['-U"$USERNAME%$PASSWORD"'], py3_compatible=True)
 planoldpythontestsuite("ad_dc:local", "samba.tests.gpo", extra_args=['-U"$USERNAME%$PASSWORD"'], py3_compatible=True)
 planoldpythontestsuite("ad_dc:local", "samba.tests.dckeytab", extra_args=['-U"$USERNAME%$PASSWORD"'], py3_compatible=True)
-planoldpythontestsuite("ad_dc:local", "samba.tests.smb", extra_args=['-U"$USERNAME%$PASSWORD"'], py3_compatible=True)
+
+for env in ["ad_dc", smbv1_disabled_testenv]:
+    planoldpythontestsuite(env, "samba.tests.smb", extra_args=['-U"$USERNAME%$PASSWORD"'], py3_compatible=True)
 
 planoldpythontestsuite(
     "ad_dc_ntvfs:local", "samba.tests.dcerpc.registry",
-- 
2.7.4



More information about the samba-technical mailing list