DISPATCH-990: Reject add of vhost that has pattern conflict. Add self test showing how conflicts are managed.
Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/9c0a09d7 Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/9c0a09d7 Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/9c0a09d7 Branch: refs/heads/master Commit: 9c0a09d793795acbc165df37a45f8fa86329da0f Parents: f17631c Author: Chuck Rolke <[email protected]> Authored: Mon May 14 12:31:13 2018 -0400 Committer: Chuck Rolke <[email protected]> Committed: Mon May 14 12:31:13 2018 -0400 ---------------------------------------------------------------------- python/qpid_dispatch_internal/dispatch.py | 2 +- .../policy/policy_local.py | 12 ++-- src/dispatch.c | 4 +- src/policy.c | 14 +++- src/policy.h | 8 ++- tests/parse_tree_tests.c | 67 ++++++++++++++++++++ 6 files changed, 93 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/9c0a09d7/python/qpid_dispatch_internal/dispatch.py ---------------------------------------------------------------------- diff --git a/python/qpid_dispatch_internal/dispatch.py b/python/qpid_dispatch_internal/dispatch.py index 609396e..c819122 100644 --- a/python/qpid_dispatch_internal/dispatch.py +++ b/python/qpid_dispatch_internal/dispatch.py @@ -78,7 +78,7 @@ 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, None, [self.qd_dispatch_p, c_char_p]) + 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]) http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/9c0a09d7/python/qpid_dispatch_internal/policy/policy_local.py ---------------------------------------------------------------------- diff --git a/python/qpid_dispatch_internal/policy/policy_local.py b/python/qpid_dispatch_internal/policy/policy_local.py index ce47e21..6f8dbb3 100644 --- a/python/qpid_dispatch_internal/policy/policy_local.py +++ b/python/qpid_dispatch_internal/policy/policy_local.py @@ -531,6 +531,11 @@ class PolicyLocal(object): if len(warnings) > 0: for warning in warnings: self._manager.log_warning(warning) + # Reject if parse tree optimized name collision + if self.use_hostname_patterns: + agent = self._manager.get_agent() + if not agent.qd.qd_dispatch_policy_host_pattern_add(agent.dispatch, name): + raise PolicyError("Policy '%s' optimized pattern conflicts with existing pattern" % name) if name not in self.rulesetdb: if name not in self.statsdb: self.statsdb[name] = AppStats(name, self._manager, candidate) @@ -539,13 +544,6 @@ class PolicyLocal(object): self.statsdb[name].update_ruleset(candidate) self._manager.log_info("Updated policy rules for vhost %s" % name) # TODO: ruleset lock - if self.use_hostname_patterns: - agent = self._manager.get_agent() - if name in self.rulesetdb: - # an update. remove existing hostname pattern - agent.qd.qd_dispatch_policy_host_pattern_remove(agent.dispatch, name) - # add new pattern - agent.qd.qd_dispatch_policy_host_pattern_add(agent.dispatch, name) self.rulesetdb[name] = {} self.rulesetdb[name].update(candidate) http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/9c0a09d7/src/dispatch.c ---------------------------------------------------------------------- diff --git a/src/dispatch.c b/src/dispatch.c index 6f82471..3c2629a 100644 --- a/src/dispatch.c +++ b/src/dispatch.c @@ -270,9 +270,9 @@ void qd_dispatch_policy_c_counts_refresh(long ccounts, qd_entity_t *entity) qd_policy_c_counts_refresh(ccounts, entity); } -void qd_dispatch_policy_host_pattern_add(qd_dispatch_t *qd, char *hostPattern) +bool qd_dispatch_policy_host_pattern_add(qd_dispatch_t *qd, char *hostPattern) { - qd_policy_host_pattern_add(qd->policy, hostPattern); + return qd_policy_host_pattern_add(qd->policy, hostPattern); } void qd_dispatch_policy_host_pattern_remove(qd_dispatch_t *qd, char *hostPattern) http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/9c0a09d7/src/policy.c ---------------------------------------------------------------------- diff --git a/src/policy.c b/src/policy.c index 77523ce..dec645e 100644 --- a/src/policy.c +++ b/src/policy.c @@ -834,14 +834,22 @@ bool qd_policy_approve_link_name(const char *username, // Add a hostname to the lookup parse_tree -void qd_policy_host_pattern_add(qd_policy_t *policy, char *hostPattern) +bool qd_policy_host_pattern_add(qd_policy_t *policy, char *hostPattern) { sys_mutex_lock(policy->tree_lock); void *oldp = qd_parse_tree_add_pattern_str(policy->hostname_tree, hostPattern, hostPattern); sys_mutex_unlock(policy->tree_lock); if (oldp) { - qd_log(policy->log_source, QD_LOG_INFO, "vhost hostname pattern '%s' replaced existing pattern", hostPattern); + qd_log(policy->log_source, + QD_LOG_WARNING, + "vhost hostname pattern '%s' failed to replace optimized pattern '%s'", + hostPattern, oldp); + sys_mutex_lock(policy->tree_lock); + void *recovered = qd_parse_tree_add_pattern_str(policy->hostname_tree, (char *)oldp, oldp); + sys_mutex_unlock(policy->tree_lock); + assert (recovered && !strcmp((char *)recovered, hostPattern)); } + return oldp == 0; } @@ -852,7 +860,7 @@ void qd_policy_host_pattern_remove(qd_policy_t *policy, char *hostPattern) void *oldp = qd_parse_tree_remove_pattern_str(policy->hostname_tree, hostPattern); sys_mutex_unlock(policy->tree_lock); if (!oldp) { - qd_log(policy->log_source, QD_LOG_INFO, "vhost hostname pattern '%s' for removal not found", hostPattern); + qd_log(policy->log_source, QD_LOG_WARNING, "vhost hostname pattern '%s' for removal not found", hostPattern); } } http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/9c0a09d7/src/policy.h ---------------------------------------------------------------------- diff --git a/src/policy.h b/src/policy.h index 57a64fe..98500ec 100644 --- a/src/policy.h +++ b/src/policy.h @@ -179,10 +179,16 @@ bool qd_policy_approve_link_name(const char *username, ); /** Add a hostname to the lookup parse_tree + * Note that the parse_tree may store an 'optimised' pattern for a given + * pattern. Thus the patterns a user puts in may collide with existing + * patterns even though the text of the host patterns is different. + * This function does not allow new patterns with thier optimizations + * to overwrite existing patterns that may have been optimised. * @param[in] policy qd_policy_t * @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 */ -void qd_policy_host_pattern_add(qd_policy_t *policy, char *hostPattern); +bool qd_policy_host_pattern_add(qd_policy_t *policy, char *hostPattern); /** Remove a hostname from the lookup parse_tree * @param[in] policy qd_policy_t http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/9c0a09d7/tests/parse_tree_tests.c ---------------------------------------------------------------------- diff --git a/tests/parse_tree_tests.c b/tests/parse_tree_tests.c index 5800b4a..1dba5ba 100644 --- a/tests/parse_tree_tests.c +++ b/tests/parse_tree_tests.c @@ -98,6 +98,72 @@ static char *test_add_and_match_str(void *context) return NULL; } +static char *test_usurpation_recovery_str(void *context) +{ + const char *A = "#"; + const char *B = "#.#.#.#"; + qd_parse_tree_t *node = qd_parse_tree_new(QD_PARSE_TREE_ADDRESS); + void *payload; + void *usurped; + void *deposed; + + // rightful owner is ensconsced + if (qd_parse_tree_add_pattern_str(node, A, (void *)A)) + return "Add returned existing value (1)"; + + // matches on A or B both return A + if (!qd_parse_tree_retrieve_match_str(node, A, &payload)) + return "Could not get pattern"; + + if (!payload || strcmp(A, (char *)payload)) + return "Got bad pattern"; + + if (!qd_parse_tree_retrieve_match_str(node, B, &payload)) + return "Could not get pattern"; + + if (!payload || strcmp(A, (char *)payload)) + return "Got bad pattern"; + + // usurper comes along + usurped = qd_parse_tree_add_pattern_str(node, B, (void *)B); + if (!usurped || strcmp(A, (char *)usurped)) + return "Usurper should have grabbed '#' optimized match"; + + // matches on A or B both return B + if (!qd_parse_tree_retrieve_match_str(node, A, &payload)) + return "Could not get pattern"; + + if (!payload || strcmp(B, (char *)payload)) + return "Got bad pattern"; + + if (!qd_parse_tree_retrieve_match_str(node, B, &payload)) + return "Could not get pattern"; + + if (!payload || strcmp(B, (char *)payload)) + return "Got bad pattern"; + + // Restore rightful owner + deposed = qd_parse_tree_add_pattern_str(node, usurped, usurped); + if (!deposed || strcmp(B, (char *)deposed)) + return "Failed to depose B"; + + // matches on A or B both return A + if (!qd_parse_tree_retrieve_match_str(node, A, &payload)) + return "Could not get pattern"; + + if (!payload || strcmp(A, (char *)payload)) + return "Got bad pattern"; + + if (!qd_parse_tree_retrieve_match_str(node, B, &payload)) + return "Could not get pattern"; + + if (!payload || strcmp(A, (char *)payload)) + return "Got bad pattern"; + + qd_parse_tree_free(node); + return NULL; +} + // for pattern match callback typedef struct { int count; @@ -633,6 +699,7 @@ int parse_tree_tests(void) TEST_CASE(test_add_remove, 0); TEST_CASE(test_add_and_match_str, 0); + TEST_CASE(test_usurpation_recovery_str, 0); TEST_CASE(test_normalization, 0); TEST_CASE(test_matches, 0); TEST_CASE(test_multiple_matches, 0); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
