s4/librpc/py_security: use SDDLValueError for better error messages

The aim is to allow samba-tool to tell users where their SDDL went
wrong.

Some tests would turn into errors (not knownfail-able failures)
if they were not changed at the same time, so they are changed too.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/python/samba/tests/sddl.py b/python/samba/tests/sddl.py
index 7951af01b607e2fc6295a7dd4307ba94b801ac21..5317543010baea4181c1c61f71fc532a437f46c4 100644 (file)
--- a/python/samba/tests/sddl.py
+++ b/python/samba/tests/sddl.py
@@ -82,7 +82,7 @@ class SddlDecodeEncodeBase(TestCase):
             self.assertEqual(sddl, canonical)
 
     def _test_sddl_should_fail_with_args(self, s, canonical):
-        with self.assertRaises(ValueError):
+        with self.assertRaises(security.SDDLValueError):
             sd = security.descriptor.from_sddl(s, self.domain_sid)
             print(sd.as_sddl(self.domain_sid))
 

diff --git a/python/samba/tests/security.py b/python/samba/tests/security.py
index 3b7eb3fad48f14cf5734ebb75dbe5a6cf3815bbe..fb2897e4dfc4ff13515a9b7fe59cfd775d634096 100644 (file)
--- a/python/samba/tests/security.py
+++ b/python/samba/tests/security.py
@@ -67,7 +67,7 @@ class SecurityDescriptorTests(samba.tests.TestCase):
         self.assertEqual(desc.type, 0x8004)
 
     def test_from_sddl_invalidsddl(self):
-        self.assertRaises(ValueError, security.descriptor.from_sddl, "foo",
+        self.assertRaises(security.SDDLValueError, security.descriptor.from_sddl, "foo",
                           security.dom_sid("S-1-2-3"))
 
     def test_from_sddl_invalidtype1(self):

diff --git a/source4/librpc/ndr/py_security.c b/source4/librpc/ndr/py_security.c
index 14228c79ab41e796b8c72954fb6703a61dec4384..0daeaf7aab36d7def091503afac607b8560d9765 100644 (file)
--- a/source4/librpc/ndr/py_security.c
+++ b/source4/librpc/ndr/py_security.c
@@ -276,6 +276,8 @@ static PyObject *py_descriptor_from_sddl(PyObject *self, PyObject *args)
        char *sddl;
        PyObject *py_sid;
        struct dom_sid *sid;
+       const char *err_msg = NULL;
+       size_t err_msg_offset = 0;
 
        if (!PyArg_ParseTuple(args, "sO!", &sddl, &dom_sid_Type, &py_sid))
                return NULL;
@@ -289,9 +291,35 @@ static PyObject *py_descriptor_from_sddl(PyObject *self, PyObject *args)
 
        sid = pytalloc_get_ptr(py_sid);
 
-       secdesc = sddl_decode(NULL, sddl, sid);
+       secdesc = sddl_decode_err_msg(NULL, sddl, sid,
+                                     &err_msg, &err_msg_offset);
        if (secdesc == NULL) {
-               PyErr_SetString(PyExc_ValueError, "Unable to parse SDDL");
+               PyObject *exc = NULL;
+               if (err_msg == NULL) {
+                       err_msg = "unknown error";
+               }
+               /*
+                * Some notes about this exception value:
+                *
+                * We don't want to add the offset first, so as not to
+                * confuse those who are used to the integer error
+                * code coming first.
+                *
+                * The errant sddl is added so that the exception can
+                * be caught some distance away from the call and we
+                * still know what the messages refer to.
+                */
+               exc = Py_BuildValue("(s, s, i, s)",
+                                   "Unable to parse SDDL",
+                                   err_msg,
+                                   err_msg_offset,
+                                   sddl);
+               if (exc == NULL) {
+                       /* an exception was set by Py_BuildValue() */
+                       return NULL;
+               }
+               PyErr_SetObject(PyExc_SDDLValueError, exc);
+               Py_DECREF(exc);
                return NULL;
        }