Camel
  1. Camel
  2. CAMEL-4904

org.apache.camel.util.concurrent.ExecutorServiceHelper.getThreadName(String, String) method throws IllegalArgumentException when name parameter contains '$', "${" or '}'

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.8.3, 2.9.0
    • Fix Version/s: 2.8.4, 2.9.1
    • Component/s: camel-core
    • Labels:
      None
    • Estimated Complexity:
      Novice

      Description

      When the "name" parameter contains a '$', "$

      {" or '}

      ' substring, the getThreadName method will throw an erroneous IllegalArgumentException.

        Activity

        Hide
        Claus Ibsen added a comment -

        Thanks for reporting. I fixed that on trunk yesterday with CAMEL-4903.
        Will have to manually backport a fix, for the older branches as the trunk changed. Basically you need to quote the names, so the replace works.

        Show
        Claus Ibsen added a comment - Thanks for reporting. I fixed that on trunk yesterday with CAMEL-4903 . Will have to manually backport a fix, for the older branches as the trunk changed. Basically you need to quote the names, so the replace works.
        Hide
        Claus Ibsen added a comment -

        Stephen if you wish to work on a patch, then you can take a look at this commit
        http://svn.apache.org/viewvc?rev=1232013&view=rev

        Notice the quote replacement code.

        Show
        Claus Ibsen added a comment - Stephen if you wish to work on a patch, then you can take a look at this commit http://svn.apache.org/viewvc?rev=1232013&view=rev Notice the quote replacement code.
        Hide
        Babak Vahdat added a comment -

        Claus not that much important at all but I commented a svn-typo by CAMEL-4903

        Show
        Babak Vahdat added a comment - Claus not that much important at all but I commented a svn-typo by CAMEL-4903
        Hide
        Claus Ibsen added a comment -

        Babak

        In due time you will be able to make the commit log changes yourself

        Its not really that important, and just risk something odd **** up with svn/git / syncing and whatnot.
        So I would just leave it as is.

        Show
        Claus Ibsen added a comment - Babak In due time you will be able to make the commit log changes yourself Its not really that important, and just risk something odd **** up with svn/git / syncing and whatnot. So I would just leave it as is.
        Hide
        Babak Vahdat added a comment -

        fair enough

        Show
        Babak Vahdat added a comment - fair enough
        Hide
        Claus Ibsen added a comment -

        Fixed on 2.8 branch as well.

        Show
        Claus Ibsen added a comment - Fixed on 2.8 branch as well.
        Hide
        Stephen Saucier added a comment -
        Show
        Stephen Saucier added a comment - I am confused – the bug details indicate that this fix should be present in 2.9.1 and 2.8.4, but as far as I can tell, it isn't in either. I am looking at https://svn.apache.org/repos/asf/camel/tags/camel-2.8.4/camel-core/src/main/java/org/apache/camel/util/concurrent/ExecutorServiceHelper.java and https://svn.apache.org/repos/asf/camel/tags/camel-2.9.1/camel-core/src/main/java/org/apache/camel/util/concurrent/ThreadHelper.java What is in trunk seems okay: https://svn.apache.org/repos/asf/camel/trunk/camel-core/src/main/java/org/apache/camel/util/concurrent/ThreadHelper.java Am I missing something? Testing with 2.9.1, my bug still exists.
        Hide
        Claus Ibsen added a comment -

        The fix works fine as we use this quote the matcher before using

        
            // must quote the names to have it work as literal replacement
                shortName = Matcher.quoteReplacement(shortName);
                longName = Matcher.quoteReplacement(longName);
        

        However note that the syntax in Camel 2.10 has changed to use # instead of $ as tokens. This avoids any confusions with property placeholders, simple language etc. The 2.9 and older releases uses the $ to not break compatibility.

        Show
        Claus Ibsen added a comment - The fix works fine as we use this quote the matcher before using // must quote the names to have it work as literal replacement shortName = Matcher.quoteReplacement(shortName); longName = Matcher.quoteReplacement(longName); However note that the syntax in Camel 2.10 has changed to use # instead of $ as tokens. This avoids any confusions with property placeholders, simple language etc. The 2.9 and older releases uses the $ to not break compatibility.
        Hide
        Stephen Saucier added a comment -

        Could you please check again the links I posted previously? The code that is included in 2.8.4 and 2.9.1 does not quote the replacement, and still exhibits the issue as it is still checking after the replacement that that string does not contain a '}' character which my name does. This is working in trunk due to the use of a regular expression (and the fact that '#' is being used as the delimiter for placeholders, which are not contained in my particular names).

        Show
        Stephen Saucier added a comment - Could you please check again the links I posted previously? The code that is included in 2.8.4 and 2.9.1 does not quote the replacement, and still exhibits the issue as it is still checking after the replacement that that string does not contain a '}' character which my name does. This is working in trunk due to the use of a regular expression (and the fact that '#' is being used as the delimiter for placeholders, which are not contained in my particular names).
        Hide
        Claus Ibsen added a comment -

        Then I suggest to not use { } in your thread names. Its very unusual.

        Show
        Claus Ibsen added a comment - Then I suggest to not use { } in your thread names. Its very unusual.

          People

          • Assignee:
            Claus Ibsen
            Reporter:
            Stephen Saucier
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development