Swift
summaryrefslogtreecommitdiffstats
path: root/Swiften
diff options
context:
space:
mode:
authorTobias Markmann <tm@ayena.de>2016-02-11 14:50:37 (GMT)
committerKevin Smith <kevin.smith@isode.com>2016-02-15 13:05:18 (GMT)
commit75703db2de5bbfb6622286600362016edb42dfb0 (patch)
tree2520ed777286c6b732e756387ca88d49b9c1814e /Swiften
parentca226e7bb019308db4bfc818d7e04422d9d28106 (diff)
downloadswift-75703db2de5bbfb6622286600362016edb42dfb0.zip
swift-75703db2de5bbfb6622286600362016edb42dfb0.tar.bz2
Support early IBB use in Jingle File Transfer
Previously Jingle File Transfer in Swiften only used IBB transport as fallback mechanism. With this patch Swiften will use IBB transport candidates directly in the first session-initate/session-accept message if the other party only supports IBB. Fixed a ASAN reported heap-use-after-free in SOCKS5BytestreamServerManager.cpp while testing. Test-Information: ./scons test=system passed without error. Testing all sender/receiver file-transfer option configurations with FileTransferTest resulting in expected behavior. Successfully transferring a file between two Swift instances. Change-Id: Ia0ffeaa1fd54fc0da23db75344c9e94f9d03a774
Diffstat (limited to 'Swiften')
-rw-r--r--Swiften/FileTransfer/IncomingFileTransferManager.cpp4
-rw-r--r--Swiften/FileTransfer/IncomingJingleFileTransfer.cpp14
-rw-r--r--Swiften/FileTransfer/OutgoingJingleFileTransfer.cpp40
-rw-r--r--Swiften/FileTransfer/SOCKS5BytestreamServerManager.cpp6
-rw-r--r--Swiften/FileTransfer/UnitTest/DummyFileTransferTransporterFactory.h16
-rw-r--r--Swiften/FileTransfer/UnitTest/OutgoingJingleFileTransferTest.cpp11
-rw-r--r--Swiften/QA/FileTransferTest/FileTransferTest.cpp45
7 files changed, 98 insertions, 38 deletions
diff --git a/Swiften/FileTransfer/IncomingFileTransferManager.cpp b/Swiften/FileTransfer/IncomingFileTransferManager.cpp
index c1cc757..f5b95ec 100644
--- a/Swiften/FileTransfer/IncomingFileTransferManager.cpp
+++ b/Swiften/FileTransfer/IncomingFileTransferManager.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2010-2015 Isode Limited.
+ * Copyright (c) 2010-2016 Isode Limited.
* All rights reserved.
* See the COPYING file for more information.
*/
@@ -40,7 +40,7 @@ bool IncomingFileTransferManager::handleIncomingJingleSession(
const std::vector<JingleContentPayload::ref>& contents,
const JID& recipient) {
if (JingleContentPayload::ref content = Jingle::getContentWithDescription<JingleFileTransferDescription>(contents)) {
- if (content->getTransport<JingleS5BTransportPayload>()) {
+ if (content->getTransport<JingleS5BTransportPayload>() || content->getTransport<JingleIBBTransportPayload>()) {
JingleFileTransferDescription::ref description = content->getDescription<JingleFileTransferDescription>();
if (description) {
IncomingJingleFileTransfer::ref transfer = boost::make_shared<IncomingJingleFileTransfer>(
diff --git a/Swiften/FileTransfer/IncomingJingleFileTransfer.cpp b/Swiften/FileTransfer/IncomingJingleFileTransfer.cpp
index 8cb1cab..db17620 100644
--- a/Swiften/FileTransfer/IncomingJingleFileTransfer.cpp
+++ b/Swiften/FileTransfer/IncomingJingleFileTransfer.cpp
@@ -17,6 +17,7 @@
#include <Swiften/Elements/JingleFileTransferHash.h>
#include <Swiften/Elements/JingleIBBTransportPayload.h>
#include <Swiften/Elements/JingleS5BTransportPayload.h>
+#include <Swiften/FileTransfer/FileTransferOptions.h>
#include <Swiften/FileTransfer/FileTransferTransporter.h>
#include <Swiften/FileTransfer/FileTransferTransporterFactory.h>
#include <Swiften/FileTransfer/IncrementalBytestreamHashCalculator.h>
@@ -82,7 +83,9 @@ void IncomingJingleFileTransfer::accept(
writeStreamDataReceivedConnection = stream->onWrite.connect(
boost::bind(&IncomingJingleFileTransfer::handleWriteStreamDataReceived, this, _1));
- if (JingleS5BTransportPayload::ref s5bTransport = initialContent->getTransport<JingleS5BTransportPayload>()) {
+ JingleS5BTransportPayload::ref s5bTransport = initialContent->getTransport<JingleS5BTransportPayload>();
+ JingleIBBTransportPayload::ref ibbTransport = initialContent->getTransport<JingleIBBTransportPayload>();
+ if (s5bTransport) {
SWIFT_LOG(debug) << "Got S5B transport as initial payload." << std::endl;
setTransporter(transporterFactory->createResponderTransporter(
getInitiator(), getResponder(), s5bTransport->getSessionID(), options));
@@ -90,7 +93,7 @@ void IncomingJingleFileTransfer::accept(
setState(GeneratingInitialLocalCandidates);
transporter->startGeneratingLocalCandidates();
}
- else if (JingleIBBTransportPayload::ref ibbTransport = initialContent->getTransport<JingleIBBTransportPayload>()) {
+ else if (ibbTransport && options.isInBandAllowed()) {
SWIFT_LOG(debug) << "Got IBB transport as initial payload." << std::endl;
setTransporter(transporterFactory->createResponderTransporter(
getInitiator(), getResponder(), ibbTransport->getSessionID(), options));
@@ -103,8 +106,9 @@ void IncomingJingleFileTransfer::accept(
session->sendAccept(getContentID(), initialContent->getDescriptions()[0], ibbTransport);
}
else {
- // Can't happen, because the transfer would have been rejected automatically
- assert(false);
+ // This might happen on incoming transfer which only list transport methods we are not allowed to use due to file-transfer options.
+ session->sendTerminate(JinglePayload::Reason::UnsupportedTransports);
+ setFinishedState(FileTransfer::State::Failed, FileTransferError(FileTransferError::PeerError));
}
}
@@ -223,7 +227,7 @@ void IncomingJingleFileTransfer::handleWriteStreamDataReceived(
void IncomingJingleFileTransfer::handleTransportReplaceReceived(
const JingleContentID& content, JingleTransportPayload::ref transport) {
SWIFT_LOG(debug) << std::endl;
- if (state != WaitingForFallbackOrTerminate) {
+ if (state != WaitingForFallbackOrTerminate) {
SWIFT_LOG(warning) << "Incorrect state" << std::endl;
return;
}
diff --git a/Swiften/FileTransfer/OutgoingJingleFileTransfer.cpp b/Swiften/FileTransfer/OutgoingJingleFileTransfer.cpp
index a72d5ef..2c43766 100644
--- a/Swiften/FileTransfer/OutgoingJingleFileTransfer.cpp
+++ b/Swiften/FileTransfer/OutgoingJingleFileTransfer.cpp
@@ -93,9 +93,15 @@ void OutgoingJingleFileTransfer::start() {
return;
}
- setTransporter(transporterFactory->createInitiatorTransporter(getInitiator(), getResponder(), options));
- setInternalState(GeneratingInitialLocalCandidates);
- transporter->startGeneratingLocalCandidates();
+ if (!options.isInBandAllowed() && !options.isDirectAllowed() && !options.isAssistedAllowed() && !options.isProxiedAllowed()) {
+ // Started outgoing file transfer while not supporting transport methods.
+ setFinishedState(FileTransfer::State::Failed, FileTransferError(FileTransferError::UnknownError));
+ }
+ else {
+ setTransporter(transporterFactory->createInitiatorTransporter(getInitiator(), getResponder(), options));
+ setInternalState(GeneratingInitialLocalCandidates);
+ transporter->startGeneratingLocalCandidates();
+ }
}
void OutgoingJingleFileTransfer::cancel() {
@@ -124,6 +130,9 @@ void OutgoingJingleFileTransfer::handleSessionAcceptReceived(
setInternalState(TryingCandidates);
transporter->startTryingRemoteCandidates();
}
+ else if (JingleIBBTransportPayload::ref ibbPayload = boost::dynamic_pointer_cast<JingleIBBTransportPayload>(transportPayload)) {
+ startTransferring(transporter->createIBBSendSession(ibbPayload->getSessionID(), ibbPayload->getBlockSize().get_value_or(DEFAULT_BLOCK_SIZE), stream));
+ }
else {
SWIFT_LOG(debug) << "Unknown transport payload. Falling back." << std::endl;
fallback();
@@ -192,13 +201,24 @@ void OutgoingJingleFileTransfer::handleLocalTransportCandidatesGenerated(
fileInfo.addHash(HashElement("md5", ByteArray()));
description->setFileInfo(fileInfo);
- JingleS5BTransportPayload::ref transport = boost::make_shared<JingleS5BTransportPayload>();
- transport->setSessionID(s5bSessionID);
- transport->setMode(JingleS5BTransportPayload::TCPMode);
- transport->setDstAddr(dstAddr);
- foreach(JingleS5BTransportPayload::Candidate candidate, candidates) {
- transport->addCandidate(candidate);
- SWIFT_LOG(debug) << "\t" << "S5B candidate: " << candidate.hostPort.toString() << std::endl;
+ JingleTransportPayload::ref transport;
+ if (candidates.empty()) {
+ SWIFT_LOG(debug) << "no S5B candidates generated. Send IBB transport candidate." << std::endl;
+ JingleIBBTransportPayload::ref ibbTransport = boost::make_shared<JingleIBBTransportPayload>();
+ ibbTransport->setBlockSize(DEFAULT_BLOCK_SIZE);
+ ibbTransport->setSessionID(idGenerator->generateID());
+ transport = ibbTransport;
+ }
+ else {
+ JingleS5BTransportPayload::ref s5bTransport = boost::make_shared<JingleS5BTransportPayload>();
+ s5bTransport->setSessionID(s5bSessionID);
+ s5bTransport->setMode(JingleS5BTransportPayload::TCPMode);
+ s5bTransport->setDstAddr(dstAddr);
+ foreach(JingleS5BTransportPayload::Candidate candidate, candidates) {
+ s5bTransport->addCandidate(candidate);
+ SWIFT_LOG(debug) << "\t" << "S5B candidate: " << candidate.hostPort.toString() << std::endl;
+ }
+ transport = s5bTransport;
}
setInternalState(WaitingForAccept);
session->sendInitiate(contentID, description, transport);
diff --git a/Swiften/FileTransfer/SOCKS5BytestreamServerManager.cpp b/Swiften/FileTransfer/SOCKS5BytestreamServerManager.cpp
index 7b97698..3b1be89 100644
--- a/Swiften/FileTransfer/SOCKS5BytestreamServerManager.cpp
+++ b/Swiften/FileTransfer/SOCKS5BytestreamServerManager.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012-2015 Isode Limited.
+ * Copyright (c) 2012-2016 Isode Limited.
* All rights reserved.
* See the COPYING file for more information.
*/
@@ -192,6 +192,10 @@ void SOCKS5BytestreamServerManager::stop() {
forwardPortRequest->stop();
forwardPortRequest.reset();
}
+ if (unforwardPortRequest) {
+ unforwardPortRequest->stop();
+ unforwardPortRequest.reset();
+ }
if (server) {
server->stop();
delete server;
diff --git a/Swiften/FileTransfer/UnitTest/DummyFileTransferTransporterFactory.h b/Swiften/FileTransfer/UnitTest/DummyFileTransferTransporterFactory.h
index 324404d..00a931f 100644
--- a/Swiften/FileTransfer/UnitTest/DummyFileTransferTransporterFactory.h
+++ b/Swiften/FileTransfer/UnitTest/DummyFileTransferTransporterFactory.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015 Isode Limited.
+ * Copyright (c) 2015-2016 Isode Limited.
* All rights reserved.
* See the COPYING file for more information.
*/
@@ -9,19 +9,20 @@
#include <string>
#include <Swiften/Base/API.h>
-#include <Swiften/StringCodecs/Hexify.h>
#include <Swiften/Crypto/CryptoProvider.h>
+#include <Swiften/FileTransfer/FileTransferOptions.h>
#include <Swiften/FileTransfer/FileTransferTransporter.h>
#include <Swiften/FileTransfer/FileTransferTransporterFactory.h>
#include <Swiften/FileTransfer/IBBReceiveSession.h>
#include <Swiften/FileTransfer/IBBReceiveTransportSession.h>
-#include <Swiften/FileTransfer/IBBSendTransportSession.h>
#include <Swiften/FileTransfer/IBBSendSession.h>
+#include <Swiften/FileTransfer/IBBSendTransportSession.h>
#include <Swiften/FileTransfer/SOCKS5BytestreamProxiesManager.h>
#include <Swiften/FileTransfer/SOCKS5BytestreamRegistry.h>
#include <Swiften/FileTransfer/SOCKS5BytestreamServerManager.h>
#include <Swiften/FileTransfer/TransportSession.h>
#include <Swiften/JID/JID.h>
+#include <Swiften/StringCodecs/Hexify.h>
namespace Swift {
@@ -45,7 +46,7 @@ public:
TimerFactory*,
CryptoProvider* cryptoProvider,
IQRouter* iqRouter,
- const FileTransferOptions&) : initiator_(initiator), responder_(responder), role_(role), s5bRegistry_(s5bRegistry), crypto_(cryptoProvider), iqRouter_(iqRouter) {
+ const FileTransferOptions& ftOptions) : initiator_(initiator), responder_(responder), role_(role), s5bRegistry_(s5bRegistry), crypto_(cryptoProvider), iqRouter_(iqRouter), ftOptions_(ftOptions) {
}
@@ -55,6 +56,12 @@ public:
virtual void startGeneratingLocalCandidates() {
std::vector<JingleS5BTransportPayload::Candidate> candidates;
+ if (ftOptions_.isDirectAllowed()) {
+ JingleS5BTransportPayload::Candidate candidate;
+ candidate.cid = "123";
+ candidate.priority = 1235;
+ candidates.push_back(candidate);
+ }
onLocalCandidatesGenerated(s5bSessionID_, candidates, getSOCKS5DstAddr());
}
@@ -139,6 +146,7 @@ private:
CryptoProvider* crypto_;
std::string s5bSessionID_;
IQRouter* iqRouter_;
+ FileTransferOptions ftOptions_;
};
class DummyFileTransferTransporterFactory : public FileTransferTransporterFactory {
diff --git a/Swiften/FileTransfer/UnitTest/OutgoingJingleFileTransferTest.cpp b/Swiften/FileTransfer/UnitTest/OutgoingJingleFileTransferTest.cpp
index f3fe42e..fee26d5 100644
--- a/Swiften/FileTransfer/UnitTest/OutgoingJingleFileTransferTest.cpp
+++ b/Swiften/FileTransfer/UnitTest/OutgoingJingleFileTransferTest.cpp
@@ -5,7 +5,7 @@
*/
/*
- * Copyright (c) 2013-2015 Isode Limited.
+ * Copyright (c) 2013-2016 Isode Limited.
* All rights reserved.
* See the COPYING file for more information.
*/
@@ -81,7 +81,7 @@ class OutgoingJingleFileTransferTest : public CppUnit::TestFixture {
public:
- boost::shared_ptr<OutgoingJingleFileTransfer> createTestling() {
+ boost::shared_ptr<OutgoingJingleFileTransfer> createTestling(const FileTransferOptions& options = FileTransferOptions().withAssistedAllowed(false).withDirectAllowed(false).withProxiedAllowed(false)) {
JID to("test@foo.com/bla");
JingleFileTransferFileInfo fileInfo;
fileInfo.setDescription("some file");
@@ -96,7 +96,7 @@ public:
timerFactory,
idGen,
fileInfo,
- FileTransferOptions().withAssistedAllowed(false).withDirectAllowed(false).withProxiedAllowed(false),
+ options,
crypto.get()));
}
@@ -159,16 +159,17 @@ public:
CPPUNIT_ASSERT(description);
CPPUNIT_ASSERT(static_cast<size_t>(1048576) == description->getFileInfo().getSize());
- JingleS5BTransportPayload::ref transport = boost::dynamic_pointer_cast<JingleS5BTransportPayload>(call.payload);
+ JingleIBBTransportPayload::ref transport = boost::dynamic_pointer_cast<JingleIBBTransportPayload>(call.payload);
CPPUNIT_ASSERT(transport);
}
void test_FallbackToIBBAfterFailingS5B() {
- boost::shared_ptr<OutgoingJingleFileTransfer> transfer = createTestling();
+ boost::shared_ptr<OutgoingJingleFileTransfer> transfer = createTestling(FileTransferOptions().withAssistedAllowed(true).withDirectAllowed(true).withProxiedAllowed(true));
transfer->start();
FakeJingleSession::InitiateCall call = getCall<FakeJingleSession::InitiateCall>(0);
+ CPPUNIT_ASSERT(boost::dynamic_pointer_cast<JingleS5BTransportPayload>(call.payload));
fakeJingleSession->handleSessionAcceptReceived(call.id, call.description, call.payload);
// send candidate failure
diff --git a/Swiften/QA/FileTransferTest/FileTransferTest.cpp b/Swiften/QA/FileTransferTest/FileTransferTest.cpp
index 8597033..7a50e9f 100644
--- a/Swiften/QA/FileTransferTest/FileTransferTest.cpp
+++ b/Swiften/QA/FileTransferTest/FileTransferTest.cpp
@@ -49,7 +49,7 @@ enum Candidate {
class FileTransferTest {
public:
- FileTransferTest(int senderCandidates, int receiverCandidates) : senderCandidates_(senderCandidates), senderIsDone_(false), receiverCandidates_(receiverCandidates), receiverIsDone_(false) {
+ FileTransferTest(int senderCandidates, int receiverCandidates) : senderCandidates_(senderCandidates), senderError_(FileTransferError::UnknownError), senderIsDone_(false), receiverCandidates_(receiverCandidates), receiverError_(FileTransferError::UnknownError), receiverIsDone_(false) {
sender_ = boost::make_shared<Client>(JID(getenv("SWIFT_FILETRANSFERTEST_JID")), getenv("SWIFT_FILETRANSFERTEST_PASS"), networkFactories.get());
sender_->onDisconnected.connect(boost::bind(&FileTransferTest::handleSenderDisconnected, this, _1));
sender_->onConnected.connect(boost::bind(&FileTransferTest::handleSenderConnected, this));
@@ -108,6 +108,19 @@ class FileTransferTest {
}
void handleSenderConnected() {
+ DiscoInfo discoInfo;
+ discoInfo.addIdentity(DiscoInfo::Identity(CLIENT_NAME, "client", "pc"));
+ discoInfo.addFeature(DiscoInfo::JingleFeature);
+ discoInfo.addFeature(DiscoInfo::JingleFTFeature);
+ discoInfo.addFeature(DiscoInfo::Bytestream);
+ if (senderCandidates_ & InBandBytestream) {
+ discoInfo.addFeature(DiscoInfo::JingleTransportsIBBFeature);
+ }
+ if (senderCandidates_ & (S5B_Direct | S5B_Assisted | S5B_Proxied)) {
+ discoInfo.addFeature(DiscoInfo::JingleTransportsS5BFeature);
+ }
+ sender_->getDiscoManager()->setCapsNode(CLIENT_NODE);
+ sender_->getDiscoManager()->setDiscoInfo(discoInfo);
sender_->sendPresence(Presence::create());
}
@@ -119,8 +132,12 @@ class FileTransferTest {
discoInfo.addFeature(DiscoInfo::JingleFeature);
discoInfo.addFeature(DiscoInfo::JingleFTFeature);
discoInfo.addFeature(DiscoInfo::Bytestream);
- discoInfo.addFeature(DiscoInfo::JingleTransportsIBBFeature);
- discoInfo.addFeature(DiscoInfo::JingleTransportsS5BFeature);
+ if (receiverCandidates_ & InBandBytestream) {
+ discoInfo.addFeature(DiscoInfo::JingleTransportsIBBFeature);
+ }
+ if (receiverCandidates_ & (S5B_Direct | S5B_Assisted | S5B_Proxied)) {
+ discoInfo.addFeature(DiscoInfo::JingleTransportsS5BFeature);
+ }
receiver_->getDiscoManager()->setCapsNode(CLIENT_NODE);
receiver_->getDiscoManager()->setDiscoInfo(discoInfo);
receiver_->getPresenceSender()->sendPresence(Presence::create());
@@ -167,6 +184,8 @@ class FileTransferTest {
outgoingFileTransfer_->start();
} else {
std::cout << "ERROR: No outgoing file transfer returned." << std::endl;
+ receiverIsDone_ = true;
+ senderIsDone_ = true;
endTest();
}
}
@@ -223,6 +242,9 @@ class FileTransferTest {
timeOut_->onTick.connect(boost::bind(&FileTransferTest::endTest, this));
timeOut_->start();
}
+ else if (error) {
+ endTest();
+ }
}
void run() {
@@ -287,16 +309,17 @@ static bool runTest(int senderCandidates, int receiverCandidates) {
testRun->run();
- if (testRun->isDone()) {
- bool wasSuccessful = testRun->wasSuccessful();
- if (expectSuccess == wasSuccessful) {
- success = true;
- } else {
- std::cout << "expected success: " << expectSuccess << ", wasSuccessful: " << wasSuccessful << std::endl;
+ bool wasSuccessful = testRun->wasSuccessful();
+ if (expectSuccess == wasSuccessful) {
+ success = true;
+ }
+ else {
+ if (!testRun->isDone()) {
+ std::cout << "Test did not finish transfer. Sender candidates = " << senderCandidates << ", receiver candidates = " << receiverCandidates << "." << std::endl;
}
- } else {
- std::cout << "Failed to run test! Sender candidates = " << senderCandidates << ", receiver candidates = " << receiverCandidates << "." << std::endl;
}
+ std::cout << "expected success: " << expectSuccess << ", wasSuccessful: " << wasSuccessful << std::endl;
+
testRun.reset();
networkFactories.reset();