[RFC] Advice on SMB client python bindings?
Tim Beale
timbeale at catalyst.net.nz
Tue Dec 4 04:28:47 UTC 2018
Hi,
Thanks for the patches Metze. Unfortunately that doesn't help me with
supporting SMBv2 at all. Attached is an example test-case
(smbv2-problem.txt) that tries to run the .unlink() API against a DC
with SMBv1 disabled.
Taking a step back, the problem I'm trying to solve is this:
A). Samba needs a set of python bindings that support both SMBv1 and
SMBv2. We need this in order to use _existing_ samba-tool commands on a
DC with SMBv1 disabled (and disabling SMBv1 is now recommended best
practice). Ideally, I'd like to get this fix into samba v4.10.
I'm trying to do this by consolidating the existing source3 and source4
python bindings, which I think on its own would be a small, but useful,
improvement to the Samba codebase.
The feedback I'm hearing is to use the async APIs, which involves:
B). Reworking the libsmb client library code so that the async APIs
support SMBv2.
Unfortunately, I don't have the time, skills, or knowledge to do that.
I'm not saying it's not a worthwhile goal, I just don't have the time to
do it, especially not before 4.10. I think that work is non-trivial (for
me, at least). If it were trivial, I think someone would have already
implemented it.
However, I don't think A is dependent on B. We can implement A now, and
B later (potentially piecemeal, one API at a time).
Attached is a possible compromise (compromise-patch.txt) where we use
either the async or sync client API depending on whether the SMB
connection is using multi-threaded support. It's not ideal, as it makes
the python bindings code more complicated than it needs to be. But it
should make it simple to switch over the implementation to solely use
the async APIs in future (i.e. just delete the synchronous 'else' case).
If you guys have a better idea, I'm open to hearing it. But I'm running
out of time for v4.10, and unless we can reach some sort of agreement, I
may have to abandon this approach altogether.
Finally, I don't really understand why using the async APIs in the
Python bindings is so important. For example, smbclient uses the
synchronous APIs. If someone could explain the rationale behind all
this, it might help. Or to put it another way, why is using the async
APIs more important than delivering software that fixes a real problem
for real Samba users?
Thanks,
Tim
On 4/12/18 7:41 AM, Stefan Metzmacher wrote:
> Hi Tim,
>> OK, just to re-cap, would you be happy with the following?
>> - I add an optional async=True|False parameter to
>> libsmb_samba_internal.Conn(). Default to async=False. This changes the
>> py_cli_state_init() behaviour (mostly around self->oplock_waiter).
>> - Where possible/simple, we use the async libsmb APIs so they work in
>> either case.
>> - Where that's not so simple, we just use the synchronous API, i.e. more
>> work would be needed in future if we ever wanted to support that API in
>> the async=True case.
>>
>> A bunch of the libsmb APIs (cli_unlink(), cli_rmdir(), cli_mkdir(),
>> cli_chkpath(), cli_query_security_descriptor()) seem to do the
>> SMBv1/SMBv2 check in the synchronous API rather than the asynchronous
>> API. And it looks to me like trying to add SMBv2 support to the async
>> API would not be trivial.
>>
>> The attached patch-set (#3 & #4) is an example of what I mean.
> I think we should keep the async versions.
>
> Can you start with the attached patches?
> This is is enough in order to implement my dcerpc over ncacn_np
> testcases.
>
> Thanks!
> metze
-------------- next part --------------
From e35a04c896c1e1600b9484072141c67e7dcb7062 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/17] s3:pylibsmb: pass self to py_tevent_req_wait_exc()
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 226cce1912c71440cff06978ddacc85996e755ac 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/17] s3:pylibsmb: only use poll_mt backend if
multi_threaded=True is specified
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 6c3a97c7123dbb189d75bbb64d7b69ddec927848 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/17] s3:pylibsmb: add sign=True to require signing
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 71e908a887c761a4a8a9c049422310c7c436bf73 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 04/17] 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.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source3/libsmb/pylibsmb.c | 33 +++++++++++++++++++++++----------
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 8e63cbf..f0bf308 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -88,6 +88,7 @@ struct py_cli_state {
struct tevent_req *oplock_waiter;
struct py_cli_oplock_break *oplock_breaks;
struct py_tevent_cond *oplock_cond;
+ bool multi_threaded;
};
#ifdef HAVE_PTHREAD
@@ -408,6 +409,7 @@ static PyObject *py_cli_state_new(PyTypeObject *type, PyObject *args,
self->oplock_waiter = NULL;
self->oplock_cond = NULL;
self->oplock_breaks = NULL;
+ self->multi_threaded = false;
return (PyObject *)self;
}
@@ -421,7 +423,6 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
PyObject *creds = NULL;
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;
@@ -459,14 +460,14 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
return -1;
}
- multi_threaded = PyObject_IsTrue(py_multi_threaded);
+ self->multi_threaded = PyObject_IsTrue(py_multi_threaded);
sign = PyObject_IsTrue(py_sign);
if (sign) {
signing_state = SMB_SIGNING_REQUIRED;
}
- if (multi_threaded) {
+ if (self->multi_threaded) {
#ifdef HAVE_PTHREAD
ret = py_cli_state_setup_mt_ev(self);
if (!ret) {
@@ -504,14 +505,20 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
return -1;
}
- self->oplock_waiter = cli_smb_oplock_break_waiter_send(
- self->ev, self->ev, self->cli);
- if (self->oplock_waiter == NULL) {
- PyErr_NoMemory();
- return -1;
+ /*
+ * Setup needed for .get_oplock_break() (which relies on being
+ * multi-threaded to work).
+ */
+ if (self->multi_threaded) {
+ self->oplock_waiter = cli_smb_oplock_break_waiter_send(
+ self->ev, self->ev, self->cli);
+ if (self->oplock_waiter == NULL) {
+ PyErr_NoMemory();
+ return -1;
+ }
+ tevent_req_set_callback(self->oplock_waiter, py_cli_got_oplock_break,
+ self);
}
- tevent_req_set_callback(self->oplock_waiter, py_cli_got_oplock_break,
- self);
return 0;
}
@@ -559,6 +566,12 @@ static PyObject *py_cli_get_oplock_break(struct py_cli_state *self,
{
size_t num_oplock_breaks;
+ if (!self->multi_threaded) {
+ DBG_ERR("SMB connection not setup with multi_threaded=True");
+ PyErr_SetNTSTATUS(NT_STATUS_INVALID_PARAMETER);
+ return NULL;
+ }
+
if (!PyArg_ParseTuple(args, "")) {
return NULL;
}
--
2.7.4
From 6e25a69dc28928a30f3381073d3f816c51b35639 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 3 Dec 2018 15:44:43 +0100
Subject: [PATCH 05/17] python/tests: also initialize the s3 loadparm subsystem
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
python/samba/tests/__init__.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/python/samba/tests/__init__.py b/python/samba/tests/__init__.py
index 9b30d2e..6ae8fbd 100644
--- a/python/samba/tests/__init__.py
+++ b/python/samba/tests/__init__.py
@@ -24,6 +24,7 @@ import warnings
import ldb
import samba
from samba import param
+from samba.samba3 import param as s3param
from samba import credentials
from samba.credentials import Credentials
from samba import gensec
@@ -302,8 +303,10 @@ class TestCaseInTempDir(TestCase):
def env_loadparm():
lp = param.LoadParm()
+ lp3 = s3param.get_context()
try:
lp.load(os.environ["SMB_CONF_PATH"])
+ lp3.load(lp.configfile)
except KeyError:
raise KeyError("SMB_CONF_PATH not set")
return lp
--
2.7.4
From 1aa455932ecc89ca7406901e86b361423c11cd71 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 06/17] tests: Add SMB 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.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
python/samba/tests/smb.py | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index e0e60e3..de2ecfc 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 = os.path.join(test_dir, 'dont_exist').replace('/', '\\')
+ 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 = os.path.join(test_dir, 'test-new').replace('/', '\\')
+ 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'))
--
2.7.4
From 9cf77e7b9045165c48023d4eac9dbee64418bf4e 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 07/17] tests: Fix SMB 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).
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 de2ecfc..6ee5eaa 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 1757c26a17d8355b6c1d6aa1ae27758e25f02cb1 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 4 Dec 2018 14:01:33 +1300
Subject: [PATCH 08/17] s3:pylibsmb: Support SMBv2 for non-multithreaded
connections
Allow the SMB python bindings to negotiate an SMBv2 connection in
non-multi-threaded mode.
Unfortunately the async client library APIs don't support SMBv2
connections currently.
In summary, you could have:
- multithreaded (async) and SMBv1, or
- non-multithreaded (synchronous) and SMBv2 (or SMBv1)
- but not async and SMBv2
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source3/libsmb/pylibsmb.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index f0bf308..961c6f9 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -428,13 +428,7 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
int signing_state = SMB_SIGNING_DEFAULT;
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",
@@ -468,6 +462,14 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
}
if (self->multi_threaded) {
+
+ /*
+ * For now we only support SMB1, if multi-threaded,
+ * as most of the cli_*_send() function
+ * don't support SMB2, it's only plugged
+ * into the sync wrapper functions currently.
+ */
+ flags = CLI_FULL_CONNECTION_FORCE_SMB1;
#ifdef HAVE_PTHREAD
ret = py_cli_state_setup_mt_ev(self);
if (!ret) {
--
2.7.4
From b4cff2d1df77fb4556047d80d1b68f82bdc77027 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 09/17] s3:pylibsmb: Add .unlink API
Add a basic .unlink() API to the source3 bindings. This is based on the
source4 python bindings, but uses the source3 client library APIs.
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>
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
python/samba/tests/smb.py | 7 ++++++-
source3/libsmb/pylibsmb.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index 6ee5eaa..91b487d 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -22,6 +22,7 @@ import sys
from samba import smb
from samba import NTSTATUSError
from samba.ntstatus import NT_STATUS_OBJECT_NAME_NOT_FOUND
+from samba.samba3 import libsmb_samba_internal
PY3 = sys.version_info[0] == 3
addom = 'addom.samba.example.com/'
@@ -43,6 +44,10 @@ class SMBTests(samba.tests.TestCase):
"sysvol",
lp=self.get_loadparm(),
creds=creds)
+
+ # temporarily create a 2nd SMB connection for migrating the py-bindings
+ self.smb_conn = libsmb_samba_internal.Conn(self.server, "sysvol", creds)
+
self.conn.mkdir(test_dir)
def tearDown(self):
@@ -81,7 +86,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 961c6f9..d889534 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -907,6 +907,35 @@ static PyObject *py_cli_list(struct py_cli_state *self,
return result;
}
+static PyObject *py_smb_unlink(struct py_cli_state *self, PyObject *args)
+{
+ NTSTATUS status;
+ const char *filename = NULL;
+ uint16_t attrs = (FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN);
+
+ if (!PyArg_ParseTuple(args, "s:unlink", &filename)) {
+ return NULL;
+ }
+
+ if (self->multi_threaded) {
+ 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 NULL;
+ }
+ status = cli_unlink_recv(req);
+ TALLOC_FREE(req);
+ } else {
+ status = cli_unlink(self->cli, filename, attrs);
+ }
+
+ PyErr_NTSTATUS_NOT_OK_RAISE(status);
+
+ Py_RETURN_NONE;
+}
+
static PyMethodDef py_cli_state_methods[] = {
{ "create", (PyCFunction)py_cli_create, METH_VARARGS|METH_KEYWORDS,
"Open a file" },
@@ -927,6 +956,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 06ec2d6eb23e9fdc96c11d0f3747694e9adeb5c0 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 10/17] 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.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
python/samba/tests/smb.py | 6 ++---
source3/libsmb/pylibsmb.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index 91b487d..378ed33 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -48,7 +48,7 @@ class SMBTests(samba.tests.TestCase):
# temporarily create a 2nd SMB connection for migrating the py-bindings
self.smb_conn = libsmb_samba_internal.Conn(self.server, "sysvol", creds)
- self.conn.mkdir(test_dir)
+ self.smb_conn.mkdir(test_dir)
def tearDown(self):
super(SMBTests, self).tearDown()
@@ -104,9 +104,9 @@ class SMBTests(samba.tests.TestCase):
# check correct result after creating and then deleting a new dir
new_dir = os.path.join(test_dir, 'test-new').replace('/', '\\')
- 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 d889534..48501bd 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -936,6 +936,64 @@ static PyObject *py_smb_unlink(struct py_cli_state *self, PyObject *args)
Py_RETURN_NONE;
}
+/*
+ * Delete an empty directory
+ */
+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;
+ }
+
+ if (self->multi_threaded) {
+ struct tevent_req *req = NULL;
+
+ req = cli_rmdir_send(NULL, self->ev, self->cli, dirname);
+ if (!py_tevent_req_wait_exc(self, req)) {
+ return NULL;
+ }
+ status = cli_rmdir_recv(req);
+ TALLOC_FREE(req);
+ } else {
+ status = cli_rmdir(self->cli, 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->multi_threaded) {
+ 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[] = {
{ "create", (PyCFunction)py_cli_create, METH_VARARGS|METH_KEYWORDS,
"Open a file" },
@@ -959,6 +1017,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 5bee1d4121720d203e53c1c4e371ca55d3649480 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 11/17] s3:pylibsmb: Add .chkpath() API to SMB py bindings
Note that this is checking the existence of *directories*, not *files*.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
python/samba/tests/smb.py | 10 +++++-----
source3/libsmb/pylibsmb.c | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index 378ed33..188ff12 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -92,22 +92,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 = os.path.join(test_dir, 'dont_exist').replace('/', '\\')
- 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 = os.path.join(test_dir, 'test-new').replace('/', '\\')
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 48501bd..7e51b2a 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -994,6 +994,38 @@ static PyObject *py_smb_mkdir(struct py_cli_state *self, PyObject *args)
Py_RETURN_NONE;
}
+/*
+ * Checks existence of a directory
+ */
+static PyObject *py_smb_chkpath(struct py_cli_state *self, PyObject *args)
+{
+ NTSTATUS status;
+ const char *path;
+
+ if (!PyArg_ParseTuple(args, "s:chkpath", &path)) {
+ return NULL;
+ }
+
+ if (self->multi_threaded) {
+ struct tevent_req *req = NULL;
+
+ req = cli_chkpath_send(NULL, self->ev, self->cli, path);
+ if (!py_tevent_req_wait_exc(self, req)) {
+ return NULL;
+ }
+ status = cli_chkpath_recv(req);
+ TALLOC_FREE(req);
+ } else {
+ status = cli_chkpath(self->cli, path);
+ }
+
+ if (NT_STATUS_IS_OK(status)) {
+ Py_RETURN_TRUE;
+ }
+
+ Py_RETURN_FALSE;
+}
+
static PyMethodDef py_cli_state_methods[] = {
{ "create", (PyCFunction)py_cli_create, METH_VARARGS|METH_KEYWORDS,
"Open a file" },
@@ -1021,6 +1053,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 \
+ Return true if directory exists, false otherwise." },
{ NULL, NULL, 0, NULL }
};
--
2.7.4
From bfe2c6a62be3bba4af1ee7db75fd47ad9275a1e3 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 12/17] tests: Extend SMB py binding .list() test-case
Extend the tests to better reflect some of the .list() functionality we
expect.
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 188ff12..c095ad5 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -58,11 +58,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 61182cfc8479ccd67eaa7f4afdc789b7b0513238 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 13/17] s3:pylibsmb: Consolidate .readdir()/.list() 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.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
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 7e51b2a..6bfe328 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -844,6 +844,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 |
@@ -855,16 +857,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)) {
@@ -872,6 +882,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);
@@ -1041,9 +1052,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 fa5937689456e7287c708c980ed09f58f91da60b 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 14/17] s3:pylibsmb: Split out code into list_helper()
Refactor out the code so we can re-use it for the synchronous API as
well.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
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 6bfe328..e0eeb6e 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -839,6 +839,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)
@@ -895,21 +920,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 eaadfdfb1e07914f2375095bf874d39f12b3e513 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 4 Dec 2018 15:56:50 +1300
Subject: [PATCH 15/17] s3:pylibsmb: Make .list() work for SMBv2
In order for .list() to work on an SMBv2 connection we currently need to
use the synchronous API. We only do this if the connection is not using
multi-threading.
It the multi-threading case, it still uses the asynchronous API that it
currently does. Note however that this won't work currently with SMBv2.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source3/libsmb/pylibsmb.c | 37 +++++++++++++++++++++++++------------
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index e0eeb6e..cbaf0d2 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -876,7 +876,6 @@ static PyObject *py_cli_list(struct py_cli_state *self,
FILE_ATTRIBUTE_SYSTEM |
FILE_ATTRIBUTE_HIDDEN;
unsigned info_level = SMB_FIND_FILE_BOTH_DIRECTORY_INFO;
- struct tevent_req *req;
NTSTATUS status;
struct file_info *finfos;
size_t i, num_finfos;
@@ -900,22 +899,36 @@ static PyObject *py_cli_list(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)) {
+ result = Py_BuildValue("[]");
+ if (result == NULL) {
+ TALLOC_FREE(mask);
return NULL;
}
- 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 NULL;
+ if (self->multi_threaded) {
+ struct tevent_req *req = NULL;
+
+ req = cli_list_send(NULL, self->ev, self->cli, mask, attribute,
+ info_level);
+ TALLOC_FREE(mask);
+
+ if (!py_tevent_req_wait_exc(self, req)) {
+ return NULL;
+ }
+ status = cli_list_recv(req, NULL, &finfos, &num_finfos);
+ TALLOC_FREE(req);
+ } else {
+ status = cli_list(self->cli, mask, attribute, list_helper,
+ result);
+ TALLOC_FREE(mask);
+
+ if (NT_STATUS_IS_OK(status)) {
+ return result;
+ }
}
- result = Py_BuildValue("[]");
- if (result == NULL) {
+ if (!NT_STATUS_IS_OK(status)) {
+ PyErr_SetNTSTATUS(status);
return NULL;
}
--
2.7.4
From 75292bb91dda3dab192972073ad2371d51d540b0 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 16/17] 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.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
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 c095ad5..f0bc6b6 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -59,7 +59,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,
@@ -70,11 +70,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 cbaf0d2..8354cc9 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -849,6 +849,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 cd17f14a46900ad253194adec55f345af6559811 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 17/17] 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.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
python/samba/tests/smb.py | 2 +-
source3/libsmb/pylibsmb.c | 28 ++++++++++++++++++++++++----
2 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index f0bc6b6..1049d10 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -80,7 +80,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 8354cc9..9af517d 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -848,19 +848,34 @@ static NTSTATUS list_helper(const char *mntpoint, struct file_info *finfo,
PyObject *result = (PyObject *)state;
PyObject *file = NULL;
int ret;
+ time_t mtime;
/* 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);
+ file = PyDict_New();
if (file == NULL) {
return NT_STATUS_NO_MEMORY;
}
+ PyDict_SetItemString(file, "name", PyStr_FromString(finfo->name));
+
+ /* Windows does not always return short_name */
+ if (finfo->short_name) {
+ PyDict_SetItemString(file, "short_name",
+ PyStr_FromString(finfo->short_name));
+ } else {
+ PyDict_SetItemString(file, "short_name", Py_None);
+ }
+
+ PyDict_SetItemString(file, "size",
+ PyLong_FromUnsignedLongLong(finfo->size));
+ PyDict_SetItemString(file, "attrib", PyInt_FromLong(finfo->mode));
+ mtime = convert_timespec_to_time_t(finfo->mtime_ts);
+ PyDict_SetItemString(file, "mtime", PyInt_FromLong(mtime));
+
ret = PyList_Append(result, file);
if (ret == -1) {
return NT_STATUS_INTERNAL_ERROR;
@@ -1087,7 +1102,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
-------------- next part --------------
From 529426bb684e1756b0b461e165eb1b3f6d977711 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 4 Dec 2018 10:03:36 +1300
Subject: [PATCH] HACK: Example of async APIs failing with SMBv2
To run the test:
SELFTEST_TESTENV=restoredc:local make testenv
python source4/scripting/bin/subunitrun samba.tests.smbexample
-U"Administrator%locDCpass1
You get an error:
test: samba.tests.smbexample.SMBTests.test_smbv2_api
smb1cli_req_writev_submit: called for dialect[SMB2_10] server[restoredc]
time: 2018-12-03 21:01:11.919815Z
error: samba.tests.smbexample.SMBTests.test_smbv2_api [
Traceback (most recent call last):
File "bin/python/samba/tests/smbexample.py", line 54, in
test_smbv2_api
self.smb_conn.unlink(test_file)
NTSTATUSError: (3221225561, 'Indicates that two revision levels are
incompatible.')
]
---
python/samba/tests/smbexample.py | 54 ++++++++++++++++++++++++++++++++++++++++
source3/libsmb/pylibsmb.c | 8 +-----
2 files changed, 55 insertions(+), 7 deletions(-)
create mode 100644 python/samba/tests/smbexample.py
diff --git a/python/samba/tests/smbexample.py b/python/samba/tests/smbexample.py
new file mode 100644
index 0000000..7527fff
--- /dev/null
+++ b/python/samba/tests/smbexample.py
@@ -0,0 +1,54 @@
+# -*- coding: utf-8 -*-
+# Unix SMB/CIFS implementation. Tests for smb manipulation
+# Copyright (C) David Mulder <dmulder at suse.com> 2018
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+import samba
+import os
+import random
+import sys
+from samba import smb
+from samba import NTSTATUSError
+from samba.ntstatus import NT_STATUS_OBJECT_NAME_NOT_FOUND
+from samba.samba3 import libsmb_samba_internal
+
+PY3 = sys.version_info[0] == 3
+addom = 'addom.samba.example.com/'
+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_file = os.path.join(test_dir, 'testing').replace('/', '\\')
+
+
+class SMBTests(samba.tests.TestCase):
+ def setUp(self):
+ super(SMBTests, self).setUp()
+ self.server = os.environ["SERVER"]
+ creds = self.insta_creds(template=self.get_credentials())
+
+ from samba.samba3 import param as s3param
+ from samba import credentials
+ s3_lp = s3param.get_context()
+ s3_lp.load(os.getenv("SMB_CONF_PATH"))
+
+ # temporarily create a 2nd SMB connection for migrating the py-bindings
+ self.smb_conn = libsmb_samba_internal.Conn(self.server, "sysvol", creds)
+
+ def test_smbv2_api(self):
+ # just try to call the unlink API - we get a 'two revision levels are incompatible' error
+ self.smb_conn.unlink(test_file)
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 3a241e1..a71a066 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -427,13 +427,7 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
int signing_state = SMB_SIGNING_DEFAULT;
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",
--
2.7.4
More information about the samba-technical
mailing list