[QPID-3832] Qpid 0.14 broke transport connection setting

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 0.14
• Fix Version/s: 0.15
• Component/s: C++ Client
• Labels: SSL
• Assignee: Gordon Sim
• Reporter: Paul Colby
• Created: 13/Feb/12 01:15
• Updated: 29/Jul/13 18:53
• Resolved: 01/Mar/12 06:40

Description

The transport connection setting was broken in r1156262.

Consider the following test code:

#include <qpid/messaging/Connection.h>

int main(int argc, char *argv[]) {
    qpid::types::Variant::Map options;
    options["transport" ] = "ssl";
    qpid::messaging::Connection connection("localhost:5671", options);
    connection.open();
    return 0;
}

Using Qpid 0.12 client libraries, the above code would use SSL. But with Qpid 0.14, the code will not use SSL.

The change in behaviour is a result of code changes to the ConnectionImpl::tryConnect function in cpp/src/qpid/client/amqp0_10/ConnectionImpl.cpp in r1156262

Prior to that revision, the SimpleUrlParser was used, and settings passed to a Connection::open overload directly. This worked fine.

Post that revision, the code looks like this:

Url url(*i);
if (url.getUser().size()) settings.username = url.getUser();
if (url.getPass().size()) settings.password = url.getPass();
QPID_LOG(debug, *i << " " << settings.protocol);
QPID_LOG(debug, url);
connection.open(url, settings);

(I added the QPID_LOG calls).

The QPID_LOG calls produce output like:

localhost:5671 ssl
amqp:tcp:localhost:5671

The problem is that the Url constructor is defaulting the protocol to TCP (via UrlParser::protocolAddr). Then, when the Connection::open overload is called, the Url object's defaulted-but-now-explicit protocol value (TCP) is used in preference to the settings.protocol value (SSL).

I think the correct solution here, thought it would be non-trivial, would be to pass settings (or at least settings.protocol) to the Url constructor as an optional default protocol. Then the Url class should use that default protocol instead of TCP when the address string does not include an explicit protocol.

Another option would be to provide some sort of Url::protocolWasDefaulted flag so that the ConnectionImpl::tryConnect function could override from settings when appropriate - this would be less code change, but the above solution would be more elegant IMO.

Note, the obvious workaround is to always use explicit protocols in URLs. That is, always use "ssl:hostname" instead of "hostname".