You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@activemq.apache.org by "Clebert Suconic (Jira)" <ji...@apache.org> on 2021/02/09 21:23:04 UTC
[jira] [Closed] (ARTEMIS-3068) Comparator implementation of
HierarchicalObjectRepository issue
[ https://issues.apache.org/jira/browse/ARTEMIS-3068?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Clebert Suconic closed ARTEMIS-3068.
------------------------------------
Resolution: Fixed
> Comparator implementation of HierarchicalObjectRepository issue
> ---------------------------------------------------------------
>
> Key: ARTEMIS-3068
> URL: https://issues.apache.org/jira/browse/ARTEMIS-3068
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Affects Versions: 2.9.0, 2.10.0, 2.10.1, 2.11.0, 2.12.0, 2.13.0, 2.14.0, 2.15.0, 2.16.0
> Reporter: Luis Miguel De Bello
> Priority: Major
> Fix For: 2.17.0
>
> Time Spent: 2h 40m
> Remaining Estimate: 0h
>
> Hi, I am having an issue with the MatchComparator implemented in the
> HierarchicalObjectRepository class.
> Let me put an example on what's going on right now:
> I have created a class called DynamicRolesSecuritySettingPlugin implementing
> the interface SecuritySettingPlugin so that I can implement the
> setSecurityRepository method. There, I am adding some roles to the
> HierarchicalRepository based on a certificate. The HierarchicalRepository
> type T is Set<Role>. To summarize, the HierarchicalObjectRepository ends up
> having two different strings as key to the internal map (they are added with
> the addMatch method):
> - *.Provider.*.Agent.*.State
> - uswest.Provider.RR.Agent.*.State
> For the string "*.Provider.*.Agent.*.State", i end up setting a Role with
> permissions to publish but not to consume.
> For the string "uswest.Provider.RR.Agent.*.State", i end up setting a Role
> with permissions to consume.
> Now, when I create a consumer for a queue named
> uswest.Provider.RR.Agent.1.State (there is already a queue created with that
> name and with messages on it), I get an error on the consumer saying that I
> don't have the permission 'CONSUME' for queue
> uswest.Provider.RR.Agent.1.State.
> So I went ahead and debugged the code. I ended up in the class
> HierarchicalObjectRepository reading the getMatch method:
> @Override
> public T getMatch(final String match) {
> String modifiedMatch = matchModifier.modify(match);
> T cacheResult = cache.get(modifiedMatch);
> if (cacheResult != null) {
> return cacheResult;
> }
> lock.readLock().lock();
> try {
> T actualMatch;
> Map<String, Match<T>> possibleMatches =
> getPossibleMatches(modifiedMatch);
> Collection<Match<T>> orderedMatches = sort(possibleMatches);
> actualMatch = merge(orderedMatches);
> T value = actualMatch != null ? actualMatch : defaultmatch;
> if (value != null) {
> cache.put(modifiedMatch, value);
> }
> return value;
> } finally {
> lock.readLock().unlock();
> }
> }
> When I set a breakpoint in the getPossibleMatches I get the two entries
> correctly (both "*.Provider.*.Agent.*.State" and
> "uswest.Provider.RR.Agent.*.State"). Then when i see the sort method, i see
> that I do get first the "*.Provider.*.Agent.*.State" instead of the
> "uswest.Provider.RR.Agent.*State". As I said before, the role attached to
> "*.Provider.*.Agent.*.State" does not have permission to consume on the
> queue "uswest.Provider.RR.Agent.1.State" and that is the reason why I can't
> consume.
> When I looked at the comparator (MatchComparator) that decides which one
> should be first in the list, I see the following code:
> if (o1.contains(anyWords) && !o2.contains(anyWords)) {
> return +1;
> } else if (!o1.contains(anyWords) && o2.contains(anyWords)) {
> return -1;
> } else if (o1.contains(anyWords) && o2.contains(anyWords)) {
> return o2.length() - o1.length();
> } else if (o1.contains(singleWord) && !o2.contains(singleWord)) {
> return +1;
> } else if (!o1.contains(singleWord) && o2.contains(singleWord)) {
> return -1;
> } else if (o1.contains(singleWord) && o2.contains(singleWord)) {
> String[] leftSplits = o1.split(quotedDelimiter);
> String[] rightSplits = o2.split(quotedDelimiter);
> for (int i = 0; i < leftSplits.length; i++) {
> String left = leftSplits[i];
> if (left.equals(singleWord)) {
> if (rightSplits.length < i ||
> !rightSplits[i].equals(singleWord)) {
> return -1;
> } else {
> return +1;
> }
> }
> }
> }
> return o1.length() - o2.length();
> Since in this case both strings contains the "*" it goes through the last
> else if and this is what ends up doing the comparation:
>
> String[] leftSplits = o1.split(quotedDelimiter);
> String[] rightSplits = o2.split(quotedDelimiter);
> for (int i = 0; i < leftSplits.length; i++) {
> String left = leftSplits[i];
> if (left.equals(singleWord)) {
> if (rightSplits.length < i ||
> !rightSplits[i].equals(singleWord)) {
> return -1;
> } else {
> return +1;
> }
> }
> }
> This ends up returning the "*.Provider.*.Agent.*.State" when it shouldn't.
> There is a more specific string for the queue
> "uswest.Provider.RR.Agent.1.State".
> It's not taking into account the amount of "*" present in the string.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)