Qpid
  1. Qpid
  2. QPID-3156

Java client implementation of the Address syntax for Destinations throws a ClassCastException when used with the documented 'True' or 'False' values for node durability

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.6, 0.7, 0.8, 0.9, 0.10
    • Fix Version/s: 0.11
    • Component/s: Java Client
    • Labels:
      None

      Description

      The Java client implementation of the Address syntax for destinations throws a ClassCastException when used with the documented (http://qpid.apache.org/books/0.8/Programming-In-Apache-Qpid/html/ch02s04.html#section-address-string-bnf) 'True' or 'False' values for node durability, instead requiring that users specify 'true' or 'false' as the value to achieve the desired effect. The former is parsed as a boolean value by the AddressParser whereas the later is parsed as a String, but the AddressHelper implementation expects the durable property to be defined as a String and casts the value as such without doing an instanceof check after retrieving it from an untyped Map, thus leading to the ClassCastException observed.

      Ideally I would suggest the parser/helper should accept both variations going forward, thus preserving compatibility for anyone with already functional code and also making it possible for users to use whichever variant they prefer because e.g. it is best suited to a particular implementation language idiom.

      Reported by a user via the dev list:

      I tried to create a durable queue like this

      queue = queueSession.createQueue("myQueue;{create:always, node:{durable:
      True}}");
      QueueSender queueSender = queueSession.createSender(queue);
      queueSender.send(textMessage);

      and getting this exception.

      Caused by: java.lang.ClassCastException: java.lang.Boolean cannot be cast to java.lang.String
      at
      org.apache.qpid.client.messaging.address.AddressHelper.getDurability(AddressHelper.java:237)
      at
      org.apache.qpid.client.messaging.address.AddressHelper.fillInCommonNodeArgs(AddressHelper.java:222)
      at
      org.apache.qpid.client.messaging.address.AddressHelper.createQueueNode(AddressHelper.java:215)
      at
      org.apache.qpid.client.messaging.address.AddressHelper.getSourceNode(AddressHelper.java:254)
      at
      org.apache.qpid.client.AMQDestination.rebuildTargetAndSourceNodes(AMQDestination.java:888)
      at
      org.apache.qpid.client.AMQSession_0_10.resolveAddressType(AMQSession_0_10.java:1272)

      thanks,
      Amila.

        Activity

        Hide
        Robbie Gemmell added a comment -

        Closing out, changes were committed long ago to address this.

        Show
        Robbie Gemmell added a comment - Closing out, changes were committed long ago to address this.
        Hide
        Rajith Attapattu added a comment -

        As mentioned I am neutral about including it for 0.10 as there is clearly an alternative to get this working.
        However we need to release note this.

        Show
        Rajith Attapattu added a comment - As mentioned I am neutral about including it for 0.10 as there is clearly an alternative to get this working. However we need to release note this.
        Hide
        Rajith Attapattu added a comment -

        Andrew,

        I introduced MapAccessor precisely for that reason, and should have used it.
        However I plan to clean up the code a bit more in this area and I am hoping to commit that in the coming days.
        Will definitely ensure that the proper methods are used.

        Rajith

        Show
        Rajith Attapattu added a comment - Andrew, I introduced MapAccessor precisely for that reason, and should have used it. However I plan to clean up the code a bit more in this area and I am hoping to commit that in the coming days. Will definitely ensure that the proper methods are used. Rajith
        Hide
        Justin Ross added a comment -

        Robbie pointed out on the dev list that this behavior has existed for two releases. To me, that makes the evident level of user gotcha much less. Therefore the incentive to merge this to 0.10 is much less as well. Rejected for 0.10.

        Show
        Justin Ross added a comment - Robbie pointed out on the dev list that this behavior has existed for two releases. To me, that makes the evident level of user gotcha much less. Therefore the incentive to merge this to 0.10 is much less as well. Rejected for 0.10.
        Hide
        Andrew Kennedy added a comment -

        Use existing MapAccessor code to retreive the Boolean property, as in the rest of AddressHelper, rather than duplicating the logic.

        If this is to be included in 0.10, I suggest we use this patch instead.

        Show
        Andrew Kennedy added a comment - Use existing MapAccessor code to retreive the Boolean property, as in the rest of AddressHelper, rather than duplicating the logic. If this is to be included in 0.10, I suggest we use this patch instead.
        Hide
        Justin Ross added a comment -

        In my opinion, this is a worthy fix for a user gotcha and a low risk change. I tilt in favor of merging this to 0.10, but I am keen to hear what others think.

        Show
        Justin Ross added a comment - In my opinion, this is a worthy fix for a user gotcha and a low risk change. I tilt in favor of merging this to 0.10, but I am keen to hear what others think.
        Hide
        Rajith Attapattu added a comment -

        I've committed a fix to this on trunk. Should we port this to the 0.10 branch as well ?
        IMO this is not critical as if 'true' is used instead of 'True' it will work.
        We could probably release note this issue and get away with it.

        Show
        Rajith Attapattu added a comment - I've committed a fix to this on trunk. Should we port this to the 0.10 branch as well ? IMO this is not critical as if 'true' is used instead of 'True' it will work. We could probably release note this issue and get away with it.

          People

          • Assignee:
            Unassigned
            Reporter:
            Robbie Gemmell
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development