Repository: qpid-dispatch Updated Branches: refs/heads/master 550c6711d -> 69c613413
DISPATCH-1046: fixes for running the policy code under python3 This closes #334 Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/69c61341 Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/69c61341 Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/69c61341 Branch: refs/heads/master Commit: 69c61341344ccf737023e4a473f0f1e62ac75b2b Parents: 550c671 Author: Kenneth Giusti <kgiu...@apache.org> Authored: Mon Jun 25 12:05:12 2018 -0400 Committer: Kenneth Giusti <kgiu...@apache.org> Committed: Thu Jun 28 13:44:41 2018 -0400 ---------------------------------------------------------------------- python/qpid_dispatch_internal/dispatch.py | 32 +++++++++++----------- src/dispatch.c | 20 +++++++++----- src/policy.c | 37 +++++++++++++++++++++----- src/policy.h | 6 ++--- tests/system_tests_policy.py | 13 ++++----- 5 files changed, 71 insertions(+), 37 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/69c61341/python/qpid_dispatch_internal/dispatch.py ---------------------------------------------------------------------- diff --git a/python/qpid_dispatch_internal/dispatch.py b/python/qpid_dispatch_internal/dispatch.py index 34ac342..ddbb874 100644 --- a/python/qpid_dispatch_internal/dispatch.py +++ b/python/qpid_dispatch_internal/dispatch.py @@ -38,7 +38,7 @@ from __future__ import print_function import sys, ctypes from ctypes import c_char_p, c_long, py_object import qpid_dispatch_site -from .compat import UNICODE +from .compat import IS_PY2 class CError(Exception): """Exception raised if there is an error in a C call""" @@ -60,9 +60,6 @@ class QdDll(ctypes.PyDLL): # No check on qd_error_* functions, it would be recursive self._prototype(self.qd_error_code, c_long, [], check=False) self._prototype(self.qd_error_message, c_char_p, [], check=False) - # in python3 c_char_p returns a byte type for the error message. We - # need to convert that to a unicode string: - self.qd_error_message.errcheck = lambda x,y,z : UNICODE(x) self._prototype(self.qd_log_entity, c_long, [py_object]) self._prototype(self.qd_dispatch_configure_router, None, [self.qd_dispatch_p, py_object]) self._prototype(self.qd_dispatch_prepare, None, [self.qd_dispatch_p]) @@ -86,10 +83,9 @@ class QdDll(ctypes.PyDLL): self._prototype(self.qd_dispatch_policy_c_counts_alloc, c_long, [], check=False) self._prototype(self.qd_dispatch_policy_c_counts_free, None, [c_long], check=False) self._prototype(self.qd_dispatch_policy_c_counts_refresh, None, [c_long, py_object]) - self._prototype(self.qd_dispatch_policy_host_pattern_add, ctypes.c_bool, [self.qd_dispatch_p, c_char_p]) - self._prototype(self.qd_dispatch_policy_host_pattern_remove, None, [self.qd_dispatch_p, c_char_p]) - self._prototype(self.qd_dispatch_policy_host_pattern_lookup, c_char_p, [self.qd_dispatch_p, c_char_p]) - + self._prototype(self.qd_dispatch_policy_host_pattern_add, ctypes.c_bool, [self.qd_dispatch_p, py_object]) + self._prototype(self.qd_dispatch_policy_host_pattern_remove, None, [self.qd_dispatch_p, py_object]) + self._prototype(self.qd_dispatch_policy_host_pattern_lookup, c_char_p, [self.qd_dispatch_p, py_object]) self._prototype(self.qd_dispatch_register_display_name_service, None, [self.qd_dispatch_p, py_object]) @@ -106,16 +102,22 @@ class QdDll(ctypes.PyDLL): self._prototype(self.qd_log_recent_py, py_object, [c_long]) - def _errcheck(self, result, func, args): - if self.qd_error_code(): - raise CError(self.qd_error_message()) - return result - def _prototype(self, f, restype, argtypes, check=True): - """Set up the return and argument types and the error checker for a ctypes function""" + """Set up the return and argument types and the error checker for a + ctypes function""" + + def _do_check(result, func, args): + if check and self.qd_error_code(): + raise CError(self.qd_error_message()) + if restype is c_char_p and result and not IS_PY2: + # in python3 c_char_p returns a byte type for the error + # message. We need to convert that to a string + result = result.decode('utf-8') + return result + f.restype = restype f.argtypes = argtypes - if check: f.errcheck = self._errcheck + f.errcheck = _do_check return f def function(self, fname, restype, argtypes, check=True): http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/69c61341/src/dispatch.c ---------------------------------------------------------------------- diff --git a/src/dispatch.c b/src/dispatch.c index 3c2629a..2966bf3 100644 --- a/src/dispatch.c +++ b/src/dispatch.c @@ -17,7 +17,7 @@ * under the License. */ -#include <Python.h> +#include "python_private.h" #include <qpid/dispatch/python_embedded.h> #include <qpid/dispatch.h> #include <qpid/dispatch/server.h> @@ -270,19 +270,27 @@ void qd_dispatch_policy_c_counts_refresh(long ccounts, qd_entity_t *entity) qd_policy_c_counts_refresh(ccounts, entity); } -bool qd_dispatch_policy_host_pattern_add(qd_dispatch_t *qd, char *hostPattern) +bool qd_dispatch_policy_host_pattern_add(qd_dispatch_t *qd, void *py_obj) { - return qd_policy_host_pattern_add(qd->policy, hostPattern); + char *hostPattern = py_string_2_c(py_obj); + bool rc = qd_policy_host_pattern_add(qd->policy, hostPattern); + free(hostPattern); + return rc; } -void qd_dispatch_policy_host_pattern_remove(qd_dispatch_t *qd, char *hostPattern) +void qd_dispatch_policy_host_pattern_remove(qd_dispatch_t *qd, void *py_obj) { + char *hostPattern = py_string_2_c(py_obj); qd_policy_host_pattern_remove(qd->policy, hostPattern); + free(hostPattern); } -char * qd_dispatch_policy_host_pattern_lookup(qd_dispatch_t *qd, char *hostPattern) +char * qd_dispatch_policy_host_pattern_lookup(qd_dispatch_t *qd, void *py_obj) { - return qd_policy_host_pattern_lookup(qd->policy, hostPattern); + char *hostPattern = py_string_2_c(py_obj); + char *rc = qd_policy_host_pattern_lookup(qd->policy, hostPattern); + free(hostPattern); + return rc; } qd_error_t qd_dispatch_prepare(qd_dispatch_t *qd) http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/69c61341/src/policy.c ---------------------------------------------------------------------- diff --git a/src/policy.c b/src/policy.c index 2f8aef1..a483fcd 100644 --- a/src/policy.c +++ b/src/policy.c @@ -65,6 +65,8 @@ static const char * const user_subst_i_embed = "e"; static const char * const user_subst_i_suffix = "s"; static const char * const user_subst_i_wildcard = "*"; +static void hostname_tree_free(qd_parse_tree_t *hostname_tree); + // // Policy configuration/statistics management interface // @@ -110,9 +112,9 @@ void qd_policy_free(qd_policy_t *policy) { if (policy->policyDir) free(policy->policyDir); - qd_parse_tree_free(policy->hostname_tree); if (policy->tree_lock) sys_mutex_free(policy->tree_lock); + hostname_tree_free(policy->hostname_tree); free(policy); } @@ -1048,39 +1050,44 @@ bool qd_policy_approve_link_name(const char *username, // Add a hostname to the lookup parse_tree -bool qd_policy_host_pattern_add(qd_policy_t *policy, char *hostPattern) +bool qd_policy_host_pattern_add(qd_policy_t *policy, const char *hostPattern) { + void *payload = strdup(hostPattern); sys_mutex_lock(policy->tree_lock); - void *oldp = qd_parse_tree_add_pattern_str(policy->hostname_tree, hostPattern, hostPattern); + void *oldp = qd_parse_tree_add_pattern_str(policy->hostname_tree, hostPattern, payload); if (oldp) { void *recovered = qd_parse_tree_add_pattern_str(policy->hostname_tree, (char *)oldp, oldp); assert (recovered); (void)recovered; /* Silence compiler complaints of unused variable */ } sys_mutex_unlock(policy->tree_lock); - if (oldp) + if (oldp) { + free(payload); qd_log(policy->log_source, QD_LOG_WARNING, "vhost hostname pattern '%s' failed to replace optimized pattern '%s'", hostPattern, oldp); + } return oldp == 0; } // Remove a hostname from the lookup parse_tree -void qd_policy_host_pattern_remove(qd_policy_t *policy, char *hostPattern) +void qd_policy_host_pattern_remove(qd_policy_t *policy, const char *hostPattern) { sys_mutex_lock(policy->tree_lock); void *oldp = qd_parse_tree_remove_pattern_str(policy->hostname_tree, hostPattern); sys_mutex_unlock(policy->tree_lock); - if (!oldp) { + if (oldp) { + free(oldp); + } else { qd_log(policy->log_source, QD_LOG_WARNING, "vhost hostname pattern '%s' for removal not found", hostPattern); } } // Look up a hostname in the lookup parse_tree -char * qd_policy_host_pattern_lookup(qd_policy_t *policy, char *hostPattern) +char * qd_policy_host_pattern_lookup(qd_policy_t *policy, const char *hostPattern) { void *payload = 0; sys_mutex_lock(policy->tree_lock); @@ -1095,6 +1102,22 @@ char * qd_policy_host_pattern_lookup(qd_policy_t *policy, char *hostPattern) } +// free the hostname parse tree and associated resources +// +static bool _hostname_tree_free_payload(void *handle, + const char *pattern, + void *payload) +{ + free(payload); + return true; +} + +static void hostname_tree_free(qd_parse_tree_t *hostname_tree) +{ + qd_parse_tree_walk(hostname_tree, _hostname_tree_free_payload, NULL); + qd_parse_tree_free(hostname_tree); +} + // Convert naked CSV allow list into parsed settings 3-tuple // Note that this logic is also present in python compile_app_settings. char * qd_policy_compile_allowed_csv(char * csv) http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/69c61341/src/policy.h ---------------------------------------------------------------------- diff --git a/src/policy.h b/src/policy.h index c24ad6b..a1d415a 100644 --- a/src/policy.h +++ b/src/policy.h @@ -188,20 +188,20 @@ bool qd_policy_approve_link_name(const char *username, * @param[in] hostPattern the hostname pattern with possible parse_tree wildcards * @return True if the possibly optimised pattern was added to the lookup parse tree */ -bool qd_policy_host_pattern_add(qd_policy_t *policy, char *hostPattern); +bool qd_policy_host_pattern_add(qd_policy_t *policy, const char *hostPattern); /** Remove a hostname from the lookup parse_tree * @param[in] policy qd_policy_t * @param[in] hostPattern the hostname pattern with possible parse_tree wildcards */ -void qd_policy_host_pattern_remove(qd_policy_t *policy, char *hostPattern); +void qd_policy_host_pattern_remove(qd_policy_t *policy, const char *hostPattern); /** Look up a hostname in the lookup parse_tree * @param[in] policy qd_policy_t * @param[in] hostname a concrete vhost name * @return the name of the ruleset whose hostname pattern matched this actual hostname */ -char * qd_policy_host_pattern_lookup(qd_policy_t *policy, char *hostPattern); +char * qd_policy_host_pattern_lookup(qd_policy_t *policy, const char *hostPattern); /** * Compile raw CSV spec of allowed sources/targets and return http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/69c61341/tests/system_tests_policy.py ---------------------------------------------------------------------- diff --git a/tests/system_tests_policy.py b/tests/system_tests_policy.py index 9f462d2..1951adb 100644 --- a/tests/system_tests_policy.py +++ b/tests/system_tests_policy.py @@ -817,8 +817,8 @@ class PolicyLinkNamePatternTest(TestCase): qdm_out = self.run_qdmanage('create --type=vhost --name=DISPATCH-1993-3 --stdin', input=self.disallowed_source_pattern1()) except Exception as e: exception = True - self.assertTrue("InternalServerErrorStatus: PolicyError:" in e.message) - self.assertTrue("Policy 'DISPATCH-1993-3' is invalid:" in e.message) + self.assertTrue("InternalServerErrorStatus: PolicyError:" in str(e)) + self.assertTrue("Policy 'DISPATCH-1993-3' is invalid:" in str(e)) self.assertTrue(exception) # attempt another create that should be rejected - name subst must be prefix or suffix @@ -828,8 +828,8 @@ class PolicyLinkNamePatternTest(TestCase): qdm_out = self.run_qdmanage('create --type=vhost --name=DISPATCH-1993-3 --stdin', input=self.disallowed_source_pattern2()) except Exception as e: exception = True - self.assertTrue("InternalServerErrorStatus: PolicyError:" in e.message) - self.assertTrue("Policy 'DISPATCH-1993-3' is invalid:" in e.message) + self.assertTrue("InternalServerErrorStatus: PolicyError:" in str(e)) + self.assertTrue("Policy 'DISPATCH-1993-3' is invalid:" in str(e)) self.assertTrue(exception) @@ -861,7 +861,8 @@ class PolicyHostamePatternTest(TestCase): def run_qdmanage(self, cmd, input=None, expect=Process.EXIT_OK): p = self.popen( ['qdmanage'] + cmd.split(' ') + ['--bus', re.sub(r'amqp://', 'amqp://u1:password@', self.address()), '--indent=-1', '--timeout', str(TIMEOUT)], - stdin=PIPE, stdout=PIPE, stderr=STDOUT, expect=expect) + stdin=PIPE, stdout=PIPE, stderr=STDOUT, expect=expect, + universal_newlines=True) out = p.communicate(input)[0] try: p.teardown() @@ -904,7 +905,7 @@ class PolicyHostamePatternTest(TestCase): try: qdm_out = self.run_qdmanage('create --type=vhost --name=#.#.0.0 --stdin', input=self.disallowed_hostname()) except Exception as e: - self.assertTrue("pattern conflicts" in e.message, msg=('Error running qdmanage %s' % e.message)) + self.assertTrue("pattern conflicts" in str(e), msg=('Error running qdmanage %s' % str(e))) self.assertFalse("222222" in qdm_out) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org