Fix hang during file-transfer preparation with missing proxy

The code used to call the onProxiesFound signal only if a proxy was found. In case of missing S5B proxy at the users server, the file-transfer preparation would hang. Now the code wants until the discovery phase is done and then calls the onProxiesFound signal with the list of discovered proxies. In case of missing S5B proxy server the signal is called with an empty list so the file-transfer flow can continue. Test-Information: Ran FileTransferTest integration test and manually tested a file-transfer on a server without S5B proxy. Change-Id: I31d3cc08fe6453b5cdfe6be286f884a920470d28
author: Tobias Markmann <tm@ayena.de> 2015-10-07 21:57:17 (GMT)
committer: Tobias Markmann <tm@ayena.de> 2015-10-07 21:57:17 (GMT)
commit: df5bfad6f032b17ee0dffe85cdaadc2c20edaae2 (patch)
tree: 5b5166d0aeae60ab78d75528adeea8ef620cd7bd
parent: 37aafcb4d693a0b4f5944a52e0c070e5aa384245 (diff)
download: swift-df5bfad6f032b17ee0dffe85cdaadc2c20edaae2.zip
swift-df5bfad6f032b17ee0dffe85cdaadc2c20edaae2.tar.bz2
4 files changed, 54 insertions, 27 deletions
diff --git a/Swiften/FileTransfer/SOCKS5BytestreamProxiesManager.cpp b/Swiften/FileTransfer/SOCKS5BytestreamProxiesManager.cpp
index 1036e12..25a12ea 100644
--- a/Swiften/FileTransfer/SOCKS5BytestreamProxiesManager.cpp
+++ b/Swiften/FileTransfer/SOCKS5BytestreamProxiesManager.cpp
@@ -110,25 +110,24 @@ boost::shared_ptr<SOCKS5BytestreamClientSession> SOCKS5BytestreamProxiesManager:
 boost::shared_ptr<SOCKS5BytestreamClientSession> SOCKS5BytestreamProxiesManager::createSOCKS5BytestreamClientSession(HostAddressPort addressPort, const std::string& destAddr) {
        SOCKS5BytestreamClientSession::ref connection = boost::make_shared<SOCKS5BytestreamClientSession>(connectionFactory_->createConnection(), addressPort, destAddr, timerFactory_);
        return connection;
 }
