Sluift: Add timeout to roster receiving operations

This avoids Sluift indefinitely waiting if the server does not respond Test-Information: Before the patch, a sluift client would wait indefinitely if a buggy XMPP server does not respond. With this patch, sluift clients now timeout and through an exception after SLUIFT_TIMEOUT when the server does not respond. Change-Id: I9d36f53a8f4d5b3d594cef68c42de38fd5a1c296
author: Roger Planas <roger.planas@isode.com> 2016-02-05 12:45:39 (GMT)
committer: Kevin Smith <kevin.smith@isode.com> 2016-02-10 14:33:37 (GMT)
commit: af6493bf45fcd9174b485f8ae0532e2eb4e6914e (patch)
tree: cfabe60bd9db93ce05557ee26ba18cc4622e0db0
parent: dcfe31ce3055f3af7e3b617752d9c9fd16672569 (diff)
download: swift-af6493bf45fcd9174b485f8ae0532e2eb4e6914e.zip
swift-af6493bf45fcd9174b485f8ae0532e2eb4e6914e.tar.bz2
3 files changed, 18 insertions, 10 deletions
diff --git a/Sluift/SluiftClient.cpp b/Sluift/SluiftClient.cpp
index 9db8969..d2b1beb 100644
--- a/Sluift/SluiftClient.cpp
+++ b/Sluift/SluiftClient.cpp
@@ -1,32 +1,32 @@
 /*
- * Copyright (c) 2013-2014 Isode Limited.
+ * Copyright (c) 2013-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #include <Sluift/SluiftClient.h>
 
 #include <boost/numeric/conversion/cast.hpp>
 
 #include <Swiften/Client/ClientXMLTracer.h>
 #include <Swiften/Client/Client.h>
 #include <Swiften/Roster/XMPPRoster.h>
 #include <Sluift/SluiftGlobals.h>
 #include <Sluift/Lua/Exception.h>
 #include <Swiften/Elements/Message.h>
 #include <Swiften/Elements/PubSubEvent.h>
 #include <Swiften/Queries/RawRequest.h>
 #include <Sluift/Helpers.h>
 #include <Swiften/Elements/Presence.h>
 
 using namespace Swift;
 
 SluiftClient::SluiftClient(
 		const JID& jid, 
 		const std::string& password, 
 		NetworkFactories* networkFactories, 
 		SimpleEventLoop* eventLoop) :
 			networkFactories(networkFactories), 
 			eventLoop(eventLoop),
 			tracer(NULL) {
 	client = new Client(jid, password, networkFactories);
@@ -98,67 +98,75 @@ void SluiftClient::setSoftwareVersion(const std::string& name, const std::string
 
 boost::optional<SluiftClient::Event> SluiftClient::getNextEvent(
 		int timeout, boost::function<bool (const Event&)> condition) {
 	Watchdog watchdog(timeout, networkFactories->getTimerFactory());
 	size_t currentIndex = 0;
 	while (true) {
 		// Look for pending events in the queue
 		while (currentIndex < pendingEvents.size()) {
 			Event event = pendingEvents[currentIndex];
 			if (!condition || condition(event)) {
 				pendingEvents.erase(
 						pendingEvents.begin() 
 						+ boost::numeric_cast<int>(currentIndex));
 				return event;
 			}
 			++currentIndex;
 		}
 
 		// Wait for new events
 		while (!watchdog.getTimedOut() && currentIndex >= pendingEvents.size() && client->isActive()) {
 			eventLoop->runUntilEvents();
 		}
 
 		// Finish if we're disconnected or timed out
 		if (watchdog.getTimedOut() || !client->isActive()) {
 			return boost::optional<Event>();
 		}
 	}
 }
 
-std::vector<XMPPRosterItem> SluiftClient::getRoster() {
+std::vector<XMPPRosterItem> SluiftClient::getRoster(int timeout) {
+	Watchdog watchdog(timeout, networkFactories->getTimerFactory());
 	if (!rosterReceived) {
 		// If we haven't requested it yet, request it for the first time
 		client->requestRoster();
-	}
-	while (!rosterReceived) {
-		eventLoop->runUntilEvents();
+
+		// Wait for new events
+		while (!watchdog.getTimedOut() && !rosterReceived) {
+			eventLoop->runUntilEvents();
+		}
+
+		// Throw an error if we're timed out
+		if (watchdog.getTimedOut()) {
+			throw Lua::Exception("Timeout while requesting roster");
+		}
 	}
 	return client->getRoster()->getItems();
 }
 
 void SluiftClient::handleIncomingMessage(boost::shared_ptr<Message> stanza) {
 	if (stanza->getPayload<PubSubEvent>()) {
 		// Already handled by pubsub manager
 		return;
 	}
 	pendingEvents.push_back(Event(stanza));
 }
 
 void SluiftClient::handleIncomingPresence(boost::shared_ptr<Presence> stanza) {
 	pendingEvents.push_back(Event(stanza));
 }
 
 void SluiftClient::handleIncomingPubSubEvent(const JID& from, boost::shared_ptr<PubSubEventPayload> event) {
 	pendingEvents.push_back(Event(from, event));
 }
 
 void SluiftClient::handleInitialRosterPopulated() {
 	rosterReceived = true;
 }
 
 void SluiftClient::handleRequestResponse(boost::shared_ptr<Payload> response, boost::shared_ptr<ErrorPayload> error) {
 	requestResponse = response;
 	requestError = error;
 	requestResponseReceived = true;
 }
 
diff --git a/Sluift/SluiftClient.h b/Sluift/SluiftClient.h
index b7a6578..68c4b61 100644
--- a/Sluift/SluiftClient.h
+++ b/Sluift/SluiftClient.h
@@ -1,32 +1,32 @@
 /*
- * Copyright (c) 2013 Isode Limited.
+ * Copyright (c) 2013-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #pragma once
 
 #include <deque>
 #include <boost/optional.hpp>
 #include <boost/bind.hpp>
 #include <boost/function.hpp>
 
 #include <Swiften/Client/ClientOptions.h>
 #include <Swiften/Elements/IQ.h>
 #include <Swiften/Elements/Message.h>
 #include <Swiften/Elements/Presence.h>
 #include <Swiften/Queries/GenericRequest.h>
 #include <Swiften/Roster/XMPPRosterItem.h>
 #include <Swiften/Client/ClientError.h>
 #include <Swiften/Network/NetworkFactories.h>
 #include <Swiften/Client/Client.h>
 #include <Swiften/EventLoop/SimpleEventLoop.h>
 #include <Sluift/Watchdog.h>
 #include <Swiften/PubSub/PubSubManager.h>
 #include <Sluift/Response.h>
 
 namespace Swift {
 	struct SluiftGlobals;
 	class ClientXMLTracer;
 	class Client;
 	class Stanza;
@@ -76,56 +76,56 @@ namespace Swift {
 			void connect(const std::string& host, int port);
 			void waitConnected(int timeout);
 			bool isConnected() const;
 			void setTraceEnabled(bool b);
 
 			template<typename T>
 				Sluift::Response sendPubSubRequest(
 					IQ::Type type, const JID& jid, boost::shared_ptr<T> payload, int timeout) {
 				return sendRequest(client->getPubSubManager()->createRequest(
 							type, jid, payload), timeout);
 			}
 
 			template<typename REQUEST_TYPE>
 			Sluift::Response sendRequest(REQUEST_TYPE request, int timeout) {
 				boost::signals::scoped_connection c = request->onResponse.connect(
 						boost::bind(&SluiftClient::handleRequestResponse, this, _1, _2));
 				return doSendRequest(request, timeout);
 			}
 
 			template<typename REQUEST_TYPE>
 			Sluift::Response sendVoidRequest(REQUEST_TYPE request, int timeout) {
 				boost::signals::scoped_connection c = request->onResponse.connect(
 						boost::bind(&SluiftClient::handleRequestResponse, this, boost::shared_ptr<Payload>(), _1));
 				return doSendRequest(request, timeout);
 			}
 
 			void disconnect();
 			void setSoftwareVersion(const std::string& name, const std::string& version, const std::string& os);
 			boost::optional<SluiftClient::Event> getNextEvent(int timeout, 
 					boost::function<bool (const Event&)> condition = 0);
-			std::vector<XMPPRosterItem> getRoster();
+			std::vector<XMPPRosterItem> getRoster(int timeout);
 
 		private:
 			Sluift::Response doSendRequest(boost::shared_ptr<Request> request, int timeout);
 
 			void handleIncomingMessage(boost::shared_ptr<Message> stanza);
 			void handleIncomingPresence(boost::shared_ptr<Presence> stanza);
 			void handleIncomingPubSubEvent(const JID& from, boost::shared_ptr<PubSubEventPayload> event);
 			void handleInitialRosterPopulated();
 			void handleRequestResponse(boost::shared_ptr<Payload> response, boost::shared_ptr<ErrorPayload> error);
 			void handleDisconnected(const boost::optional<ClientError>& error);
 		
 		private:
 			NetworkFactories* networkFactories;
 			SimpleEventLoop* eventLoop;
 			Client* client;
 			ClientOptions options;
 			ClientXMLTracer* tracer;
 			bool rosterReceived;
 			std::deque<Event> pendingEvents;
 			boost::optional<ClientError> disconnectedError;
 			bool requestResponseReceived;
 			boost::shared_ptr<Payload> requestResponse;
 			boost::shared_ptr<ErrorPayload> requestError;
 	};
 }
diff --git a/Sluift/client.cpp b/Sluift/client.cpp
index 8f6ff3a..6373a20 100644
--- a/Sluift/client.cpp
+++ b/Sluift/client.cpp
@@ -1,32 +1,32 @@
 /*
- * Copyright (c) 2013-2015 Isode Limited.
+ * Copyright (c) 2013-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #include <boost/lambda/lambda.hpp>
 #include <boost/lambda/bind.hpp>
 #include <boost/assign/list_of.hpp>
 #include <iostream>
 
 #include <Sluift/SluiftClient.h>
 #include <Swiften/JID/JID.h>
 #include <Swiften/Elements/SoftwareVersion.h>
 #include <Swiften/Elements/Message.h>
 #include <Swiften/Elements/Presence.h>
 #include <Swiften/Elements/RawXMLPayload.h>
 #include <Swiften/Elements/RosterItemPayload.h>
 #include <Swiften/Elements/RosterPayload.h>
 #include <Swiften/Elements/DiscoInfo.h>
 #include <Swiften/Elements/MAMQuery.h>
 #include <Swiften/Disco/ClientDiscoManager.h>
 #include <Swiften/Queries/GenericRequest.h>
 #include <Swiften/Presence/PresenceSender.h>
 #include <Swiften/Roster/XMPPRoster.h>
 #include <Swiften/Roster/SetRosterRequest.h>
 #include <Swiften/Presence/SubscriptionManager.h>
 #include <Swiften/Roster/XMPPRosterItem.h>
 #include <Swiften/Queries/IQRouter.h>
 #include <Swiften/Queries/Requests/GetSoftwareVersionRequest.h>
 #include <Swiften/TLS/PKCS12Certificate.h>
 #include <Sluift/Lua/FunctionRegistration.h>
@@ -159,61 +159,61 @@ SLUIFT_LUA_FUNCTION_WITH_HELP(
 
 SLUIFT_LUA_FUNCTION_WITH_HELP(
 		Client, set_version,
 
 		"Sets the published version of this client.",
 
 		"self",
 
 		"name  the name of the client software\n"
 		"version  the version identifier of this client\n"
 		"os  the OS this client is running on\n"
 ) {
 	Sluift::globals.eventLoop.runOnce();
 	SluiftClient* client = getClient(L);
 	if (boost::shared_ptr<SoftwareVersion> version = boost::dynamic_pointer_cast<SoftwareVersion>(Sluift::globals.elementConvertor.convertFromLuaUntyped(L, 2, "software_version"))) {
 		client->setSoftwareVersion(version->getName(), version->getVersion(), version->getOS());
 	}
 	return 0;
 }
 
 SLUIFT_LUA_FUNCTION_WITH_HELP(
 		Client, get_contacts,
 		"Returns a table of all the contacts in the contact list.",
 		"self\n",
 		""
 ) {
 	Sluift::globals.eventLoop.runOnce();
 
 	SluiftClient* client = getClient(L);
 	Lua::Table contactsTable;
-	foreach(const XMPPRosterItem& item, client->getRoster()) {
+	foreach(const XMPPRosterItem& item, client->getRoster(getGlobalTimeout(L))) {
 		std::string subscription;
 		switch(item.getSubscription()) {
 			case RosterItemPayload::None: subscription = "none"; break;
 			case RosterItemPayload::To: subscription = "to"; break;
 			case RosterItemPayload::From: subscription = "from"; break;
 			case RosterItemPayload::Both: subscription = "both"; break;
 			case RosterItemPayload::Remove: subscription = "remove"; break;
 		}
 		Lua::Table itemTable = boost::assign::map_list_of
 			("jid", boost::make_shared<Lua::Value>(item.getJID().toString()))
 			("name", boost::make_shared<Lua::Value>(item.getName()))
 			("subscription", boost::make_shared<Lua::Value>(subscription))
 			("groups", boost::make_shared<Lua::Value>(std::vector<Lua::Value>(item.getGroups().begin(), item.getGroups().end())));
 		contactsTable[item.getJID().toString()] = boost::make_shared<Lua::Value>(itemTable);
 	}
 	pushValue(L, contactsTable);
 	Lua::registerTableToString(L, -1);
 	return 1;
 }
 
 SLUIFT_LUA_FUNCTION_WITH_HELP(
 		Client, send_message,
 		"Send a message.",
 		"self\n"
 		"to  the JID to send the message to\n"
 		"body  the body of the message. Can alternatively be specified using the `body` option\n",
 
 		"to  the JID to send the message to\n"
 		"body  the body of the message\n"
 		"subject  the subject of the MUC room to set\n"
@@ -609,61 +609,61 @@ SLUIFT_LUA_FUNCTION_WITH_HELP(
 		const char* rawJID = lua_tostring(L, -1);
 		if (rawJID) {
 			item.setJID(std::string(rawJID));
 		}
 		lua_getfield(L, 2, "name");
 		const char* rawName = lua_tostring(L, -1);
 		if (rawName) {
 			item.setName(rawName);
 		}
 		lua_getfield(L, 2, "groups");
 		if (!lua_isnil(L, -1)) {
 			if (lua_type(L, -1) == LUA_TTABLE) {
 				for (size_t i = 1; i <= lua_objlen(L, -1); ++i) {
 					lua_rawgeti(L, -1, boost::numeric_cast<int>(i));
 					const char* rawGroup = lua_tostring(L, -1);
 					if (rawGroup) {
 						item.addGroup(rawGroup);
 					}
 					lua_pop(L, 1);
 				}
 			}
 			else {
 				throw Lua::Exception("Groups should be a table");
 			}
 		}
 	}
 	else {
 		item.setJID(Lua::checkString(L, 2));
 	}
 
-	client->getRoster();
+	client->getRoster(timeout);
 	if (!client->getClient()->getRoster()->containsJID(item.getJID())) {
 		RosterPayload::ref roster = boost::make_shared<RosterPayload>();
 		roster->addItem(item);
 
 		Sluift::Response response = client->sendVoidRequest(
 			SetRosterRequest::create(roster, client->getClient()->getIQRouter()), timeout);
 		if (response.error) {
 			return response.convertToLuaResult(L);
 		}
 	}
 	client->getClient()->getSubscriptionManager()->requestSubscription(item.getJID());
 	lua_pushboolean(L, true);
 	return 1;
 }
 
 SLUIFT_LUA_FUNCTION_WITH_HELP(
 		Client, remove_contact,
 		"Remove a contact from the contact list.",
 		"self\n"
 		"jid  the JID of the contact to remove\n",
 		""
 ) {
 	Sluift::globals.eventLoop.runOnce();
 	SluiftClient* client = getClient(L);
 	JID jid(Lua::checkString(L, 2));
 	int timeout = getGlobalTimeout(L);
 
 	RosterPayload::ref roster = boost::make_shared<RosterPayload>();
 	roster->addItem(RosterItemPayload(JID(Lua::checkString(L, 2)), "", RosterItemPayload::Remove));
author	Roger Planas <roger.planas@isode.com>	2016-02-05 12:45:39 (GMT)
committer	Kevin Smith <kevin.smith@isode.com>	2016-02-10 14:33:37 (GMT)
commit	af6493bf45fcd9174b485f8ae0532e2eb4e6914e (patch)
tree	cfabe60bd9db93ce05557ee26ba18cc4622e0db0
parent	dcfe31ce3055f3af7e3b617752d9c9fd16672569 (diff)
download	swift-af6493bf45fcd9174b485f8ae0532e2eb4e6914e.zip swift-af6493bf45fcd9174b485f8ae0532e2eb4e6914e.tar.bz2