From 87d5f54a90f2759aae587f01650e44aee8f2fe3f Mon Sep 17 00:00:00 2001 From: Tobias Markmann Date: Mon, 13 Mar 2017 10:01:54 +0100 Subject: Do no emit onXMPPDataRead when the session is terminated On a bosh error BOSHSessionStream, posts an event to the event loop that closes the XMPP stream. If onXMPPDataRead is emitted, that data is handled before the close of the stream though, which can cause another BOSH request to be sent. Test-Information: Added a unit test verifying the behaviour. Wrote a custom python script replicating the original error and verified that Swift does not stay connected on a BOSH error with additional text data. Tested on macOS 10.12.3 with recent clang trunk. Change-Id: Ie90099afa0934707a6758b00706a65227ceb48b8 diff --git a/Swiften/Network/BOSHConnection.cpp b/Swiften/Network/BOSHConnection.cpp index 1c468f1..b4ffa7d 100644 --- a/Swiften/Network/BOSHConnection.cpp +++ b/Swiften/Network/BOSHConnection.cpp @@ -5,7 +5,7 @@ */ /* - * Copyright (c) 2011-2016 Isode Limited. + * Copyright (c) 2011-2017 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -296,6 +296,7 @@ void BOSHConnection::handleDataRead(std::shared_ptr data) { if (parser.getBody()->attributes.getAttribute("type") == "terminate") { BOSHError::Type errorType = parseTerminationCondition(parser.getBody()->attributes.getAttribute("condition")); onSessionTerminated(errorType == BOSHError::NoError ? std::shared_ptr() : std::make_shared(errorType)); + return; } buffer_.clear(); if (waitingForStartResponse_) { diff --git a/Swiften/Network/UnitTest/BOSHConnectionTest.cpp b/Swiften/Network/UnitTest/BOSHConnectionTest.cpp index 99dd462..e34cb96 100644 --- a/Swiften/Network/UnitTest/BOSHConnectionTest.cpp +++ b/Swiften/Network/UnitTest/BOSHConnectionTest.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2016 Isode Limited. + * Copyright (c) 2011-2017 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -40,6 +40,8 @@ class BOSHConnectionTest : public CppUnit::TestFixture { CPPUNIT_TEST(testRead_Fragment); CPPUNIT_TEST(testHTTPRequest); CPPUNIT_TEST(testHTTPRequest_Empty); + CPPUNIT_TEST(testTerminate); + CPPUNIT_TEST(testTerminateWithAdditionalData); CPPUNIT_TEST_SUITE_END(); public: @@ -52,6 +54,7 @@ class BOSHConnectionTest : public CppUnit::TestFixture { connectFinished = false; disconnected = false; disconnectedError = false; + sessionTerminatedError.reset(); dataRead.clear(); } @@ -190,6 +193,39 @@ class BOSHConnectionTest : public CppUnit::TestFixture { CPPUNIT_ASSERT_EQUAL(fullBody, response.substr(bodyPosition+4)); } + void testTerminate() { + BOSHConnection::ref testling = createTestling(); + testling->connect(); + eventLoop->processEvents(); + testling->startStream("localhost", 1); + std::string initial("" + ""); + readResponse(initial, connectionFactory->connections[0]); + CPPUNIT_ASSERT(sessionTerminatedError); + CPPUNIT_ASSERT_EQUAL(BOSHError::BadRequest, sessionTerminatedError->getType()); + CPPUNIT_ASSERT_EQUAL(true, dataRead.empty()); + } + + // On a BOSH error no additional data may be emitted. + void testTerminateWithAdditionalData() { + BOSHConnection::ref testling = createTestling(); + testling->connect(); + eventLoop->processEvents(); + testling->startStream("localhost", 1); + std::string initial("" + "an error message" + ""); + readResponse(initial, connectionFactory->connections[0]); + CPPUNIT_ASSERT(sessionTerminatedError); + CPPUNIT_ASSERT_EQUAL(BOSHError::BadRequest, sessionTerminatedError->getType()); + CPPUNIT_ASSERT_EQUAL(true, dataRead.empty()); + } + + private: BOSHConnection::ref createTestling() { @@ -200,6 +236,7 @@ class BOSHConnectionTest : public CppUnit::TestFixture { c->onDisconnected.connect(boost::bind(&BOSHConnectionTest::handleDisconnected, this, _1)); c->onXMPPDataRead.connect(boost::bind(&BOSHConnectionTest::handleDataRead, this, _1)); c->onSessionStarted.connect(boost::bind(&BOSHConnectionTest::handleSID, this, _1)); + c->onSessionTerminated.connect(boost::bind(&BOSHConnectionTest::handleSessionTerminated, this, _1)); c->setRID(42); return c; } @@ -222,6 +259,10 @@ class BOSHConnectionTest : public CppUnit::TestFixture { sid = s; } + void handleSessionTerminated(BOSHError::ref error) { + sessionTerminatedError = error; + } + struct MockConnection : public Connection { public: MockConnection(const std::vector& failingPorts, EventLoop* eventLoop) : eventLoop(eventLoop), failingPorts(failingPorts), disconnected(false) { @@ -293,6 +334,7 @@ class BOSHConnectionTest : public CppUnit::TestFixture { bool connectFinishedWithError; bool disconnected; bool disconnectedError; + BOSHError::ref sessionTerminatedError; ByteArray dataRead; PlatformXMLParserFactory parserFactory; StaticDomainNameResolver* resolver; -- cgit v0.10.2-6-g49f6