-void SOCKS5BytestreamProxiesManager::handleProxyFound(S5BProxyRequest::ref proxy) {
+void SOCKS5BytestreamProxiesManager::handleProxiesFound(std::vector<S5BProxyRequest::ref> proxyHosts) {
-        if (proxy) {
+        foreach(S5BProxyRequest::ref proxy, proxyHosts) {
-                if (HostAddress(proxy->getStreamHost().get().host).isValid()) {
+                if (proxy) {
-                        addS5BProxy(proxy);
+                        if (HostAddress(proxy->getStreamHost().get().host).isValid()) {
-                        onDiscoveredProxiesChanged();
+                                addS5BProxy(proxy);
-                }
+                                onDiscoveredProxiesChanged();
-                else {
+                        }
-                        DomainNameAddressQuery::ref resolveRequest = resolver_->createAddressQuery(proxy->getStreamHost().get().host);
+                        else {
-                        resolveRequest->onResult.connect(boost::bind(&SOCKS5BytestreamProxiesManager::handleNameLookupResult, this, _1, _2, proxy));
+                                DomainNameAddressQuery::ref resolveRequest = resolver_->createAddressQuery(proxy->getStreamHost().get().host);
-                        resolveRequest->run();
+                                resolveRequest->onResult.connect(boost::bind(&SOCKS5BytestreamProxiesManager::handleNameLookupResult, this, _1, _2, proxy));
+                                resolveRequest->run();
+                        }
                }
        }
-        else {
-                onDiscoveredProxiesChanged();
-        }
        proxyFinder_->stop();
        proxyFinder_.reset();
 }
 void SOCKS5BytestreamProxiesManager::handleNameLookupResult(const std::vector<HostAddress>& addresses, boost::optional<DomainNameResolveError> error, S5BProxyRequest::ref proxy) {
@@ -155,16 +154,16 @@ void SOCKS5BytestreamProxiesManager::handleNameLookupResult(const std::vector<Ho
 }
 void SOCKS5BytestreamProxiesManager::queryForProxies() {
        proxyFinder_ = boost::make_shared<SOCKS5BytestreamProxyFinder>(serviceRoot_, iqRouter_);
-        proxyFinder_->onProxyFound.connect(boost::bind(&SOCKS5BytestreamProxiesManager::handleProxyFound, this, _1));
+        proxyFinder_->onProxiesFound.connect(boost::bind(&SOCKS5BytestreamProxiesManager::handleProxiesFound, this, _1));
        proxyFinder_->start();
 }
 void SOCKS5BytestreamProxiesManager::handleProxySessionReady(const std::string& sessionID, const JID& jid, boost::shared_ptr<SOCKS5BytestreamClientSession> session, bool error) {
-        session->onSessionReady.disconnect(boost::bind(&SOCKS5BytestreamProxiesManager::handleProxySessionFinished, this, boost::cref(sessionID), boost::cref(jid), session, _1));
+        session->onSessionReady.disconnect(boost::bind(&SOCKS5BytestreamProxiesManager::handleProxySessionReady, this, boost::cref(sessionID), boost::cref(jid), session, _1));
        if (!error) {
                // The SOCKS5 bytestream session to the proxy succeeded; stop and remove other sessions.
                if (proxySessions_.find(sessionID) != proxySessions_.end()) {
                        for (ProxyJIDClientSessionVector::iterator i = proxySessions_[sessionID].begin(); i != proxySessions_[sessionID].end();) {
                                if ((i->first == jid) && (i->second != session)) {
diff --git a/Swiften/FileTransfer/SOCKS5BytestreamProxiesManager.h b/Swiften/FileTransfer/SOCKS5BytestreamProxiesManager.h
index e4bf1d9..c7daee7 100644
--- a/Swiften/FileTransfer/SOCKS5BytestreamProxiesManager.h
+++ b/Swiften/FileTransfer/SOCKS5BytestreamProxiesManager.h
@@ -27,10 +27,11 @@
 namespace Swift {
        class TimerFactory;
        class ConnectionFactory;
        class DomainNameResolver;
        class DomainNameResolveError;
+        class IQRouter;
        /**
         *      - manages list of working S5B proxies
         *      - creates initial connections (for the candidates you provide)
         */
@@ -54,11 +55,11 @@ namespace Swift {
                public:
                        boost::signal<void ()> onDiscoveredProxiesChanged;
                private:
-                        void handleProxyFound(S5BProxyRequest::ref proxy);
+                        void handleProxiesFound(std::vector<S5BProxyRequest::ref> proxyHosts);
                        void handleNameLookupResult(const std::vector<HostAddress>&, boost::optional<DomainNameResolveError>, S5BProxyRequest::ref proxy);
                        void queryForProxies();
                        void handleProxySessionReady(const std::string& sessionID, const JID& jid, boost::shared_ptr<SOCKS5BytestreamClientSession> session, bool error);
diff --git a/Swiften/FileTransfer/SOCKS5BytestreamProxyFinder.cpp b/Swiften/FileTransfer/SOCKS5BytestreamProxyFinder.cpp
index 9d7505b..78cf2e6 100644
--- a/Swiften/FileTransfer/SOCKS5BytestreamProxyFinder.cpp
+++ b/Swiften/FileTransfer/SOCKS5BytestreamProxyFinder.cpp
@@ -2,16 +2,23 @@
 * Copyright (c) 2011 Tobias Markmann
 * Licensed under the simplified BSD license.
 * See Documentation/Licenses/BSD-simplified.txt for more information.
 */
+/*
+ * Copyright (c) 2015 Isode Limited.
+ * All rights reserved.
+ * See the COPYING file for more information.
+ */
 #include <Swiften/FileTransfer/SOCKS5BytestreamProxyFinder.h>
-#include <boost/smart_ptr/make_shared.hpp>
 #include <boost/bind.hpp>
+#include <boost/smart_ptr/make_shared.hpp>
 #include <Swiften/Base/Log.h>
+#include <Swiften/Base/foreach.h>
 #include <Swiften/Elements/S5BProxyRequest.h>
 #include <Swiften/Queries/GenericRequest.h>
 #include <Swiften/Queries/IQRouter.h>
 namespace Swift {
@@ -23,40 +30,58 @@ SOCKS5BytestreamProxyFinder::~SOCKS5BytestreamProxyFinder() {
 }
 void SOCKS5BytestreamProxyFinder::start() {
        serviceWalker = boost::make_shared<DiscoServiceWalker>(service, iqRouter);
        serviceWalker->onServiceFound.connect(boost::bind(&SOCKS5BytestreamProxyFinder::handleServiceFound, this, _1, _2));
+        serviceWalker->onWalkComplete.connect(boost::bind(&SOCKS5BytestreamProxyFinder::handleWalkEnded, this));
        serviceWalker->beginWalk();
 }
 void SOCKS5BytestreamProxyFinder::stop() {
+        typedef boost::shared_ptr<GenericRequest<S5BProxyRequest> > S5BProxyRequestGenericRequest;
+        foreach (S5BProxyRequestGenericRequest requester, pendingRequests) {
+                requester->onResponse.disconnect(boost::bind(&SOCKS5BytestreamProxyFinder::handleProxyResponse, this, requester, _1, _2));
+        }
        serviceWalker->endWalk();
        serviceWalker->onServiceFound.disconnect(boost::bind(&SOCKS5BytestreamProxyFinder::handleServiceFound, this, _1, _2));
+        serviceWalker->onWalkComplete.disconnect(boost::bind(&SOCKS5BytestreamProxyFinder::handleWalkEnded, this));
        serviceWalker.reset();
 }
 void SOCKS5BytestreamProxyFinder::sendBytestreamQuery(const JID& jid) {
        S5BProxyRequest::ref proxyRequest = boost::make_shared<S5BProxyRequest>();
        boost::shared_ptr<GenericRequest<S5BProxyRequest> > request = boost::make_shared<GenericRequest<S5BProxyRequest> >(IQ::Get, jid, proxyRequest, iqRouter);
-        request->onResponse.connect(boost::bind(&SOCKS5BytestreamProxyFinder::handleProxyResponse, this, _1, _2));
+        request->onResponse.connect(boost::bind(&SOCKS5BytestreamProxyFinder::handleProxyResponse, this, request, _1, _2));
+        pendingRequests.insert(request);
        request->send();
 }
 void SOCKS5BytestreamProxyFinder::handleServiceFound(const JID& jid, boost::shared_ptr<DiscoInfo> discoInfo) {
        if (discoInfo->hasFeature(DiscoInfo::Bytestream)) {
                sendBytestreamQuery(jid);
        }
 }
-void SOCKS5BytestreamProxyFinder::handleProxyResponse(boost::shared_ptr<S5BProxyRequest> request, ErrorPayload::ref error) {
+void SOCKS5BytestreamProxyFinder::handleWalkEnded() {
+        if (pendingRequests.empty()) {
+                onProxiesFound(proxyHosts);
+        }
+}
+void SOCKS5BytestreamProxyFinder::handleProxyResponse(boost::shared_ptr<GenericRequest<S5BProxyRequest> > requester, boost::shared_ptr<S5BProxyRequest> request, ErrorPayload::ref error) {
+        requester->onResponse.disconnect(boost::bind(&SOCKS5BytestreamProxyFinder::handleProxyResponse, this, requester, _1, _2));
+        pendingRequests.erase(requester);
        if (error) {
                SWIFT_LOG(debug) << "ERROR" << std::endl;
        } else {
                if (request) {
-                        onProxyFound(request);
+                        SWIFT_LOG(debug) << "add request" << std::endl;
-                } else {
+                        proxyHosts.push_back(request);
-                        //assert(false);
                }
        }
+        if (pendingRequests.empty() && !serviceWalker->isActive()) {
+                onProxiesFound(proxyHosts);
+        }
 }
 }
diff --git a/Swiften/FileTransfer/SOCKS5BytestreamProxyFinder.h b/Swiften/FileTransfer/SOCKS5BytestreamProxyFinder.h
index c5ad72a..1c24497 100644
--- a/Swiften/FileTransfer/SOCKS5BytestreamProxyFinder.h
+++ b/Swiften/FileTransfer/SOCKS5BytestreamProxyFinder.h
@@ -15,11 +15,10 @@
 #include <boost/shared_ptr.hpp>
 #include <Swiften/Base/API.h>
 #include <Swiften/Disco/DiscoServiceWalker.h>
-#include <Swiften/Network/HostAddressPort.h>
 #include <Swiften/Elements/S5BProxyRequest.h>
 namespace Swift {
 class JID;
@@ -35,20 +34,23 @@ class SWIFTEN_API SOCKS5BytestreamProxyFinder {
                ~SOCKS5BytestreamProxyFinder();
                void start();
                void stop();
-                boost::signal<void(boost::shared_ptr<S5BProxyRequest>)> onProxyFound;
+                boost::signal<void(std::vector<boost::shared_ptr<S5BProxyRequest> >)> onProxiesFound;
        private:
                void sendBytestreamQuery(const JID&);
                void handleServiceFound(const JID&, boost::shared_ptr<DiscoInfo>);
-                void handleProxyResponse(boost::shared_ptr<S5BProxyRequest>, ErrorPayload::ref);
+                void handleProxyResponse(boost::shared_ptr<GenericRequest<S5BProxyRequest> > requester, boost::shared_ptr<S5BProxyRequest>, ErrorPayload::ref);
+                void handleWalkEnded();
        private:
                JID service;
                IQRouter* iqRouter;
                boost::shared_ptr<DiscoServiceWalker> serviceWalker;
-                std::vector<boost::shared_ptr<GenericRequest<S5BProxyRequest> > > requests;
+                std::vector<S5BProxyRequest::ref> proxyHosts;
-        };
+                std::set<boost::shared_ptr<GenericRequest<S5BProxyRequest> > > pendingRequests;
+};
 }
author	Tobias Markmann <tm@ayena.de>	2015-10-07 21:57:17 (GMT)
committer	Tobias Markmann <tm@ayena.de>	2015-10-07 21:57:17 (GMT)
commit	df5bfad6f032b17ee0dffe85cdaadc2c20edaae2 (patch)
tree	5b5166d0aeae60ab78d75528adeea8ef620cd7bd
parent	37aafcb4d693a0b4f5944a52e0c070e5aa384245 (diff)
download	swift-df5bfad6f032b17ee0dffe85cdaadc2c20edaae2.zip swift-df5bfad6f032b17ee0dffe85cdaadc2c20edaae2.tar.bz2