From 72f37a88565370abeee3e523a123099d468cc0b4 Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Wed, 27 Nov 2019 23:04:05 +0100 Subject: [PATCH] Add pathlib support in apply_to and has_extended MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This is done by switching these functions to PyUnicode_FSConverter, thus removing a chunk of custom, and probably buggy, code. Easier to read as well… --- NEWS | 16 ++++++++ acl.c | 100 ++++++++++++++++++++++----------------------- tests/test_acls.py | 29 +++++++++---- 3 files changed, 87 insertions(+), 58 deletions(-) diff --git a/NEWS b/NEWS index 1567ec1..9e8f2e8 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,22 @@ News ==== +Version 0.6.0 +------------- + +*unreleased* + +Major release removing Python 2 support. This allow both code cleanup +and new features, such as: + +- Support for pathlib objects in `apply_to` and `has_extended` + functions. +- Use of built-in C API functions for bytes/unicode/pathlib conversion + when dealing with file names, removing custom code (with the + associated benefits). + +Additionally, test suite has changed to `pytest`. + Version 0.5.4 ------------- diff --git a/acl.c b/acl.c index bb55c5b..5a8d630 100644 --- a/acl.c +++ b/acl.c @@ -375,40 +375,39 @@ static char __applyto_doc__[] = /* Applies the ACL to a file */ static PyObject* ACL_applyto(PyObject* obj, PyObject* args) { ACL_Object *self = (ACL_Object*) obj; - PyObject *myarg; + PyObject *target, *tmp; acl_type_t type = ACL_TYPE_ACCESS; int nret; int fd; - if (!PyArg_ParseTuple(args, "O|I", &myarg, &type)) + if (!PyArg_ParseTuple(args, "O|I", &target, &type)) return NULL; - - if(PyBytes_Check(myarg)) { - char *filename = PyBytes_AS_STRING(myarg); - nret = acl_set_file(filename, type, self->acl); - } else if (PyUnicode_Check(myarg)) { - PyObject *o = - PyUnicode_AsEncodedString(myarg, - Py_FileSystemDefaultEncoding, "strict"); - if (o == NULL) - return NULL; - const char *filename = PyBytes_AS_STRING(o); - nret = acl_set_file(filename, type, self->acl); - Py_DECREF(o); - } else if((fd = PyObject_AsFileDescriptor(myarg)) != -1) { - nret = acl_set_fd(fd, self->acl); + if ((fd = PyObject_AsFileDescriptor(target)) != -1) { + if((nret = acl_set_fd(fd, self->acl)) == -1) { + PyErr_SetFromErrno(PyExc_IOError); + } } else { - PyErr_SetString(PyExc_TypeError, "argument 1 must be string, int," - " or file-like object"); - return 0; + // PyObject_AsFileDescriptor sets an error when failing, so clear + // it such that further code works; some method lookups fail if an + // error already occured when called, which breaks at least + // PyOS_FSPath (called by FSConverter). + PyErr_Clear(); + if(PyUnicode_FSConverter(target, &tmp)) { + char *filename = PyBytes_AS_STRING(tmp); + if ((nret = acl_set_file(filename, type, self->acl)) == -1) { + PyErr_SetFromErrnoWithFilename(PyExc_IOError, filename); + } + Py_DECREF(tmp); + } else { + nret = -1; + } } - if(nret == -1) { - return PyErr_SetFromErrno(PyExc_IOError); + if (nret < 0) { + return NULL; + } else { + Py_INCREF(Py_None); + return Py_None; } - - /* Return the result */ - Py_INCREF(Py_None); - return Py_None; } static char __valid_doc__[] = @@ -1575,38 +1574,39 @@ static char __has_extended_doc__[] = /* Check for extended ACL a file or fd */ static PyObject* aclmodule_has_extended(PyObject* obj, PyObject* args) { - PyObject *myarg; + PyObject *item, *tmp; int nret; int fd; - if (!PyArg_ParseTuple(args, "O", &myarg)) + if (!PyArg_ParseTuple(args, "O", &item)) return NULL; - if(PyBytes_Check(myarg)) { - const char *filename = PyBytes_AS_STRING(myarg); - nret = acl_extended_file(filename); - } else if (PyUnicode_Check(myarg)) { - PyObject *o = - PyUnicode_AsEncodedString(myarg, - Py_FileSystemDefaultEncoding, "strict"); - if (o == NULL) - return NULL; - const char *filename = PyBytes_AS_STRING(o); - nret = acl_extended_file(filename); - Py_DECREF(o); - } else if((fd = PyObject_AsFileDescriptor(myarg)) != -1) { - nret = acl_extended_fd(fd); + if((fd = PyObject_AsFileDescriptor(item)) != -1) { + if((nret = acl_extended_fd(fd)) == -1) { + PyErr_SetFromErrno(PyExc_IOError); + } } else { - PyErr_SetString(PyExc_TypeError, "argument 1 must be string, int," - " or file-like object"); - return 0; - } - if(nret == -1) { - return PyErr_SetFromErrno(PyExc_IOError); + // PyObject_AsFileDescriptor sets an error when failing, so clear + // it such that further code works; some method lookups fail if an + // error already occured when called, which breaks at least + // PyOS_FSPath (called by FSConverter). + PyErr_Clear(); + if(PyUnicode_FSConverter(item, &tmp)) { + char *filename = PyBytes_AS_STRING(tmp); + if ((nret = acl_extended_file(filename)) == -1) { + PyErr_SetFromErrnoWithFilename(PyExc_IOError, filename); + } + Py_DECREF(tmp); + } else { + nret = -1; + } } - /* Return the result */ - return PyBool_FromLong(nret); + if (nret < 0) { + return NULL; + } else { + return PyBool_FromLong(nret); + } } #endif diff --git a/tests/test_acls.py b/tests/test_acls.py index ebb14f6..0205a52 100644 --- a/tests/test_acls.py +++ b/tests/test_acls.py @@ -307,18 +307,31 @@ class TestAclExtensions: assert acl2.check() @require_extended_check - def test_extended(self, testdir): + def test_applyto_extended(self, subject): """Test the acl_extended function""" - fd, fname = get_file(testdir) basic_acl = posix1e.ACL(text=BASIC_ACL_TEXT) - basic_acl.applyto(fd) - for item in fd, fname: - assert not has_extended(item) + basic_acl.applyto(subject) + assert not has_extended(subject) enhanced_acl = posix1e.ACL(text="u::rw,g::-,o::-,u:root:rw,mask::r") assert enhanced_acl.valid() - enhanced_acl.applyto(fd) - for item in fd, fname: - assert has_extended(item) + enhanced_acl.applyto(subject) + assert has_extended(subject) + + @require_extended_check + @pytest.mark.parametrize( + "gen", [ get_file_and_symlink, get_file_and_fobject ]) + def test_applyto_extended_mixed(self, testdir, gen): + """Test the acl_extended function""" + with gen(testdir) as (a, b): + basic_acl = posix1e.ACL(text=BASIC_ACL_TEXT) + basic_acl.applyto(a) + for item in a, b: + assert not has_extended(item) + enhanced_acl = posix1e.ACL(text="u::rw,g::-,o::-,u:root:rw,mask::r") + assert enhanced_acl.valid() + enhanced_acl.applyto(b) + for item in a, b: + assert has_extended(item) @require_extended_check def test_extended_arg_handling(self): -- 2.39.5