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
author: Tobias Markmann <tm@ayena.de> 2017-03-13 09:01:54 (GMT)
committer: Kevin Smith <kevin.smith@isode.com> 2017-03-15 11:44:46 (GMT)
commit: 87d5f54a90f2759aae587f01650e44aee8f2fe3f (patch)
tree: c84015f0343228aba73642fdd2adc6322631f937
parent: 3368a6e5f220f00e03b3ebb41bdd8872ffdf81be (diff)
download: swift-87d5f54a90f2759aae587f01650e44aee8f2fe3f.zip
swift-87d5f54a90f2759aae587f01650e44aee8f2fe3f.tar.bz2
2 files changed, 45 insertions, 2 deletions
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
@@ -1,38 +1,38 @@
 /*
  * Copyright (c) 2011 Thilo Cestonaro
  * Licensed under the simplified BSD license.
  * See Documentation/Licenses/BSD-simplified.txt for more information.
  */
 
 /*
- * Copyright (c) 2011-2016 Isode Limited.
+ * Copyright (c) 2011-2017 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #include <Swiften/Network/BOSHConnection.h>
 
 #include <string>
 #include <thread>
 
 #include <boost/bind.hpp>
 #include <boost/lexical_cast.hpp>
 
 #include <Swiften/Base/ByteArray.h>
 #include <Swiften/Base/Concat.h>
 #include <Swiften/Base/Log.h>
 #include <Swiften/Base/String.h>
 #include <Swiften/Network/HostAddressPort.h>
 #include <Swiften/Parser/BOSHBodyExtractor.h>
 #include <Swiften/StreamStack/DummyStreamLayer.h>
 #include <Swiften/StreamStack/TLSLayer.h>
 #include <Swiften/TLS/TLSContext.h>
 #include <Swiften/TLS/TLSOptions.h>
 
 namespace Swift {
 
 BOSHConnection::BOSHConnection(const URL& boshURL, Connector::ref connector, XMLParserFactory* parserFactory, TLSContextFactory* tlsContextFactory, const TLSOptions& tlsOptions)
     : boshURL_(boshURL),
       connector_(connector),
       parserFactory_(parserFactory),
       sid_(),
@@ -269,60 +269,61 @@ void BOSHConnection::startStream(const std::string& to, unsigned long long rid)
             << "Content-Length: " << contentString.size() << "\r\n\r\n"
             << contentString;
 
     waitingForStartResponse_ = true;
     SafeByteArray safeHeader = createSafeByteArray(header.str());
     onBOSHDataWritten(safeHeader);
     writeData(safeHeader);
     SWIFT_LOG(debug) << "write stream header: " << safeByteArrayToString(safeHeader) << std::endl;
 }
 
 void BOSHConnection::handleDataRead(std::shared_ptr<SafeByteArray> data) {
     onBOSHDataRead(*data);
     buffer_ = concat(buffer_, *data);
     std::string response = safeByteArrayToString(buffer_);
     if (response.find("\r\n\r\n") == std::string::npos) {
         onBOSHDataRead(createSafeByteArray("[[Previous read incomplete, pending]]"));
         return;
     }
 
     std::string httpCode = response.substr(response.find(" ") + 1, 3);
     if (httpCode != "200") {
         onHTTPError(httpCode);
         return;
     }
 
     BOSHBodyExtractor parser(parserFactory_, createByteArray(response.substr(response.find("\r\n\r\n") + 4)));
     if (parser.getBody()) {
         if (parser.getBody()->attributes.getAttribute("type") == "terminate") {
             BOSHError::Type errorType = parseTerminationCondition(parser.getBody()->attributes.getAttribute("condition"));
             onSessionTerminated(errorType == BOSHError::NoError ? std::shared_ptr<BOSHError>() : std::make_shared<BOSHError>(errorType));
+            return;
         }
         buffer_.clear();
         if (waitingForStartResponse_) {
             waitingForStartResponse_ = false;
             sid_ = parser.getBody()->attributes.getAttribute("sid");
             std::string requestsString = parser.getBody()->attributes.getAttribute("requests");
             size_t requests = 2;
             if (!requestsString.empty()) {
                 try {
                     requests = boost::lexical_cast<size_t>(requestsString);
                 }
                 catch (const boost::bad_lexical_cast&) {
                 }
             }
             onSessionStarted(sid_, requests);
         }
         SafeByteArray payload = createSafeByteArray(parser.getBody()->content);
         /* Say we're good to go again, so don't add anything after here in the method */
         pending_ = false;
         onXMPPDataRead(payload);
     }
 
 }
 
 BOSHError::Type BOSHConnection::parseTerminationCondition(const std::string& text) {
     BOSHError::Type condition = BOSHError::UndefinedCondition;
     if (text == "bad-request") {
         condition = BOSHError::BadRequest;
     }
     else if (text == "host-gone") {
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,84 +1,87 @@
 /*
- * Copyright (c) 2011-2016 Isode Limited.
+ * Copyright (c) 2011-2017 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #include <memory>
 
 #include <boost/bind.hpp>
 #include <boost/lexical_cast.hpp>
 #include <boost/optional.hpp>
 
 #include <QA/Checker/IO.h>
 
 #include <cppunit/extensions/HelperMacros.h>
 #include <cppunit/extensions/TestFactoryRegistry.h>
 
 #include <Swiften/Base/Algorithm.h>
 #include <Swiften/EventLoop/DummyEventLoop.h>
 #include <Swiften/Network/BOSHConnection.h>
 #include <Swiften/Network/Connection.h>
 #include <Swiften/Network/ConnectionFactory.h>
 #include <Swiften/Network/DummyTimerFactory.h>
 #include <Swiften/Network/HostAddressPort.h>
 #include <Swiften/Network/StaticDomainNameResolver.h>
 #include <Swiften/Parser/PlatformXMLParserFactory.h>
 #include <Swiften/TLS/TLSOptions.h>
 
 using namespace Swift;
 
 class BOSHConnectionTest : public CppUnit::TestFixture {
     CPPUNIT_TEST_SUITE(BOSHConnectionTest);
     CPPUNIT_TEST(testHeader);
     CPPUNIT_TEST(testReadiness_ok);
     CPPUNIT_TEST(testReadiness_pending);
     CPPUNIT_TEST(testReadiness_disconnect);
     CPPUNIT_TEST(testReadiness_noSID);
     CPPUNIT_TEST(testWrite_Receive);
     CPPUNIT_TEST(testWrite_ReceiveTwice);
     CPPUNIT_TEST(testRead_Fragment);
     CPPUNIT_TEST(testHTTPRequest);
     CPPUNIT_TEST(testHTTPRequest_Empty);
+    CPPUNIT_TEST(testTerminate);
+    CPPUNIT_TEST(testTerminateWithAdditionalData);
     CPPUNIT_TEST_SUITE_END();
 
     public:
         void setUp() {
             eventLoop = new DummyEventLoop();
             connectionFactory = new MockConnectionFactory(eventLoop);
             resolver = new StaticDomainNameResolver(eventLoop);
             timerFactory = new DummyTimerFactory();
             tlsContextFactory = nullptr;
             connectFinished = false;
             disconnected = false;
             disconnectedError = false;
+            sessionTerminatedError.reset();
             dataRead.clear();
         }
 
         void tearDown() {
             eventLoop->processEvents();
             delete connectionFactory;
             delete resolver;
             delete timerFactory;
             delete eventLoop;
         }
 
         void testHeader() {
             BOSHConnection::ref testling = createTestling();
             testling->connect();
             eventLoop->processEvents();
             testling->startStream("wonderland.lit", 1);
             std::string initial("<body wait='60' "
                                 "inactivity='30' "
                                 "polling='5' "
                                 "requests='2' "
                                 "hold='1' "
                                 "maxpause='120' "
                                 "sid='MyShinySID' "
                                 "ver='1.6' "
                                 "from='wonderland.lit' "
                                 "xmlns='http://jabber.org/protocol/httpbind'/>");
             readResponse(initial, connectionFactory->connections[0]);
             CPPUNIT_ASSERT_EQUAL(std::string("MyShinySID"), sid);
             CPPUNIT_ASSERT(testling->isReadyToSend());
         }
@@ -163,92 +166,130 @@ class BOSHConnectionTest : public CppUnit::TestFixture {
                 "<bl"));
             std::shared_ptr<SafeByteArray> data3 = std::make_shared<SafeByteArray>(createSafeByteArray(
                 "ah/>"
                 "</body>"));
             connection->onDataRead(data1);
             connection->onDataRead(data2);
             CPPUNIT_ASSERT(dataRead.empty());
             connection->onDataRead(data3);
             CPPUNIT_ASSERT_EQUAL(std::string("<blah/>"), byteArrayToString(dataRead));
         }
 
         void testHTTPRequest() {
             std::string data = "<blah/>";
             std::string sid = "wigglebloom";
             std::string fullBody = "<body xmlns='http://jabber.org/protocol/httpbind' sid='" + sid + "' rid='20'>" + data + "</body>";
             std::pair<SafeByteArray, size_t> http = BOSHConnection::createHTTPRequest(createSafeByteArray(data), false, false, 20, sid, URL());
             CPPUNIT_ASSERT_EQUAL(fullBody.size(), http.second);
         }
 
         void testHTTPRequest_Empty() {
             std::string data = "";
             std::string sid = "wigglebloomsickle";
             std::string fullBody = "<body rid='42' sid='" + sid + "' xmlns='http://jabber.org/protocol/httpbind'>" + data + "</body>";
             std::pair<SafeByteArray, size_t> http = BOSHConnection::createHTTPRequest(createSafeByteArray(data), false, false, 42, sid, URL());
             CPPUNIT_ASSERT_EQUAL(fullBody.size(), http.second);
             std::string response = safeByteArrayToString(http.first);
             size_t bodyPosition = response.find("\r\n\r\n");
             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("<body xmlns=\"http://jabber.org/protocol/httpbind\" "
+                                    "condition=\"bad-request\" "
+                                    "type=\"terminate\">"
+                                "</body>");
+            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("<body xmlns=\"http://jabber.org/protocol/httpbind\" "
+                                    "condition=\"bad-request\" "
+                                    "type=\"terminate\">"
+                                    "<text>an error message</text>"
+                                "</body>");
+            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() {
             resolver->addAddress("wonderland.lit", HostAddress::fromString("127.0.0.1").get());
             Connector::ref connector = Connector::create("wonderland.lit", 5280, boost::optional<std::string>(), resolver, connectionFactory, timerFactory);
             BOSHConnection::ref c = BOSHConnection::create(URL("http", "wonderland.lit", 5280, "/http-bind"), connector, &parserFactory, tlsContextFactory, TLSOptions());
             c->onConnectFinished.connect(boost::bind(&BOSHConnectionTest::handleConnectFinished, this, _1));
             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;
         }
 
         void handleConnectFinished(bool error) {
             connectFinished = true;
             connectFinishedWithError = error;
         }
 
         void handleDisconnected(bool e) {
             disconnected = true;
             disconnectedError = e;
         }
 
         void handleDataRead(const SafeByteArray& d) {
             append(dataRead, d);
         }
 
         void handleSID(const std::string& s) {
             sid = s;
         }
 
+        void handleSessionTerminated(BOSHError::ref error) {
+            sessionTerminatedError = error;
+        }
+
         struct MockConnection : public Connection {
             public:
                 MockConnection(const std::vector<HostAddressPort>& failingPorts, EventLoop* eventLoop) : eventLoop(eventLoop), failingPorts(failingPorts), disconnected(false) {
                 }
 
                 void listen() { assert(false); }
 
                 void connect(const HostAddressPort& address) {
                     hostAddressPort = address;
                     bool fail = std::find(failingPorts.begin(), failingPorts.end(), address) != failingPorts.end();
                     eventLoop->postEvent(boost::bind(boost::ref(onConnectFinished), fail));
                 }
 
                 HostAddressPort getLocalAddress() const { return HostAddressPort(); }
                 HostAddressPort getRemoteAddress() const { return HostAddressPort(); }
 
                 void disconnect() {
                     disconnected = true;
                     onDisconnected(boost::optional<Connection::Error>());
                 }
 
                 void write(const SafeByteArray& d) {
                     append(dataWritten, d);
                 }
 
                 EventLoop* eventLoop;
                 boost::optional<HostAddressPort> hostAddressPort;
                 std::vector<HostAddressPort> failingPorts;
                 ByteArray dataWritten;
                 bool disconnected;
@@ -266,41 +307,42 @@ class BOSHConnectionTest : public CppUnit::TestFixture {
 
             EventLoop* eventLoop;
             std::vector< std::shared_ptr<MockConnection> > connections;
             std::vector<HostAddressPort> failingPorts;
         };
 
         void readResponse(const std::string& response, std::shared_ptr<MockConnection> connection) {
             std::shared_ptr<SafeByteArray> data1 = std::make_shared<SafeByteArray>(createSafeByteArray(
                 "HTTP/1.1 200 OK\r\n"
                 "Content-Type: text/xml; charset=utf-8\r\n"
                 "Access-Control-Allow-Origin: *\r\n"
                 "Access-Control-Allow-Headers: Content-Type\r\n"
                 "Content-Length: "));
             connection->onDataRead(data1);
             std::shared_ptr<SafeByteArray> data2 = std::make_shared<SafeByteArray>(createSafeByteArray(boost::lexical_cast<std::string>(response.size())));
             connection->onDataRead(data2);
             std::shared_ptr<SafeByteArray> data3 = std::make_shared<SafeByteArray>(createSafeByteArray("\r\n\r\n"));
             connection->onDataRead(data3);
             std::shared_ptr<SafeByteArray> data4 = std::make_shared<SafeByteArray>(createSafeByteArray(response));
             connection->onDataRead(data4);
         }
 
 
     private:
         DummyEventLoop* eventLoop;
         MockConnectionFactory* connectionFactory;
         bool connectFinished;
         bool connectFinishedWithError;
         bool disconnected;
         bool disconnectedError;
+        BOSHError::ref sessionTerminatedError;
         ByteArray dataRead;
         PlatformXMLParserFactory parserFactory;
         StaticDomainNameResolver* resolver;
         TimerFactory* timerFactory;
         TLSContextFactory* tlsContextFactory;
         std::string sid;
 
 };
 
 
 CPPUNIT_TEST_SUITE_REGISTRATION(BOSHConnectionTest);
author	Tobias Markmann <tm@ayena.de>	2017-03-13 09:01:54 (GMT)
committer	Kevin Smith <kevin.smith@isode.com>	2017-03-15 11:44:46 (GMT)
commit	87d5f54a90f2759aae587f01650e44aee8f2fe3f (patch)
tree	c84015f0343228aba73642fdd2adc6322631f937
parent	3368a6e5f220f00e03b3ebb41bdd8872ffdf81be (diff)
download	swift-87d5f54a90f2759aae587f01650e44aee8f2fe3f.zip swift-87d5f54a90f2759aae587f01650e44aee8f2fe3f.tar.bz2