[
https://issues.apache.org/jira/browse/QPID-3832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13218170#comment-13218170
]
jirapos...@reviews.apache.org commented on QPID-3832:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4083/
-----------------------------------------------------------
Review request for Alan Conway, Kenneth Giusti and michael goulish.
Summary
-------
Previous unification of URL support broke the ability to supply the 'transport'
(e.g. tcp, rdma, ssl etc) on a connection. This can still be supplied in the
URL itself if the 0-10 URL format is used but the regression should be fixed.
The change in this patch adds a method to the Url class to parse a string
without populating any resulting addresses with TCP as the default protocol if
none is specified. This allows the protocol in the Address to indicate what was
explicitly provided and lets the decision on defaults be taken elsewhere (in
this case in the underlying client API).
This addresses bug QPID-3832.
https://issues.apache.org/jira/browse/QPID-3832
Diffs
-----
/trunk/qpid/cpp/include/qpid/Url.h 1294525
/trunk/qpid/cpp/src/qpid/Url.cpp 1294525
/trunk/qpid/cpp/src/qpid/client/Connection.cpp 1294525
/trunk/qpid/cpp/src/qpid/client/amqp0_10/ConnectionImpl.cpp 1294525
/trunk/qpid/cpp/src/tests/ssl_test 1294525
/trunk/qpid/cpp/src/tests/ssl_test 1294525
Diff: https://reviews.apache.org/r/4083/diff
Testing
-------
New case added to sssl tests that uses a mixture of url and connection-option.
Make check passes.
Thanks,
Gordon
> Qpid 0.14 broke transport connection setting
> --------------------------------------------
>
> Key: QPID-3832
> URL: https://issues.apache.org/jira/browse/QPID-3832
> Project: Qpid
> Issue Type: Bug
> Components: C++ Client
> Affects Versions: 0.14
> Reporter: Paul Colby
> Assignee: Gordon Sim
> Labels: SSL
>
> The transport connection setting was broken in r1156262.
> Consider the following test code:
> {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;
> }
> {code}
> 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:
> {code}
> 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);
> {code}
> (I added the {{QPID_LOG}} calls).
> The {{QPID_LOG}} calls produce output like:
> {code}
> localhost:5671 ssl
> amqp:tcp:localhost:5671
> {code}
> 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".
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscr...@qpid.apache.org
