You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@activemq.apache.org by "juan.buireo" <ju...@mulesoft.com> on 2019/04/08 18:15:54 UTC

Comparator implementation of HierarchicalObjectRepository issue

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&lt;T>> possibleMatches =
getPossibleMatches(modifiedMatch);
         Collection<Match&lt;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. 



--
Sent from: http://activemq.2283324.n4.nabble.com/ActiveMQ-User-f2341805.html

Re: Comparator implementation of HierarchicalObjectRepository issue

Posted by "Pablo.rodriguez" <Qu...@gmail.Com>.
We are currently facing the same issue with the compare algorithm in our
organizarion. Is there an ETA on the fix?



--
Sent from: http://activemq.2283324.n4.nabble.com/ActiveMQ-User-f2341805.html

Re: Comparator implementation of HierarchicalObjectRepository issue

Posted by Justin Bertram <jb...@apache.org>.
I just stumbled across this and wanted to follow up for posterity's sake.

This issue has been resolved in ActiveMQ Artemis 2.17.0 via ARTEMIS-3068
[1].


Justin

[1] https://issues.apache.org/jira/browse/ARTEMIS-3068

On Mon, Apr 8, 2019 at 1:26 PM juan.buireo <ju...@mulesoft.com> wrote:

> 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&lt;T>> possibleMatches =
> getPossibleMatches(modifiedMatch);
>          Collection<Match&lt;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.
>
>
>
> --
> Sent from:
> http://activemq.2283324.n4.nabble.com/ActiveMQ-User-f2341805.html
>