From 9a80a4b67f8fd86b3a2f5bccef3c0cd59dd83d75 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Remko=20Tron=C3=A7on?= <git@el-tramo.be>
Date: Fri, 26 Aug 2011 22:21:52 +0200
Subject: Don't consider Get and Set requests as Response results.

Release-Notes: Fixed a problem where we would treat incoming queries with IDs clashing with pending requests as errors.

diff --git a/Swiften/Queries/Request.cpp b/Swiften/Queries/Request.cpp
index 6c47725..a77cf49 100644
--- a/Swiften/Queries/Request.cpp
+++ b/Swiften/Queries/Request.cpp
@@ -39,25 +39,27 @@ void Request::send() {
 
 bool Request::handleIQ(boost::shared_ptr<IQ> iq) {
 	bool handled = false;
-	if (sent_ && iq->getID() == id_) {
-		if (iq->getType() == IQ::Result) {
-			boost::shared_ptr<Payload> payload = iq->getPayloadOfSameType(payload_);
-			if (!payload && boost::dynamic_pointer_cast<RawXMLPayload>(payload_) && !iq->getPayloads().empty()) {
-				payload = iq->getPayloads().front();
-			}
-			handleResponse(payload, ErrorPayload::ref());
-		}
-		else {
-			ErrorPayload::ref errorPayload = iq->getPayload<ErrorPayload>();
-			if (errorPayload) {
-				handleResponse(boost::shared_ptr<Payload>(), errorPayload);
+	if (iq->getType() == IQ::Result || iq->getType() == IQ::Error) {
+		if (sent_ && iq->getID() == id_) {
+			if (iq->getType() == IQ::Result) {
+				boost::shared_ptr<Payload> payload = iq->getPayloadOfSameType(payload_);
+				if (!payload && boost::dynamic_pointer_cast<RawXMLPayload>(payload_) && !iq->getPayloads().empty()) {
+					payload = iq->getPayloads().front();
+				}
+				handleResponse(payload, ErrorPayload::ref());
 			}
 			else {
-				handleResponse(boost::shared_ptr<Payload>(), ErrorPayload::ref(new ErrorPayload(ErrorPayload::UndefinedCondition)));
+				ErrorPayload::ref errorPayload = iq->getPayload<ErrorPayload>();
+				if (errorPayload) {
+					handleResponse(boost::shared_ptr<Payload>(), errorPayload);
+				}
+				else {
+					handleResponse(boost::shared_ptr<Payload>(), ErrorPayload::ref(new ErrorPayload(ErrorPayload::UndefinedCondition)));
+				}
 			}
+			router_->removeHandler(this);
+			handled = true;
 		}
-		router_->removeHandler(this);
-		handled = true;
 	}
 	return handled;
 }
diff --git a/Swiften/Queries/UnitTest/RequestTest.cpp b/Swiften/Queries/UnitTest/RequestTest.cpp
index 34d07c9..46eb6fd 100644
--- a/Swiften/Queries/UnitTest/RequestTest.cpp
+++ b/Swiften/Queries/UnitTest/RequestTest.cpp
@@ -29,6 +29,8 @@ class RequestTest : public CppUnit::TestFixture {
 		CPPUNIT_TEST(testHandleIQ_BeforeSend);
 		CPPUNIT_TEST(testHandleIQ_DifferentPayload);
 		CPPUNIT_TEST(testHandleIQ_RawXMLPayload);
+		CPPUNIT_TEST(testHandleIQ_GetWithSameID);
+		CPPUNIT_TEST(testHandleIQ_SetWithSameID);
 		CPPUNIT_TEST_SUITE_END();
 
 	public:
@@ -122,14 +124,14 @@ class RequestTest : public CppUnit::TestFixture {
 			testling.send();
 
 			boost::shared_ptr<IQ> error = createError("test-id");
-			boost::shared_ptr<Payload> errorPayload = boost::shared_ptr<ErrorPayload>(new ErrorPayload(ErrorPayload::FeatureNotImplemented));
+			boost::shared_ptr<Payload> errorPayload = boost::shared_ptr<ErrorPayload>(new ErrorPayload(ErrorPayload::InternalServerError));
 			error->addPayload(errorPayload);
 			channel_->onIQReceived(error);
 
 			CPPUNIT_ASSERT_EQUAL(0, responsesReceived_);
 			CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(receivedErrors.size()));
 			CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(channel_->iqs_.size()));
-			CPPUNIT_ASSERT_EQUAL(ErrorPayload::FeatureNotImplemented, receivedErrors[0].getCondition());
+			CPPUNIT_ASSERT_EQUAL(ErrorPayload::InternalServerError, receivedErrors[0].getCondition());
 		}
 
 		void testHandleIQ_ErrorWithoutPayload() {
@@ -182,6 +184,35 @@ class RequestTest : public CppUnit::TestFixture {
 			CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(channel_->iqs_.size()));
 		}
 
+	 void testHandleIQ_GetWithSameID() {
+			MyRequest testling(IQ::Get, JID("foo@bar.com/baz"), payload_, router_);
+			testling.onResponse.connect(boost::bind(&RequestTest::handleResponse, this, _1, _2));
+			testling.send();
+
+			boost::shared_ptr<IQ> response = createResponse("test-id");
+			response->setType(IQ::Get);
+			channel_->onIQReceived(response);
+
+			CPPUNIT_ASSERT_EQUAL(0, responsesReceived_);
+			CPPUNIT_ASSERT_EQUAL(0, static_cast<int>(receivedErrors.size()));
+			CPPUNIT_ASSERT_EQUAL(2, static_cast<int>(channel_->iqs_.size()));
+		}
+
+		void testHandleIQ_SetWithSameID() {
+			MyRequest testling(IQ::Get, JID("foo@bar.com/baz"), payload_, router_);
+			testling.onResponse.connect(boost::bind(&RequestTest::handleResponse, this, _1, _2));
+			testling.send();
+
+			boost::shared_ptr<IQ> response = createResponse("test-id");
+			response->setType(IQ::Set);
+			channel_->onIQReceived(response);
+
+			CPPUNIT_ASSERT_EQUAL(0, responsesReceived_);
+			CPPUNIT_ASSERT_EQUAL(0, static_cast<int>(receivedErrors.size()));
+			CPPUNIT_ASSERT_EQUAL(2, static_cast<int>(channel_->iqs_.size()));
+		}
+
+
 	private:
 		void handleResponse(boost::shared_ptr<Payload> p, ErrorPayload::ref e) {
 			if (e) {
-- 
cgit v0.10.2-6-g49f6