ActiveMQ
  1. ActiveMQ
  2. AMQ-3209

URISupport.createURIWithQuery() fails on some composite uris.

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 5.5.0
    • Fix Version/s: 5.5.0
    • Component/s: None
    • Labels:
    • Patch Info:
      Patch Available

      Description

      URISupport.createURIWithQuery() fails on composite URIs that have an inner query but not an outer query, e.g.:

      outerscheme:(innerscheme:innerssp?innerquery=0)

      The failure is due to the method not taking into account this possibility and assuming the query consists of everything after the last question mark. The attached patch adds a test case in org.apache.activemq.util.URISupportTest that demonstrates the problem, and also modifies URISupport.createURIWithQuery() with a suggested fix for the problem.

      1. URISupport.patch
        4 kB
        Aja Walker
      2. URISupport.patch
        3 kB
        Aja Walker

        Activity

        Aja Walker created issue -
        Aja Walker made changes -
        Field Original Value New Value
        Attachment URISupportTest.java.diff [ 12472877 ]
        Aja Walker made changes -
        Attachment URISupportTest.java.diff [ 12472877 ]
        Aja Walker made changes -
        Description URISupport.createURIWithQuery() fails on composite URIs that have an inner query but not an outer query, e.g.:

        outerscheme:(innerscheme:innerssp?innerquery=0)

        The failure is due to the method not taking into account this possibility and assuming the query consists of everything after the last question mark. The attached patch is for a test case in org.apache.activemq.util.URISupportTest that demonstrates the problem.
        URISupport.createURIWithQuery() fails on composite URIs that have an inner query but not an outer query, e.g.:

        outerscheme:(innerscheme:innerssp?innerquery=0)

        The failure is due to the method not taking into account this possibility and assuming the query consists of everything after the last question mark. The attached patch adds a test case in org.apache.activemq.util.URISupportTest that demonstrates the problem, and also modifies URISupport.createURIWithQuery() with a suggested fix for the problem.
        Aja Walker made changes -
        Attachment URISupport.patch [ 12472970 ]
        Hide
        Timothy Bish added a comment -

        Looking at your patch I think it probably would suffice to check if the "?" lies beyond the last parenthesis instead of searching for the matching set, what do you think?

        Also the assert statements in the unit tests have the arguments backwards, the expected result should be first, otherwise the error message is a bit confusing.

        Show
        Timothy Bish added a comment - Looking at your patch I think it probably would suffice to check if the "?" lies beyond the last parenthesis instead of searching for the matching set, what do you think? Also the assert statements in the unit tests have the arguments backwards, the expected result should be first, otherwise the error message is a bit confusing.
        Hide
        Aja Walker added a comment -

        Timothy, yes, you are right I think. We should only have to check for the last parenthesis. I was worried about getting confused on an orphan closing parenthesis, but in reality that would seem to indicate a malformed URI, and so in that case we shouldn't have to guarantee any particular behavior.

        I've created a new patch that only checks for closing parenthesis and also fixes the backwards assert statements in the unit tests. I'm going to upload this new patch over the previous one.

        Show
        Aja Walker added a comment - Timothy, yes, you are right I think. We should only have to check for the last parenthesis. I was worried about getting confused on an orphan closing parenthesis, but in reality that would seem to indicate a malformed URI, and so in that case we shouldn't have to guarantee any particular behavior. I've created a new patch that only checks for closing parenthesis and also fixes the backwards assert statements in the unit tests. I'm going to upload this new patch over the previous one.
        Aja Walker made changes -
        Attachment URISupport.patch [ 12473138 ]
        Hide
        Timothy Bish added a comment -

        Patch applied, thanks.

        Show
        Timothy Bish added a comment - Patch applied, thanks.
        Timothy Bish made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Assignee Timothy Bish [ tabish121 ]
        Fix Version/s 5.5.0 [ 12315626 ]
        Resolution Fixed [ 1 ]

          People

          • Assignee:
            Timothy Bish
            Reporter:
            Aja Walker
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development