From df5bfad6f032b17ee0dffe85cdaadc2c20edaae2 Mon Sep 17 00:00:00 2001 From: Tobias Markmann Date: Wed, 7 Oct 2015 23:57:17 +0200 Subject: 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 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 @@ -112,21 +112,20 @@ boost::shared_ptr SOCKS5BytestreamProxiesManager: return connection; } -void SOCKS5BytestreamProxiesManager::handleProxyFound(S5BProxyRequest::ref proxy) { - if (proxy) { - if (HostAddress(proxy->getStreamHost().get().host).isValid()) { - addS5BProxy(proxy); - onDiscoveredProxiesChanged(); - } - else { - DomainNameAddressQuery::ref resolveRequest = resolver_->createAddressQuery(proxy->getStreamHost().get().host); - resolveRequest->onResult.connect(boost::bind(&SOCKS5BytestreamProxiesManager::handleNameLookupResult, this, _1, _2, proxy)); - resolveRequest->run(); +void SOCKS5BytestreamProxiesManager::handleProxiesFound(std::vector proxyHosts) { + foreach(S5BProxyRequest::ref proxy, proxyHosts) { + if (proxy) { + if (HostAddress(proxy->getStreamHost().get().host).isValid()) { + addS5BProxy(proxy); + onDiscoveredProxiesChanged(); + } + else { + DomainNameAddressQuery::ref resolveRequest = resolver_->createAddressQuery(proxy->getStreamHost().get().host); + resolveRequest->onResult.connect(boost::bind(&SOCKS5BytestreamProxiesManager::handleNameLookupResult, this, _1, _2, proxy)); + resolveRequest->run(); + } } } - else { - onDiscoveredProxiesChanged(); - } proxyFinder_->stop(); proxyFinder_.reset(); } @@ -157,12 +156,12 @@ void SOCKS5BytestreamProxiesManager::handleNameLookupResult(const std::vector(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 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()) { 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 @@ -29,6 +29,7 @@ namespace Swift { class ConnectionFactory; class DomainNameResolver; class DomainNameResolveError; + class IQRouter; /** * - manages list of working S5B proxies @@ -56,7 +57,7 @@ namespace Swift { boost::signal onDiscoveredProxiesChanged; private: - void handleProxyFound(S5BProxyRequest::ref proxy); + void handleProxiesFound(std::vector proxyHosts); void handleNameLookupResult(const std::vector&, boost::optional, S5BProxyRequest::ref proxy); void queryForProxies(); 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 @@ -4,12 +4,19 @@ * 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 -#include #include +#include #include +#include #include #include #include @@ -25,19 +32,27 @@ SOCKS5BytestreamProxyFinder::~SOCKS5BytestreamProxyFinder() { void SOCKS5BytestreamProxyFinder::start() { serviceWalker = boost::make_shared(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 > 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(); boost::shared_ptr > request = boost::make_shared >(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(); } @@ -47,16 +62,26 @@ void SOCKS5BytestreamProxyFinder::handleServiceFound(const JID& jid, boost::shar } } -void SOCKS5BytestreamProxyFinder::handleProxyResponse(boost::shared_ptr request, ErrorPayload::ref error) { +void SOCKS5BytestreamProxyFinder::handleWalkEnded() { + if (pendingRequests.empty()) { + onProxiesFound(proxyHosts); + } +} + +void SOCKS5BytestreamProxyFinder::handleProxyResponse(boost::shared_ptr > requester, boost::shared_ptr 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); - } else { - //assert(false); + SWIFT_LOG(debug) << "add request" << std::endl; + proxyHosts.push_back(request); } } + 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 @@ -17,7 +17,6 @@ #include #include -#include #include namespace Swift { @@ -37,18 +36,21 @@ class SWIFTEN_API SOCKS5BytestreamProxyFinder { void start(); void stop(); - boost::signal)> onProxyFound; + boost::signal >)> onProxiesFound; private: void sendBytestreamQuery(const JID&); void handleServiceFound(const JID&, boost::shared_ptr); - void handleProxyResponse(boost::shared_ptr, ErrorPayload::ref); + void handleProxyResponse(boost::shared_ptr > requester, boost::shared_ptr, ErrorPayload::ref); + void handleWalkEnded(); + private: JID service; IQRouter* iqRouter; boost::shared_ptr serviceWalker; - std::vector > > requests; - }; + std::vector proxyHosts; + std::set > > pendingRequests; +}; } -- cgit v0.10.2-6-g49f6