[PATCH] Initial/pre-work for consolidating SMB Python bindings
Tim Beale
timbeale at catalyst.net.nz
Thu Dec 6 04:34:10 UTC 2018
This is the first set of patches to rework the SMB Python bindings.
Mostly they are Metze's patches to rework the s3 bindings to handle
non-multithreaded, SMBv2 connections (thanks for all your help Metze!).
Plus a few patches to extend the existing (s4) SMB Python bindings tests
to cover more test cases.
I'm still working on the next step, which is porting the s4 APIs into
the s3 Python bindings. The WIP branch for this is here:
https://gitlab.com/catalyst-samba/samba/commits/tim-pysmb-2
Merge Request: https://gitlab.com/samba-team/samba/merge_requests/139
CI link: https://gitlab.com/catalyst-samba/samba/pipelines/39131037
Review appreciated. Let me know if you've got feedback - I'll address it
when I'm back in the office on Tuesday.
-------------- next part --------------
From b2a78fa56e57ef8bac6ce0f18605d09d1f462972 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 3 Dec 2018 14:37:05 +0100
Subject: [PATCH 01/12] s3:pylibsmb: pass self to py_tevent_req_wait_exc()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source3/libsmb/pylibsmb.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 99e5587..75236e7 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -364,7 +364,7 @@ static int py_tevent_req_wait(struct tevent_context *ev,
#endif
-static bool py_tevent_req_wait_exc(struct tevent_context *ev,
+static bool py_tevent_req_wait_exc(struct py_cli_state *self,
struct tevent_req *req)
{
int ret;
@@ -373,7 +373,7 @@ static bool py_tevent_req_wait_exc(struct tevent_context *ev,
PyErr_NoMemory();
return false;
}
- ret = py_tevent_req_wait(ev, req);
+ ret = py_tevent_req_wait(self->ev, req);
if (ret != 0) {
TALLOC_FREE(req);
errno = ret;
@@ -453,7 +453,7 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
req = cli_full_connection_creds_send(
NULL, self->ev, "myname", host, NULL, 0, share, "?????",
cli_creds, flags, SMB_SIGNING_DEFAULT);
- if (!py_tevent_req_wait_exc(self->ev, req)) {
+ if (!py_tevent_req_wait_exc(self, req)) {
return -1;
}
status = cli_full_connection_creds_recv(req, &self->cli);
@@ -612,7 +612,7 @@ static PyObject *py_cli_create(struct py_cli_state *self, PyObject *args,
DesiredAccess, FileAttributes, ShareAccess,
CreateDisposition, CreateOptions,
SecurityFlags);
- if (!py_tevent_req_wait_exc(self->ev, req)) {
+ if (!py_tevent_req_wait_exc(self, req)) {
return NULL;
}
status = cli_ntcreate_recv(req, &fnum, NULL);
@@ -636,7 +636,7 @@ static PyObject *py_cli_close(struct py_cli_state *self, PyObject *args)
}
req = cli_close_send(NULL, self->ev, self->cli, fnum);
- if (!py_tevent_req_wait_exc(self->ev, req)) {
+ if (!py_tevent_req_wait_exc(self, req)) {
return NULL;
}
status = cli_close_recv(req);
@@ -672,7 +672,7 @@ static PyObject *py_cli_write(struct py_cli_state *self, PyObject *args,
req = cli_write_andx_send(NULL, self->ev, self->cli, fnum, mode,
(uint8_t *)buf, offset, buflen);
- if (!py_tevent_req_wait_exc(self->ev, req)) {
+ if (!py_tevent_req_wait_exc(self, req)) {
return NULL;
}
status = cli_write_andx_recv(req, &written);
@@ -708,7 +708,7 @@ static PyObject *py_cli_read(struct py_cli_state *self, PyObject *args,
req = cli_read_andx_send(NULL, self->ev, self->cli, fnum,
offset, size);
- if (!py_tevent_req_wait_exc(self->ev, req)) {
+ if (!py_tevent_req_wait_exc(self, req)) {
return NULL;
}
status = cli_read_andx_recv(req, &buflen, &buf);
@@ -740,7 +740,7 @@ static PyObject *py_cli_ftruncate(struct py_cli_state *self, PyObject *args,
}
req = cli_ftruncate_send(NULL, self->ev, self->cli, fnum, size);
- if (!py_tevent_req_wait_exc(self->ev, req)) {
+ if (!py_tevent_req_wait_exc(self, req)) {
return NULL;
}
status = cli_ftruncate_recv(req);
@@ -771,7 +771,7 @@ static PyObject *py_cli_delete_on_close(struct py_cli_state *self,
req = cli_nt_delete_on_close_send(NULL, self->ev, self->cli, fnum,
flag);
- if (!py_tevent_req_wait_exc(self->ev, req)) {
+ if (!py_tevent_req_wait_exc(self, req)) {
return NULL;
}
status = cli_nt_delete_on_close_recv(req);
@@ -812,7 +812,7 @@ static PyObject *py_cli_list(struct py_cli_state *self,
req = cli_list_send(NULL, self->ev, self->cli, mask, attribute,
info_level);
- if (!py_tevent_req_wait_exc(self->ev, req)) {
+ if (!py_tevent_req_wait_exc(self, req)) {
return NULL;
}
status = cli_list_recv(req, NULL, &finfos, &num_finfos);
--
2.7.4
From bbdb430f137e983cec0591f7d9b1bc33682fd32e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 3 Dec 2018 15:02:06 +0100
Subject: [PATCH 02/12] s3:pylibsmb: only use poll_mt backend if
multi_threaded=True is specified
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
python/samba/tests/libsmb_samba_internal.py | 3 +-
source3/libsmb/pylibsmb.c | 52 +++++++++++++++++++++++------
2 files changed, 43 insertions(+), 12 deletions(-)
diff --git a/python/samba/tests/libsmb_samba_internal.py b/python/samba/tests/libsmb_samba_internal.py
index c88095c..db99c0b 100644
--- a/python/samba/tests/libsmb_samba_internal.py
+++ b/python/samba/tests/libsmb_samba_internal.py
@@ -59,7 +59,8 @@ class LibsmbTestCase(samba.tests.TestCase):
creds.set_username(os.getenv("USERNAME"))
creds.set_password(os.getenv("PASSWORD"))
- c = libsmb_samba_internal.Conn(os.getenv("SERVER_IP"), "tmp", creds)
+ c = libsmb_samba_internal.Conn(os.getenv("SERVER_IP"), "tmp",
+ creds, multi_threaded=True)
mythreads = []
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 75236e7..a18fbaf 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -81,6 +81,8 @@ struct py_cli_state {
PyObject_HEAD
struct cli_state *cli;
struct tevent_context *ev;
+ int (*req_wait_fn)(struct tevent_context *ev,
+ struct tevent_req *req);
struct py_cli_thread *thread_state;
struct tevent_req *oplock_waiter;
@@ -194,7 +196,10 @@ static int py_cli_thread_destructor(struct py_cli_thread *t)
return 0;
}
-static bool py_cli_state_setup_ev(struct py_cli_state *self)
+static int py_tevent_cond_req_wait(struct tevent_context *ev,
+ struct tevent_req *req);
+
+static bool py_cli_state_setup_mt_ev(struct py_cli_state *self)
{
struct py_cli_thread *t = NULL;
int ret;
@@ -206,6 +211,8 @@ static bool py_cli_state_setup_ev(struct py_cli_state *self)
samba_tevent_set_debug(self->ev, "pylibsmb_tevent_mt");
tevent_set_trace_callback(self->ev, py_cli_state_trace_callback, self);
+ self->req_wait_fn = py_tevent_cond_req_wait;
+
self->thread_state = talloc_zero(NULL, struct py_cli_thread);
if (self->thread_state == NULL) {
goto fail;
@@ -303,8 +310,8 @@ fail:
return result;
}
-static int py_tevent_req_wait(struct tevent_context *ev,
- struct tevent_req *req)
+static int py_tevent_cond_req_wait(struct tevent_context *ev,
+ struct tevent_req *req)
{
struct py_tevent_cond cond;
tevent_req_set_callback(req, py_tevent_signalme, &cond);
@@ -334,7 +341,10 @@ static void py_tevent_signalme(struct tevent_req *req)
py_tevent_cond_signal(cond);
}
-#else
+#endif
+
+static int py_tevent_req_wait(struct tevent_context *ev,
+ struct tevent_req *req);
static bool py_cli_state_setup_ev(struct py_cli_state *self)
{
@@ -345,6 +355,8 @@ static bool py_cli_state_setup_ev(struct py_cli_state *self)
samba_tevent_set_debug(self->ev, "pylibsmb_tevent");
+ self->req_wait_fn = py_tevent_req_wait;
+
return true;
}
@@ -362,8 +374,6 @@ static int py_tevent_req_wait(struct tevent_context *ev,
return 0;
}
-#endif
-
static bool py_tevent_req_wait_exc(struct py_cli_state *self,
struct tevent_req *req)
{
@@ -373,7 +383,7 @@ static bool py_tevent_req_wait_exc(struct py_cli_state *self,
PyErr_NoMemory();
return false;
}
- ret = py_tevent_req_wait(self->ev, req);
+ ret = self->req_wait_fn(self->ev, req);
if (ret != 0) {
TALLOC_FREE(req);
errno = ret;
@@ -410,6 +420,8 @@ 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_multi_threaded = Py_False;
+ bool multi_threaded = false;
struct tevent_req *req;
bool ret;
/*
@@ -421,7 +433,7 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
int flags = CLI_FULL_CONNECTION_FORCE_SMB1;
static const char *kwlist[] = {
- "host", "share", "credentials", NULL
+ "host", "share", "credentials", "multi_threaded", NULL
};
PyTypeObject *py_type_Credentials = get_pytype(
@@ -431,8 +443,10 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
}
ret = ParseTupleAndKeywords(
- args, kwds, "ss|O!", kwlist,
- &host, &share, py_type_Credentials, &creds);
+ args, kwds, "ss|O!O", kwlist,
+ &host, &share,
+ py_type_Credentials, &creds,
+ &py_multi_threaded);
Py_DECREF(py_type_Credentials);
@@ -440,8 +454,24 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
return -1;
}
- if (!py_cli_state_setup_ev(self)) {
+ multi_threaded = PyObject_IsTrue(py_multi_threaded);
+
+ if (multi_threaded) {
+#ifdef HAVE_PTHREAD
+ ret = py_cli_state_setup_mt_ev(self);
+ if (!ret) {
+ return -1;
+ }
+#else
+ PyErr_SetString(PyExc_RuntimeError,
+ "No PTHREAD support available");
return -1;
+#endif
+ } else {
+ ret = py_cli_state_setup_ev(self);
+ if (!ret) {
+ return -1;
+ }
}
if (creds == NULL) {
--
2.7.4
From 80eb1235bb6bd4cfb41d052ec0024099cee6dd58 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 3 Dec 2018 15:42:50 +0100
Subject: [PATCH 03/12] s3:pylibsmb: add sign=True to require signing
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source3/libsmb/pylibsmb.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index a18fbaf..8e63cbf 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -422,6 +422,9 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
struct cli_credentials *cli_creds;
PyObject *py_multi_threaded = Py_False;
bool multi_threaded = false;
+ PyObject *py_sign = Py_False;
+ bool sign = false;
+ int signing_state = SMB_SIGNING_DEFAULT;
struct tevent_req *req;
bool ret;
/*
@@ -433,7 +436,8 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
int flags = CLI_FULL_CONNECTION_FORCE_SMB1;
static const char *kwlist[] = {
- "host", "share", "credentials", "multi_threaded", NULL
+ "host", "share", "credentials",
+ "multi_threaded", "sign", NULL
};
PyTypeObject *py_type_Credentials = get_pytype(
@@ -443,10 +447,11 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
}
ret = ParseTupleAndKeywords(
- args, kwds, "ss|O!O", kwlist,
+ args, kwds, "ss|O!OO", kwlist,
&host, &share,
py_type_Credentials, &creds,
- &py_multi_threaded);
+ &py_multi_threaded,
+ &py_sign);
Py_DECREF(py_type_Credentials);
@@ -455,6 +460,11 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
}
multi_threaded = PyObject_IsTrue(py_multi_threaded);
+ sign = PyObject_IsTrue(py_sign);
+
+ if (sign) {
+ signing_state = SMB_SIGNING_REQUIRED;
+ }
if (multi_threaded) {
#ifdef HAVE_PTHREAD
@@ -482,7 +492,7 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
req = cli_full_connection_creds_send(
NULL, self->ev, "myname", host, NULL, 0, share, "?????",
- cli_creds, flags, SMB_SIGNING_DEFAULT);
+ cli_creds, flags, signing_state);
if (!py_tevent_req_wait_exc(self, req)) {
return -1;
}
--
2.7.4
From 07c47cab78b8346bc7cc566f16abf858c894b5c5 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 4 Dec 2018 10:40:18 +0100
Subject: [PATCH 04/12] s3:pylibsmb: add force_smb1=True in order to control
forcing of SMB1
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
python/samba/tests/libsmb_samba_internal.py | 3 ++-
source3/libsmb/pylibsmb.c | 35 ++++++++++++++++++++---------
2 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/python/samba/tests/libsmb_samba_internal.py b/python/samba/tests/libsmb_samba_internal.py
index db99c0b..8918d84 100644
--- a/python/samba/tests/libsmb_samba_internal.py
+++ b/python/samba/tests/libsmb_samba_internal.py
@@ -60,7 +60,8 @@ 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)
+ creds, multi_threaded=True,
+ force_smb1=True)
mythreads = []
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 8e63cbf..ed3c041 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -425,19 +425,16 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
PyObject *py_sign = Py_False;
bool sign = false;
int signing_state = SMB_SIGNING_DEFAULT;
+ PyObject *py_force_smb1 = Py_False;
+ bool force_smb1 = false;
struct tevent_req *req;
bool ret;
- /*
- * For now we only support SMB1,
- * as most of the cli_*_send() function
- * don't support SMB2, it's only plugged
- * into the sync wrapper functions currently.
- */
- int flags = CLI_FULL_CONNECTION_FORCE_SMB1;
+ int flags = 0;
static const char *kwlist[] = {
"host", "share", "credentials",
- "multi_threaded", "sign", NULL
+ "multi_threaded", "sign", "force_smb1",
+ NULL
};
PyTypeObject *py_type_Credentials = get_pytype(
@@ -447,11 +444,12 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
}
ret = ParseTupleAndKeywords(
- args, kwds, "ss|O!OO", kwlist,
+ args, kwds, "ss|O!OOO", kwlist,
&host, &share,
py_type_Credentials, &creds,
&py_multi_threaded,
- &py_sign);
+ &py_sign,
+ &py_force_smb1);
Py_DECREF(py_type_Credentials);
@@ -461,11 +459,22 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
multi_threaded = PyObject_IsTrue(py_multi_threaded);
sign = PyObject_IsTrue(py_sign);
+ force_smb1 = PyObject_IsTrue(py_force_smb1);
if (sign) {
signing_state = SMB_SIGNING_REQUIRED;
}
+ if (force_smb1) {
+ /*
+ * As most of the cli_*_send() function
+ * don't support SMB2 (it's only plugged
+ * into the sync wrapper functions currently)
+ * we have a way to force SMB1.
+ */
+ flags = CLI_FULL_CONNECTION_FORCE_SMB1;
+ }
+
if (multi_threaded) {
#ifdef HAVE_PTHREAD
ret = py_cli_state_setup_mt_ev(self);
@@ -477,6 +486,12 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
"No PTHREAD support available");
return -1;
#endif
+ if (!force_smb1) {
+ PyErr_SetString(PyExc_RuntimeError,
+ "multi_threaded is only possible on "
+ "SMB1 connections");
+ return -1;
+ }
} else {
ret = py_cli_state_setup_ev(self);
if (!ret) {
--
2.7.4
From a679d1d60b3a12be930d3919b6070825899121fb Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 4 Dec 2018 10:42:55 +0100
Subject: [PATCH 05/12] s3:pylibsmb: remember that a connection uses SMB1
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source3/libsmb/pylibsmb.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index ed3c041..e4552a2 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -21,6 +21,7 @@
#include <Python.h>
#include "includes.h"
#include "python/py3compat.h"
+#include "libcli/smb/smbXcli_base.h"
#include "libsmb/libsmb.h"
#include "libcli/security/security.h"
#include "system/select.h"
@@ -80,6 +81,7 @@ struct py_cli_oplock_break {
struct py_cli_state {
PyObject_HEAD
struct cli_state *cli;
+ bool is_smb1;
struct tevent_context *ev;
int (*req_wait_fn)(struct tevent_context *ev,
struct tevent_req *req);
@@ -403,6 +405,7 @@ static PyObject *py_cli_state_new(PyTypeObject *type, PyObject *args,
return NULL;
}
self->cli = NULL;
+ self->is_smb1 = false;
self->ev = NULL;
self->thread_state = NULL;
self->oplock_waiter = NULL;
@@ -519,6 +522,10 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
return -1;
}
+ if (smbXcli_conn_protocol(self->cli->conn) < PROTOCOL_SMB2_02) {
+ self->is_smb1 = true;
+ }
+
self->oplock_waiter = cli_smb_oplock_break_waiter_send(
self->ev, self->ev, self->cli);
if (self->oplock_waiter == NULL) {
--
2.7.4
From 7b9ff8c44bf0eb4e81caa46fb664f9ce39c984c7 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 4 Dec 2018 12:32:58 +1300
Subject: [PATCH 06/12] s3:pylibsmb: .get_oplock_break API is dependent on
multi_threaded=True
The .get_oplock_break is dependent on the pthread code, which is only
used when creating a SMB connection with multi_threaded=True.
Add an explicit error to the .get_oplock_break() if someone tries to use
it in non-multithreaded mode.
Initializing self->oplock_waiter in non-multithreaded mode is similarly
redundant if the API can never be used.
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>
---
source3/libsmb/pylibsmb.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index e4552a2..19587a8 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -526,6 +526,13 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
self->is_smb1 = true;
}
+ /*
+ * Oplocks require a multi threaded connection
+ */
+ if (self->thread_state == NULL) {
+ return 0;
+ }
+
self->oplock_waiter = cli_smb_oplock_break_waiter_send(
self->ev, self->ev, self->cli);
if (self->oplock_waiter == NULL) {
@@ -585,6 +592,13 @@ static PyObject *py_cli_get_oplock_break(struct py_cli_state *self,
return NULL;
}
+ if (self->thread_state == NULL) {
+ PyErr_SetString(PyExc_RuntimeError,
+ "get_oplock_break() only possible on "
+ "a multi_threaded connection");
+ return NULL;
+ }
+
if (self->oplock_cond != NULL) {
errno = EBUSY;
PyErr_SetFromErrno(PyExc_RuntimeError);
--
2.7.4
From 1b09a33c1f7fd429d7faf2497a7cb6f6e651049a Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 3 Dec 2018 11:01:14 +1300
Subject: [PATCH 07/12] tests: Add SMB Py binding .chkpath() test case
chkpath was only tested incidentally (and that assertion was wrong). Add
a proper test to prove it works correctly. We can then clean-up the
incorrect assertion in the next patch.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
python/samba/tests/smb.py | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index e0e60e3..6f33ff1 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -65,6 +65,26 @@ class SMBTests(samba.tests.TestCase):
self.conn.unlink(test_file)
self.assertFalse(self.conn.chkpath(test_file))
+ def test_chkpath(self):
+ """Tests .chkpath determines whether or not a directory exists"""
+
+ self.assertTrue(self.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))
+
+ # should return False for files (because they're not directories)
+ self.conn.savefile(test_file, binary_contents)
+ self.assertFalse(self.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.conn.mkdir(new_dir)
+ self.assertTrue(self.conn.chkpath(new_dir))
+ self.conn.rmdir(new_dir)
+ self.assertFalse(self.conn.chkpath(new_dir))
+
def test_save_load_text(self):
self.conn.savefile(test_file, test_contents.encode('utf8'))
@@ -99,3 +119,7 @@ class SMBTests(samba.tests.TestCase):
contents = self.conn.loadfile(test_file)
self.assertEquals(contents, binary_contents,
msg='contents of test file did not match what was written')
+
+ def make_sysvol_path(self, dirpath, filename):
+ # return the dir + filename as a sysvol path
+ return os.path.join(dirpath, filename).replace('/', '\\')
--
2.7.4
From 6ec0ed5ae9a75d0c49f34ef78223537bd5baf178 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 3 Dec 2018 11:15:14 +1300
Subject: [PATCH 08/12] tests: Fix SMB Py binding .unlink() test case assertion
The current assertion would never detect if the unlink API is broken.
The chkpath() API is only useful for checking if directories exist, so
it will always return False for a regular file (regardless of whether
the file actually exists or not).
Rework the test case so we assert that the file exists by trying to read
its contents (which will throw an error if the file doesn't exist).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
python/samba/tests/smb.py | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index 6f33ff1..e4366ce 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -20,6 +20,8 @@ import os
import random
import sys
from samba import smb
+from samba import NTSTATUSError
+from samba.ntstatus import NT_STATUS_OBJECT_NAME_NOT_FOUND
PY3 = sys.version_info[0] == 3
addom = 'addom.samba.example.com/'
@@ -57,13 +59,30 @@ class SMBTests(samba.tests.TestCase):
self.assertIn('Policies', ls,
msg='"Policies" directory not found in sysvol')
+ def file_exists(self, filepath):
+ """Returns whether a regular file exists (by trying to open it)"""
+ try:
+ self.conn.loadfile(filepath)
+ exists = True;
+ except NTSTATUSError as err:
+ if err.args[0] == NT_STATUS_OBJECT_NAME_NOT_FOUND:
+ exists = False
+ else:
+ raise err
+ return exists
+
def test_unlink(self):
"""
The smb.unlink API should delete file
"""
+ # create the test file
+ self.assertFalse(self.file_exists(test_file))
self.conn.savefile(test_file, binary_contents)
+ self.assertTrue(self.file_exists(test_file))
+
+ # delete it and check that it's gone
self.conn.unlink(test_file)
- self.assertFalse(self.conn.chkpath(test_file))
+ self.assertFalse(self.file_exists(test_file))
def test_chkpath(self):
"""Tests .chkpath determines whether or not a directory exists"""
--
2.7.4
From a6a373be2f5879d9883d13ff663f66c1188df702 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 3 Dec 2018 17:22:43 +1300
Subject: [PATCH 09/12] tests: Extend SMB Py binding .list() test-case
Extend the tests to better reflect some of the .list() functionality we
expect.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
python/samba/tests/smb.py | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index e4366ce..70d6cd8 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -53,11 +53,33 @@ class SMBTests(samba.tests.TestCase):
pass
def test_list(self):
+ # check a basic listing returns the items we expect
ls = [f['name'] for f in self.conn.list(addom)]
self.assertIn('scripts', ls,
msg='"scripts" directory not found in sysvol')
self.assertIn('Policies', ls,
msg='"Policies" directory not found in sysvol')
+ self.assertNotIn('..', ls,
+ msg='Parent (..) found in directory listing')
+ self.assertNotIn('.', ls,
+ 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, "*")]
+ 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*")]
+ 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.conn.list(addom):
+ for key in expected_keys:
+ self.assertIn(key, item,
+ msg="Key '%s' not in listing '%s'" % (key, item))
+
def file_exists(self, filepath):
"""Returns whether a regular file exists (by trying to open it)"""
--
2.7.4
From 4e46f6d3121f488a588eb9c3185ec0c4fb399ba6 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 5 Dec 2018 12:51:22 +1300
Subject: [PATCH 10/12] tests: Extend SMB test_save_load_text case to check
overwrite
Extend the test case to check overwriting a file as well. Currently this
has the behaviour of appending to the existing file, rather than
overwriting the file with new contents.
It's not clear from the API that this is the intended behaviour in this
case, so I've marked it as a failure.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
python/samba/tests/smb.py | 7 +++++++
selftest/knownfail.d/smb | 3 +++
2 files changed, 10 insertions(+)
create mode 100644 selftest/knownfail.d/smb
diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index 70d6cd8..e3af777 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -134,6 +134,13 @@ class SMBTests(samba.tests.TestCase):
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.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')
+
# 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):
diff --git a/selftest/knownfail.d/smb b/selftest/knownfail.d/smb
new file mode 100644
index 0000000..5e55ab6
--- /dev/null
+++ b/selftest/knownfail.d/smb
@@ -0,0 +1,3 @@
+# currently savefile appends rather than overwriting
+samba.tests.smb.samba.tests.smb.SMBTests.test_save_load_text\(ad_dc:local\)
+samba.tests.smb.python3.samba.tests.smb.SMBTests.test_save_load_text\(ad_dc:local\)
--
2.7.4
From d5a7ce7ff784e8e6f99453e8d41bfe84e02cbcdd Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 6 Dec 2018 16:03:23 +1300
Subject: [PATCH 11/12] s4:libcli: Fix error in smbcli_deltree()
Commit 094afe614b6282 fixed an uninitialized variable, which meant we
tried to delete the file twice. The 2nd time fails, so the function
returns an error, instead of success (even though the file is now gone).
Note we want to be using the source3 SMB library code going forward.
However, fixing this bug makes it easier to write tests against the
(currently s4) SMB python bindings.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/libcli/clideltree.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source4/libcli/clideltree.c b/source4/libcli/clideltree.c
index 7bce95c..e8007f4 100644
--- a/source4/libcli/clideltree.c
+++ b/source4/libcli/clideltree.c
@@ -99,7 +99,7 @@ int smbcli_deltree(struct smbcli_tree *tree, const char *dname)
/* it might be a file */
status = smbcli_unlink(tree, dname);
- if (NT_STATUS_IS_OK(smbcli_unlink(tree, dname))) {
+ if (NT_STATUS_IS_OK(status)) {
return 1;
}
if (NT_STATUS_EQUAL(smbcli_nt_error(tree), NT_STATUS_OBJECT_NAME_NOT_FOUND) ||
--
2.7.4
From eaa39b90766baec9dc6664bc3e7bf90953517b44 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 6 Dec 2018 16:16:36 +1300
Subject: [PATCH 12/12] tests: Add SMB Py binding .deltree test case
Add a more thorough test case that .deltree works as expected.
Note that we get a slightly different NT_STATUS error in file_exists()
if the parent directory doesn't exist, e.g.
/non-existent-dir/nonexistent.txt
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
python/samba/tests/smb.py | 62 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 60 insertions(+), 2 deletions(-)
diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index e3af777..6893647 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -21,7 +21,8 @@ import random
import sys
from samba import smb
from samba import NTSTATUSError
-from samba.ntstatus import NT_STATUS_OBJECT_NAME_NOT_FOUND
+from samba.ntstatus import (NT_STATUS_OBJECT_NAME_NOT_FOUND,
+ NT_STATUS_OBJECT_PATH_NOT_FOUND)
PY3 = sys.version_info[0] == 3
addom = 'addom.samba.example.com/'
@@ -80,6 +81,62 @@ class SMBTests(samba.tests.TestCase):
self.assertIn(key, item,
msg="Key '%s' not in listing '%s'" % (key, item))
+ def test_deltree(self):
+ """The smb.deltree API should delete files and sub-dirs"""
+ # create some test sub-dirs
+ 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.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)
+ empty_dirs.append(path)
+
+ # create some files in these directories
+ filepaths = []
+ for subdir in dirpaths:
+ 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'))
+ filepaths.append(path)
+
+ # sanity-check these dirs/files exist
+ for subdir in dirpaths + empty_dirs:
+ self.assertTrue(self.conn.chkpath(subdir),
+ "Failed to create {0}".format(subdir))
+ for path in filepaths:
+ self.assertTrue(self.file_exists(path),
+ "Failed to create {0}".format(path))
+
+ # try using deltree to remove a single empty directory
+ path = empty_dirs.pop(0)
+ self.conn.deltree(path)
+ self.assertFalse(self.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.assertFalse(self.file_exists(path),
+ "Failed to delete {0}".format(path))
+
+ # delete the top-level dir
+ self.conn.deltree(test_dir)
+
+ # now check that all the dirs/files are no longer there
+ for subdir in dirpaths + empty_dirs:
+ self.assertFalse(self.conn.chkpath(subdir),
+ "Failed to delete {0}".format(subdir))
+ for path in filepaths:
+ self.assertFalse(self.file_exists(path),
+ "Failed to delete {0}".format(path))
def file_exists(self, filepath):
"""Returns whether a regular file exists (by trying to open it)"""
@@ -87,7 +144,8 @@ class SMBTests(samba.tests.TestCase):
self.conn.loadfile(filepath)
exists = True;
except NTSTATUSError as err:
- if err.args[0] == NT_STATUS_OBJECT_NAME_NOT_FOUND:
+ if (err.args[0] == NT_STATUS_OBJECT_NAME_NOT_FOUND or
+ err.args[0] == NT_STATUS_OBJECT_PATH_NOT_FOUND):
exists = False
else:
raise err
--
2.7.4
More information about the samba-technical
mailing list