From cf5341433d7b2f3105b0529f1fdef81631e3153f Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Thu, 17 May 2012 04:48:12 +0200 Subject: [PATCH 01/16] Fix manifest file after reorganisations MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Forgot this after the doc changes… --- MANIFEST.in | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/MANIFEST.in b/MANIFEST.in index 04fba87..6f223ca 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,9 +1,8 @@ include COPYING -include IMPLEMENTATION -include MANIFEST include Makefile include NEWS -include PLATFORMS include README include acl.c include setup.cfg +include doc/conf.py +include doc/*.rst -- 2.39.5 From e0a3ca5df9044795347a8ba770e919b1d47fb76e Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Thu, 17 May 2012 04:49:37 +0200 Subject: [PATCH 02/16] =?utf8?q?Rename=20tests=20=E2=86=92=20test=20and=20?= =?utf8?q?distribute=20test=20files=20too?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The rename is done for consistency with other projects, and distributing the tests is of course useful. --- MANIFEST.in | 1 + setup.py | 2 +- {tests => test}/__init__.py | 0 {tests => test}/test_acls.py | 0 4 files changed, 2 insertions(+), 1 deletion(-) rename {tests => test}/__init__.py (100%) rename {tests => test}/test_acls.py (100%) diff --git a/MANIFEST.in b/MANIFEST.in index 6f223ca..f570711 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -6,3 +6,4 @@ include acl.c include setup.cfg include doc/conf.py include doc/*.rst +include test/*.py diff --git a/setup.py b/setup.py index dbb5f56..54cf97e 100755 --- a/setup.py +++ b/setup.py @@ -44,5 +44,5 @@ setup(name="pylibacl", libraries=libs, define_macros=macros, )], - test_suite="tests", + test_suite="test", ) diff --git a/tests/__init__.py b/test/__init__.py similarity index 100% rename from tests/__init__.py rename to test/__init__.py diff --git a/tests/test_acls.py b/test/test_acls.py similarity index 100% rename from tests/test_acls.py rename to test/test_acls.py -- 2.39.5 From e6559d0fefc85b404205a9eb856358b2ce083425 Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Sat, 24 May 2014 00:20:58 +0200 Subject: [PATCH 03/16] Enable testing on more Python versions and PyPy MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit … except for the refcount test which doesn't work (and makes no sense) under PyPy as it has a non-reference count model). --- Makefile | 9 +++++++-- test/test_acls.py | 3 +++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 0ba620d..1bb21b7 100644 --- a/Makefile +++ b/Makefile @@ -20,12 +20,17 @@ $(DOCHTML)/index.html: $(MODNAME) $(RSTFILES) doc: $(DOCHTML)/index.html test: - for ver in 2.4 2.5 2.6 2.7 3.0 3.1 3.2; do \ + for ver in 2.4 2.5 2.6 2.7 3.0 3.1 3.2 3.3 3.4; do \ if type python$$ver >/dev/null; then \ echo Testing with python$$ver; \ - python$$ver ./setup.py test; \ + python$$ver ./setup.py test -q; \ fi; \ done + @if type pypy >/dev/null; then \ + echo Testing with pypy; \ + pypy ./setup.py test -q; \ + fi + clean: rm -rf $(DOCHTML) $(DOCTREES) diff --git a/test/test_acls.py b/test/test_acls.py index 45cd0ed..dcd4e2c 100644 --- a/test/test_acls.py +++ b/test/test_acls.py @@ -25,6 +25,7 @@ import unittest import os import tempfile import sys +import platform import posix1e from posix1e import * @@ -197,6 +198,8 @@ class ModificationTests(aclTest, unittest.TestCase): def checkRef(self, obj): """Checks if a given obj has a 'sane' refcount""" + if platform.python_implementation() == "PyPy": + return ref_cnt = sys.getrefcount(obj) # FIXME: hardcoded value for the max ref count... but I've # seen it overflow on bad reference counting, so it's better -- 2.39.5 From 242949c0591da37ed54683efd97e77fb965eef1c Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Sat, 24 May 2014 00:22:13 +0200 Subject: [PATCH 04/16] Unify env and python variable TEST_DIR The difference is just confusing. --- test/test_acls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_acls.py b/test/test_acls.py index dcd4e2c..824dd5d 100644 --- a/test/test_acls.py +++ b/test/test_acls.py @@ -30,7 +30,7 @@ import platform import posix1e from posix1e import * -TEST_DIR = os.environ.get("TESTDIR", ".") +TEST_DIR = os.environ.get("TEST_DIR", ".") BASIC_ACL_TEXT = "u::rw,g::r,o::-" -- 2.39.5 From fced41571d7a1d1255e1fbd6f662be8ee0f49a73 Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Sat, 24 May 2014 00:36:37 +0200 Subject: [PATCH 05/16] Fix copyright years and intent to (not) test Solaris --- README | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README b/README index 7b42b08..e725fd9 100644 --- a/README +++ b/README @@ -20,7 +20,7 @@ pylibacl has been written and tested on Linux, kernel v2.4 or newer, with XFS filesystems; ext2/ext3 should also work. Since release 0.4.0, FreeBSD 7 also has quite good support. If any other platform implements the POSIX.1e draft, pylibacl can be used. I heard that -Solaris does, but I can't test it yet. +Solaris does, but I can't test it. - Python 2.4 or newer - operating system: @@ -33,7 +33,7 @@ Solaris does, but I can't test it yet. License ------- -pylibacl is Copyright (C) 2002-2009, 2012 Iustin Pop. +pylibacl is Copyright (C) 2002-2009, 2012, 2014 Iustin Pop. pylibacl is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License as published by the Free -- 2.39.5 From 1633ae3079b82f251c0a7f7bc39b91d7339b0474 Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Sat, 24 May 2014 00:36:58 +0200 Subject: [PATCH 06/16] Fix homepage URL --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 54cf97e..d95acd1 100755 --- a/setup.py +++ b/setup.py @@ -38,7 +38,7 @@ setup(name="pylibacl", long_description=long_desc, author="Iustin Pop", author_email="iusty@k1024.org", - url="http://pylibacl.sourceforge.net/", + url="http://pylibacl.k1024.org/", license="LGPL", ext_modules=[Extension("posix1e", ["acl.c"], libraries=libs, -- 2.39.5 From 35e9a0fb6e20c9621ddd8659e9995600c0fde7f3 Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Sat, 24 May 2014 00:37:13 +0200 Subject: [PATCH 07/16] Fix some indentation issues in tests --- test/test_acls.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/test/test_acls.py b/test/test_acls.py index 824dd5d..08bfd4e 100644 --- a/test/test_acls.py +++ b/test/test_acls.py @@ -152,14 +152,14 @@ class AclExtensions(aclTest, unittest.TestCase): basic_acl.applyto(fd) for item in fd, fname: self.assertFalse(has_extended(item), - "A simple ACL should not be reported as extended") + "A simple ACL should not be reported as extended") enhanced_acl = posix1e.ACL(text="u::rw,g::-,o::-,u:root:rw,mask::r") self.assertTrue(enhanced_acl.valid(), "Failure to build an extended ACL") enhanced_acl.applyto(fd) for item in fd, fname: self.assertTrue(has_extended(item), - "An extended ACL should be reported as such") + "An extended ACL should be reported as such") @has_ext(HAS_EQUIV_MODE) def testEquivMode(self): @@ -205,8 +205,8 @@ class ModificationTests(aclTest, unittest.TestCase): # seen it overflow on bad reference counting, so it's better # to be safe if ref_cnt < 2 or ref_cnt > 1024: - self.fail("Wrong reference count, expected 2-1024 and got %d" % - ref_cnt) + self.fail("Wrong reference count, expected 2-1024 and got %d" % + ref_cnt) def testStr(self): """Test str() of an ACL.""" @@ -245,7 +245,8 @@ class ModificationTests(aclTest, unittest.TestCase): e.tag_type = tag_type e.permset.clear() self.assertFalse(acl.valid(), - "ACL containing duplicate entries should not be valid") + "ACL containing duplicate entries" + " should not be valid") acl.delete_entry(e) @has_ext(HAS_ACL_ENTRY) @@ -262,8 +263,8 @@ class ModificationTests(aclTest, unittest.TestCase): e.permset.clear() acl.calc_mask() self.assertTrue(acl.valid(), - "ACL should be able to hold multiple" - " user/group entries") + "ACL should be able to hold multiple" + " user/group entries") @has_ext(HAS_ACL_ENTRY) def testMultipleBadEntries(self): @@ -279,14 +280,14 @@ class ModificationTests(aclTest, unittest.TestCase): e1.permset.clear() acl.calc_mask() self.assertTrue(acl.valid(), "ACL should be able to add a" - " user/group entry") + " user/group entry") e2 = acl.append() e2.tag_type = tag_type e2.qualifier = 0 e2.permset.clear() acl.calc_mask() self.assertFalse(acl.valid(), "ACL should not validate when" - " containing two duplicate entries") + " containing two duplicate entries") acl.delete_entry(e1) acl.delete_entry(e2) @@ -308,15 +309,15 @@ class ModificationTests(aclTest, unittest.TestCase): str_ps = str(ps) self.checkRef(str_ps) self.assertFalse(ps.test(perm), "Empty permission set should not" - " have permission '%s'" % pmap[perm]) + " have permission '%s'" % pmap[perm]) ps.add(perm) self.assertTrue(ps.test(perm), "Permission '%s' should exist" - " after addition" % pmap[perm]) + " after addition" % pmap[perm]) str_ps = str(ps) self.checkRef(str_ps) ps.delete(perm) self.assertFalse(ps.test(perm), "Permission '%s' should not exist" - " after deletion" % pmap[perm]) + " after deletion" % pmap[perm]) if __name__ == "__main__": -- 2.39.5 From 3c31be20e80ed169a1f0adf91b499001a80e7e67 Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Sat, 24 May 2014 00:40:07 +0200 Subject: [PATCH 08/16] Bump version for a trivial 0.5.2 release --- NEWS | 5 +++++ README | 2 +- doc/conf.py | 6 +++--- setup.py | 2 +- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index a84e674..95bdd6d 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,11 @@ News ==== +Version 0.5.2 +------------- + +No visible changes release: just fix tests when running under pypy. + Version 0.5.1 ------------- diff --git a/README b/README index e725fd9..6978428 100644 --- a/README +++ b/README @@ -6,7 +6,7 @@ POSIX.1e Access Control Lists present in some OS/file-systems combinations. Downloads: go to https://github.com/iustin/pylibacl/downloads. Latest -version is 0.5.1. The source repository is either +version is 0.5.2. The source repository is either at ``_ or at https://github.com/iustin/pylibacl. diff --git a/doc/conf.py b/doc/conf.py index 3b2d215..e8299c1 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -41,16 +41,16 @@ master_doc = 'index' # General information about the project. project = u'pylibacl' -copyright = u'2002-2009, 2012, Iustin Pop' +copyright = u'2002-2009, 2012, 2014, Iustin Pop' # The version info for the project you're documenting, acts as replacement for # |version| and |release|, also used in various other places throughout the # built documents. # # The short X.Y version. -version = '0.5.1' +version = '0.5.2' # The full version, including alpha/beta/rc tags. -release = '0.5.1' +release = '0.5.2' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.py b/setup.py index d95acd1..9c8f2cd 100755 --- a/setup.py +++ b/setup.py @@ -30,7 +30,7 @@ long_desc = """This is a C extension module for Python which implements POSIX ACLs manipulation. It is a wrapper on top of the systems's acl C library - see acl(5).""" -version = "0.5.1" +version = "0.5.2" setup(name="pylibacl", version=version, -- 2.39.5 From 3c21ee4aef03e217780598d9deb4aca610469b70 Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Sat, 24 May 2014 00:41:59 +0200 Subject: [PATCH 09/16] Add a 'dist' target --- Makefile | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 1bb21b7..2005cc3 100644 --- a/Makefile +++ b/Makefile @@ -19,22 +19,24 @@ $(DOCHTML)/index.html: $(MODNAME) $(RSTFILES) doc: $(DOCHTML)/index.html +dist: + fakeroot ./setup.py sdist + test: - for ver in 2.4 2.5 2.6 2.7 3.0 3.1 3.2 3.3 3.4; do \ + @for ver in 2.4 2.5 2.6 2.7 3.0 3.1 3.2 3.3 3.4; do \ if type python$$ver >/dev/null; then \ echo Testing with python$$ver; \ python$$ver ./setup.py test -q; \ - fi; \ + fi; \ done @if type pypy >/dev/null; then \ echo Testing with pypy; \ - pypy ./setup.py test -q; \ + pypy ./setup.py test -q; \ fi - clean: rm -rf $(DOCHTML) $(DOCTREES) rm -f $(MODNAME) rm -rf build -.PHONY: doc test clean +.PHONY: doc test clean dist -- 2.39.5 From a7a6ccb9a7b2cb3684460f86a2c0736296f56ae6 Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Sat, 24 May 2014 00:57:40 +0200 Subject: [PATCH 10/16] Fix download link in README MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit … since github no longer provides downloads. --- README | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README b/README index 6978428..cbfc4c5 100644 --- a/README +++ b/README @@ -5,7 +5,7 @@ This is a Python 2.4+ extension module allows you to manipulate the POSIX.1e Access Control Lists present in some OS/file-systems combinations. -Downloads: go to https://github.com/iustin/pylibacl/downloads. Latest +Downloads: go to http://pylibacl.k1024.org/downloads. Latest version is 0.5.2. The source repository is either at ``_ or at https://github.com/iustin/pylibacl. -- 2.39.5 From a107ae1df8a93834fd1e5da23d63c958e518c1a0 Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Tue, 24 Jun 2014 21:51:29 +0200 Subject: [PATCH 11/16] Fix error message in Entry_set_qualifier This closes #1 (github issue). --- acl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/acl.c b/acl.c index 6c0851b..8d757ab 100644 --- a/acl.c +++ b/acl.c @@ -1,7 +1,7 @@ /* posix1e - a python module exposing the posix acl functions - Copyright (C) 2002-2009, 2012 Iustin Pop + Copyright (C) 2002-2009, 2012, 2014 Iustin Pop This library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public @@ -838,7 +838,7 @@ static int Entry_set_qualifier(PyObject* obj, PyObject* value, void* arg) { if(!PyInt_Check(value)) { PyErr_SetString(PyExc_TypeError, - "tag type must be integer"); + "qualifier must be integer"); return -1; } uidgid = PyInt_AsLong(value); -- 2.39.5 From 5be22f1aa8c5adce7f0b66d9668986f35381a688 Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Tue, 24 Jun 2014 23:23:12 +0200 Subject: [PATCH 12/16] First step towards fixing qualifier overflow This is still a work-in-progress, since it only deals with Python-level overflows, but it's a step forward. --- acl.c | 6 +++++- test/test_acls.py | 18 +++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/acl.c b/acl.c index 8d757ab..22f83fe 100644 --- a/acl.c +++ b/acl.c @@ -841,7 +841,11 @@ static int Entry_set_qualifier(PyObject* obj, PyObject* value, void* arg) { "qualifier must be integer"); return -1; } - uidgid = PyInt_AsLong(value); + if((uidgid = PyInt_AsLong(value)) == -1) { + if(PyErr_Occurred() != NULL) { + return -1; + } + } if(acl_set_qualifier(self->entry, (void*)&uidgid) == -1) { PyErr_SetFromErrno(PyExc_IOError); return -1; diff --git a/test/test_acls.py b/test/test_acls.py index 08bfd4e..eea3f36 100644 --- a/test/test_acls.py +++ b/test/test_acls.py @@ -3,7 +3,7 @@ """Unittests for the posix1e module""" -# Copyright (C) 2002-2009, 2012 Iustin Pop +# Copyright (C) 2002-2009, 2012, 2014 Iustin Pop # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -40,6 +40,9 @@ M0500 = 320 # octal 0500 M0644 = 420 # octal 0644 M0755 = 493 # octal 755 +# Check if running under Python 3 +IS_PY_3K = sys.hexversion >= 0x03000000 + def _skip_test(fn): """Wrapper to skip a test""" new_fn = lambda x: None @@ -320,5 +323,18 @@ class ModificationTests(aclTest, unittest.TestCase): " after deletion" % pmap[perm]) + @has_ext(HAS_ACL_ENTRY and IS_PY_3K) + def testQualifierOverflow(self): + """Tests qualifier overflow handling""" + acl = posix1e.ACL() + e = acl.append() + qualifier = sys.maxsize * 2 + for tag in [posix1e.ACL_USER, posix1e.ACL_GROUP]: + e.tag_type = posix1e.ACL_USER + with self.assertRaises(OverflowError): + e.qualifier = qualifier + print(e.qualifier) + + if __name__ == "__main__": unittest.main() -- 2.39.5 From a34beacc4652c26f9d8c00d95f18f11299b9d294 Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Sat, 28 Jun 2014 14:13:32 +0200 Subject: [PATCH 13/16] Try to fix uid_t/gid_t usage in entry qualifiers The current code is very broken with regards to the casting between Python's integer type (either int in Py2 or the magic int/long in Py3) and the uid_t/gid_t POSIX types. This means that the code is broken outside "small" integer values. By using uid_t/gid_t as appropriate, we can fix most of the problem (at least as far as the new unittests are concerned). There's still no automatic printf format modifier for them, so the code hardcodes unsigned (which is what Linux/glibc defines them to), and also there's a unittest that expects negative values to fail when set. This should fix issue #3 (github). --- acl.c | 126 ++++++++++++++++++++++++++++++++++++---------- test/test_acls.py | 43 +++++++++++++++- 2 files changed, 140 insertions(+), 29 deletions(-) diff --git a/acl.c b/acl.c index 22f83fe..4811b39 100644 --- a/acl.c +++ b/acl.c @@ -672,6 +672,46 @@ static PyObject* ACL_append(PyObject *obj, PyObject *args) { /***** Entry type *****/ +typedef struct { + acl_tag_t tag; + union { + uid_t uid; + gid_t gid; + }; +} tag_qual; + +/* Helper function to get the tag and qualifier of an Entry at the + same time. This is "needed" because the acl_get_qualifier function + returns a pointer to different types, based on the tag value, and + thus it's not straightforward to get the right type. + + It sets a Python exception if an error occurs, and return 0 in this + case. If successful, the tag is set to the tag type, and the + qualifier (if any) to either the uid or the gid entry in the + tag_qual structure. +*/ +int get_tag_qualifier(acl_entry_t entry, tag_qual *tq) { + void *p; + + if(acl_get_tag_type(entry, &tq->tag) == -1) { + PyErr_SetFromErrno(PyExc_IOError); + return 0; + } + if (tq->tag == ACL_USER || tq->tag == ACL_GROUP) { + if((p = acl_get_qualifier(entry)) == NULL) { + PyErr_SetFromErrno(PyExc_IOError); + return 0; + } + if (tq->tag == ACL_USER) { + tq->uid = *(uid_t*)p; + } else { + tq->gid = *(gid_t*)p; + } + acl_free(p); + } + return 1; +} + /* Creation of a new Entry instance */ static PyObject* Entry_new(PyTypeObject* type, PyObject* args, PyObject *keywds) { @@ -725,31 +765,18 @@ static void Entry_dealloc(PyObject* obj) { /* Converts the entry to a text format */ static PyObject* Entry_str(PyObject *obj) { - acl_tag_t tag; - uid_t qualifier; - void *p; PyObject *format, *kind; Entry_Object *self = (Entry_Object*) obj; + tag_qual tq; - if(acl_get_tag_type(self->entry, &tag) == -1) { - PyErr_SetFromErrno(PyExc_IOError); + if(!get_tag_qualifier(self->entry, &tq)) { return NULL; } - if(tag == ACL_USER || tag == ACL_GROUP) { - if((p = acl_get_qualifier(self->entry)) == NULL) { - PyErr_SetFromErrno(PyExc_IOError); - return NULL; - } - qualifier = *(uid_t*)p; - acl_free(p); - } else { - qualifier = 0; - } format = MyString_FromString("ACL entry for "); if(format == NULL) return NULL; - switch(tag) { + switch(tq.tag) { case ACL_UNDEFINED_TAG: kind = MyString_FromString("undefined type"); break; @@ -763,10 +790,14 @@ static PyObject* Entry_str(PyObject *obj) { kind = MyString_FromString("the others"); break; case ACL_USER: - kind = MyString_FromFormat("user with uid %d", qualifier); + /* FIXME: here and in the group case, we're formatting with + unsigned, because there's no way to automatically determine + the signed-ness of the types; on Linux(glibc) they're + unsigned, so we'll go along with that */ + kind = MyString_FromFormat("user with uid %u", tq.uid); break; case ACL_GROUP: - kind = MyString_FromFormat("group with gid %d", qualifier); + kind = MyString_FromFormat("group with gid %u", tq.gid); break; case ACL_MASK: kind = MyString_FromString("the mask"); @@ -828,7 +859,11 @@ static PyObject* Entry_get_tag_type(PyObject *obj, void* arg) { */ static int Entry_set_qualifier(PyObject* obj, PyObject* value, void* arg) { Entry_Object *self = (Entry_Object*) obj; - int uidgid; + long uidgid; + uid_t uid; + gid_t gid; + void *p; + acl_tag_t tag; if(value == NULL) { PyErr_SetString(PyExc_TypeError, @@ -846,7 +881,38 @@ static int Entry_set_qualifier(PyObject* obj, PyObject* value, void* arg) { return -1; } } - if(acl_set_qualifier(self->entry, (void*)&uidgid) == -1) { + /* Due to how acl_set_qualifier takes its argument, we have to do + this ugly dance with two variables and a pointer that will + point to one of them. */ + if(acl_get_tag_type(self->entry, &tag) == -1) { + PyErr_SetFromErrno(PyExc_IOError); + return -1; + } + uid = uidgid; + gid = uidgid; + switch(tag) { + case ACL_USER: + if((long)uid != uidgid) { + PyErr_SetString(PyExc_OverflowError, "cannot assign given qualifier"); + return -1; + } else { + p = &uid; + } + break; + case ACL_GROUP: + if((long)gid != uidgid) { + PyErr_SetString(PyExc_OverflowError, "cannot assign given qualifier"); + return -1; + } else { + p = &gid; + } + break; + default: + PyErr_SetString(PyExc_TypeError, + "can only set qualifiers on ACL_USER or ACL_GROUP entries"); + return -1; + } + if(acl_set_qualifier(self->entry, p) == -1) { PyErr_SetFromErrno(PyExc_IOError); return -1; } @@ -857,20 +923,26 @@ static int Entry_set_qualifier(PyObject* obj, PyObject* value, void* arg) { /* Returns the qualifier of the entry */ static PyObject* Entry_get_qualifier(PyObject *obj, void* arg) { Entry_Object *self = (Entry_Object*) obj; - void *p; - int value; + long value; + tag_qual tq; if (self->entry == NULL) { PyErr_SetString(PyExc_AttributeError, "entry attribute"); return NULL; } - if((p = acl_get_qualifier(self->entry)) == NULL) { - PyErr_SetFromErrno(PyExc_IOError); + if(!get_tag_qualifier(self->entry, &tq)) { + return NULL; + } + if (tq.tag == ACL_USER) { + value = tq.uid; + } else if (tq.tag == ACL_GROUP) { + value = tq.gid; + } else { + PyErr_SetString(PyExc_TypeError, + "given entry doesn't have an user or" + " group tag"); return NULL; } - value = *(uid_t*)p; - acl_free(p); - return PyInt_FromLong(value); } diff --git a/test/test_acls.py b/test/test_acls.py index eea3f36..9928bf9 100644 --- a/test/test_acls.py +++ b/test/test_acls.py @@ -26,6 +26,7 @@ import os import tempfile import sys import platform +import re import posix1e from posix1e import * @@ -323,6 +324,33 @@ class ModificationTests(aclTest, unittest.TestCase): " after deletion" % pmap[perm]) + @has_ext(HAS_ACL_ENTRY and IS_PY_3K) + def testQualifierValues(self): + """Tests qualifier correct store/retrieval""" + acl = posix1e.ACL() + e = acl.append() + # work around deprecation warnings + if hasattr(self, 'assertRegex'): + fn = self.assertRegex + else: + fn = self.assertRegexpMatches + for tag in [posix1e.ACL_USER, posix1e.ACL_GROUP]: + qualifier = 1 + e.tag_type = tag + while True: + if tag == posix1e.ACL_USER: + regex = re.compile("user with uid %d" % qualifier) + else: + regex = re.compile("group with gid %d" % qualifier) + try: + e.qualifier = qualifier + except OverflowError: + # reached overflow condition, break + break + self.assertEqual(e.qualifier, qualifier) + fn(str(e), regex) + qualifier *= 2 + @has_ext(HAS_ACL_ENTRY and IS_PY_3K) def testQualifierOverflow(self): """Tests qualifier overflow handling""" @@ -330,10 +358,21 @@ class ModificationTests(aclTest, unittest.TestCase): e = acl.append() qualifier = sys.maxsize * 2 for tag in [posix1e.ACL_USER, posix1e.ACL_GROUP]: - e.tag_type = posix1e.ACL_USER + e.tag_type = tag with self.assertRaises(OverflowError): e.qualifier = qualifier - print(e.qualifier) + + @has_ext(HAS_ACL_ENTRY and IS_PY_3K) + def testNegativeQualifier(self): + """Tests negative qualifier handling""" + # Note: this presumes that uid_t/gid_t in C are unsigned... + acl = posix1e.ACL() + e = acl.append() + for tag in [posix1e.ACL_USER, posix1e.ACL_GROUP]: + e.tag_type = tag + for qualifier in [-10, -5, -1]: + with self.assertRaises(OverflowError): + e.qualifier = qualifier if __name__ == "__main__": -- 2.39.5 From cd11c1a73ff823a088ab212f63f08db99be8a524 Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Sat, 28 Jun 2014 22:35:32 +0200 Subject: [PATCH 14/16] Simplify a bit the module initialization A few constants are more nicely defined. --- acl.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/acl.c b/acl.c index 4811b39..d037628 100644 --- a/acl.c +++ b/acl.c @@ -1843,17 +1843,15 @@ void initposix1e(void) PyModule_AddIntConstant(m, "ACL_MISS_ERROR", ACL_MISS_ERROR); PyModule_AddIntConstant(m, "ACL_ENTRY_ERROR", ACL_ENTRY_ERROR); - /* declare the Linux extensions */ - PyModule_AddIntConstant(m, "HAS_ACL_FROM_MODE", 1); - PyModule_AddIntConstant(m, "HAS_ACL_CHECK", 1); - PyModule_AddIntConstant(m, "HAS_EXTENDED_CHECK", 1); - PyModule_AddIntConstant(m, "HAS_EQUIV_MODE", 1); +#define LINUX_EXT_VAL 1 #else - PyModule_AddIntConstant(m, "HAS_ACL_FROM_MODE", 0); - PyModule_AddIntConstant(m, "HAS_ACL_CHECK", 0); - PyModule_AddIntConstant(m, "HAS_EXTENDED_CHECK", 0); - PyModule_AddIntConstant(m, "HAS_EQUIV_MODE", 0); +#define LINUX_EXT_VAL 0 #endif + /* declare the Linux extensions */ + PyModule_AddIntConstant(m, "HAS_ACL_FROM_MODE", LINUX_EXT_VAL); + PyModule_AddIntConstant(m, "HAS_ACL_CHECK", LINUX_EXT_VAL); + PyModule_AddIntConstant(m, "HAS_EXTENDED_CHECK", LINUX_EXT_VAL); + PyModule_AddIntConstant(m, "HAS_EQUIV_MODE", LINUX_EXT_VAL); #ifdef IS_PY3K return m; -- 2.39.5 From bcb29c630f5160497373e3273de6313c63c6cea0 Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Thu, 30 Apr 2015 19:43:53 +0200 Subject: [PATCH 15/16] tests: don't call acl_to_text on an invalid ACL While Linux is happy to convert it to text, it seems that under FreeBSD this doesn't (always? sometimes?) work, so let's use a proper ACL in the str() test. Closes #5. --- test/test_acls.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_acls.py b/test/test_acls.py index 9928bf9..aa7d584 100644 --- a/test/test_acls.py +++ b/test/test_acls.py @@ -3,7 +3,7 @@ """Unittests for the posix1e module""" -# Copyright (C) 2002-2009, 2012, 2014 Iustin Pop +# Copyright (C) 2002-2009, 2012, 2014, 2015 Iustin Pop # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -214,7 +214,7 @@ class ModificationTests(aclTest, unittest.TestCase): def testStr(self): """Test str() of an ACL.""" - acl = posix1e.ACL() + acl = posix1e.ACL(text=BASIC_ACL_TEXT) str_acl = str(acl) self.checkRef(str_acl) -- 2.39.5 From 18d179a9264d6800e2df6e6030eaa5d72237324a Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Thu, 30 Apr 2015 19:48:41 +0200 Subject: [PATCH 16/16] tests: ignore IOErrors during operations invalid ACLs It seems that FreeBSD at least returns EINVAL from calc_mask() and other calls if the ACL is invalid, so let's ignore such IOErrors. This should hopefully fix #6 and #7. --- test/test_acls.py | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/test/test_acls.py b/test/test_acls.py index aa7d584..59d6604 100644 --- a/test/test_acls.py +++ b/test/test_acls.py @@ -27,6 +27,7 @@ import tempfile import sys import platform import re +import errno import posix1e from posix1e import * @@ -58,6 +59,20 @@ def has_ext(extension): else: return lambda x: x +def ignore_ioerror(errnum, fn, *args, **kwargs): + """Call a function while ignoring some IOErrors. + + This is needed as some OSes (e.g. FreeBSD) return failure (EINVAL) + when doing certain operations on an invalid ACL. + + """ + try: + fn(*args, **kwargs) + except IOError: + err = sys.exc_info()[1] + if err.errno == errnum: + return + raise class aclTest: """Support functions ACLs""" @@ -224,7 +239,7 @@ class ModificationTests(aclTest, unittest.TestCase): acl = posix1e.ACL() e = acl.append() e.tag_type = posix1e.ACL_OTHER - acl.calc_mask() + ignore_ioerror(errno.EINVAL, acl.calc_mask) str_format = str(e) self.checkRef(str_format) @@ -234,9 +249,9 @@ class ModificationTests(aclTest, unittest.TestCase): acl = posix1e.ACL() e = acl.append() e.tag_type = posix1e.ACL_OTHER - acl.calc_mask() + ignore_ioerror(errno.EINVAL, acl.calc_mask) acl.delete_entry(e) - acl.calc_mask() + ignore_ioerror(errno.EINVAL, acl.calc_mask) @has_ext(HAS_ACL_ENTRY) def testDoubleEntries(self): @@ -273,11 +288,11 @@ class ModificationTests(aclTest, unittest.TestCase): @has_ext(HAS_ACL_ENTRY) def testMultipleBadEntries(self): """Test multiple invalid entries""" - acl = posix1e.ACL(text=BASIC_ACL_TEXT) - self.assertTrue(acl.valid(), "ACL built from standard description" - " should be valid") for tag_type in (posix1e.ACL_USER, posix1e.ACL_GROUP): + acl = posix1e.ACL(text=BASIC_ACL_TEXT) + self.assertTrue(acl.valid(), "ACL built from standard description" + " should be valid") e1 = acl.append() e1.tag_type = tag_type e1.qualifier = 0 @@ -289,11 +304,13 @@ class ModificationTests(aclTest, unittest.TestCase): e2.tag_type = tag_type e2.qualifier = 0 e2.permset.clear() - acl.calc_mask() + ignore_ioerror(errno.EINVAL, acl.calc_mask) self.assertFalse(acl.valid(), "ACL should not validate when" " containing two duplicate entries") acl.delete_entry(e1) - acl.delete_entry(e2) + # FreeBSD trips over itself here and can't delete the + # entry, even though it still exists. + ignore_ioerror(errno.EINVAL, acl.delete_entry, e2) @has_ext(HAS_ACL_ENTRY) def testPermset(self): -- 2.39.5