Qpid
  1. Qpid
  2. QPID-4468

[Java client] restore ability to enable SSL using a connection level option rather than as a sub-option of each brokerlist entry

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.6, 0.8, 0.10, 0.12, 0.14, 0.16, 0.18
    • Fix Version/s: 0.20, 0.21
    • Component/s: Java Client
    • Labels:
      None

      Description

      On old client versions it was possible to specify that SSL should be used at the connection level options. It is now only possible to specify this as part of the brokerlist sub-options (in addition to much finer-grained control over the keystores/truststores and certificates etc to be used). If you are just using the SSL system properties to define they keystore/truststore etc and have multiple brokers in the brokerlist it is actually more convenient to specify ssl=true at the connection level.

      Restoring this will maintain compatibility for drop-in replacement of old clients and also help prevent user confiusion resulting from the URL parsers inability to alert them to the previous ssl configuration going unused.

      It will be restored as an ovveride of ssl option in the brokerlist entries.

        Activity

        Robbie Gemmell created issue -
        Robbie Gemmell made changes -
        Field Original Value New Value
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        Robbie Gemmell added a comment -
        Show
        Robbie Gemmell added a comment - Change made in http://svn.apache.org/viewvc?rev=1413364&view=rev
        Robbie Gemmell made changes -
        Status In Progress [ 3 ] Ready To Review [ 10006 ]
        Hide
        Rob Godfrey added a comment -

        The changes look good, though I wonder a little about the override policy, I would think that you would only want to override where the broker list SSL had not been set at all... having ssl turned on for a broker detail where it was explicitly false (or vice versa) seems weird (though really I think any such URL is just a misconfiguration in itself)

        Show
        Rob Godfrey added a comment - The changes look good, though I wonder a little about the override policy, I would think that you would only want to override where the broker list SSL had not been set at all... having ssl turned on for a broker detail where it was explicitly false (or vice versa) seems weird (though really I think any such URL is just a misconfiguration in itself)
        Rob Godfrey made changes -
        Status Ready To Review [ 10006 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Robbie Gemmell added a comment -

        I did consider doing it that way, but it was looking a little messy (the ConnectionSettings cant convey a lack of SSL option, so you then need to start looking at the BrokerDetails object as well, or switch ConnectionSettings to a Boolean to allow conveying null and add a Boolean getter instead of the boolean isUseSSL) and as you say it smacks of misconfiguration a little (I also considered throwing an Exception because of that). As its not really intended to be used together with the brokerlist option, but rather instead of it, in the end I decided just to document the overriding nature and leave it at that.

        Show
        Robbie Gemmell added a comment - I did consider doing it that way, but it was looking a little messy (the ConnectionSettings cant convey a lack of SSL option, so you then need to start looking at the BrokerDetails object as well, or switch ConnectionSettings to a Boolean to allow conveying null and add a Boolean getter instead of the boolean isUseSSL) and as you say it smacks of misconfiguration a little (I also considered throwing an Exception because of that). As its not really intended to be used together with the brokerlist option, but rather instead of it, in the end I decided just to document the overriding nature and leave it at that.
        Justin Ross made changes -
        Fix Version/s 0.20 [ 12323548 ]
        Hide
        Justin Ross added a comment -

        Reviewed by Rob. Approve for 0.20.

        Show
        Justin Ross added a comment - Reviewed by Rob. Approve for 0.20.
        Hide
        Robbie Gemmell added a comment -

        Now merged to the 0.20 release branch.

        Show
        Robbie Gemmell added a comment - Now merged to the 0.20 release branch.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development