Fix crash when saving a received file to non-writable location

WriteBytestream::write(…) now returns a boolean indicating its success state (false in case of an error). Adjusted FileWriteBytestream accordingly. The QtWebKitChatView will test if the file path selected by the user is writable before accepting it and starting the transfer. If it is not writable a red warning message will be added to the file-transfer element in the chat view. Test-Information: Added an integration test that tests the new behavior for the FileWriteBytestream class. Tested two file transfers on OS X 10.11.3, one to a write protected location and another to /tmp. The first is not accepted by the UI, and without the UI sanity check it results in a file-transfer error. The second succeeds as expected. Change-Id: I5aa0c617423073feb371365a23a294c149c88036
author: Tobias Markmann <tm@ayena.de> 2016-02-10 19:25:21 (GMT)
committer: Kevin Smith <kevin.smith@isode.com> 2016-02-15 12:31:10 (GMT)
commit: ca226e7bb019308db4bfc818d7e04422d9d28106 (patch)
tree: 9f7ef065f20db3e04eb08560674eed16d10e95e5
parent: 431eb62386101dc8fc1e7d346c49bd0d81fda70e (diff)
download: swift-ca226e7bb019308db4bfc818d7e04422d9d28106.zip
swift-ca226e7bb019308db4bfc818d7e04422d9d28106.tar.bz2
14 files changed, 171 insertions, 53 deletions
diff --git a/Swift/QtUI/QtWebKitChatView.cpp b/Swift/QtUI/QtWebKitChatView.cpp
index 260da8a..b543e34 100644
--- a/Swift/QtUI/QtWebKitChatView.cpp
+++ b/Swift/QtUI/QtWebKitChatView.cpp
@@ -1,43 +1,44 @@
 /*
- * Copyright (c) 2010-2015 Isode Limited.
+ * Copyright (c) 2010-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #include <Swift/QtUI/QtWebKitChatView.h>
 
 #include <QApplication>
 #include <QDesktopServices>
 #include <QEventLoop>
 #include <QFile>
 #include <QFileDialog>
+#include <QFileInfo>
 #include <QInputDialog>
 #include <QKeyEvent>
 #include <QMessageBox>
 #include <QStackedWidget>
 #include <QTimer>
 #include <QVBoxLayout>
 #include <QWebFrame>
 #include <QtDebug>
 
 #include <Swiften/Base/FileSize.h>
 #include <Swiften/Base/Log.h>
 #include <Swiften/StringCodecs/Base64.h>
 
 #include <Swift/Controllers/UIEvents/JoinMUCUIEvent.h>
 #include <Swift/Controllers/UIEvents/UIEventStream.h>
 
 #include <Swift/QtUI/MessageSnippet.h>
 #include <Swift/QtUI/QtChatWindow.h>
 #include <Swift/QtUI/QtChatWindowJSBridge.h>
 #include <Swift/QtUI/QtScaledAvatarCache.h>
 #include <Swift/QtUI/QtSwiftUtil.h>
 #include <Swift/QtUI/QtUtilities.h>
 #include <Swift/QtUI/QtWebView.h>
 #include <Swift/QtUI/SystemMessageSnippet.h>
 
 namespace Swift {
 
 const QString QtWebKitChatView::ButtonWhiteboardSessionCancel = QString("whiteboard-cancel");
 const QString QtWebKitChatView::ButtonWhiteboardSessionAcceptRequest = QString("whiteboard-acceptrequest");
 const QString QtWebKitChatView::ButtonWhiteboardShowWindow = QString("whiteboard-showwindow");
@@ -720,93 +721,141 @@ void QtWebKitChatView::setFileTransferStatus(std::string id, const ChatWindow::F
 }
 
 std::string QtWebKitChatView::addWhiteboardRequest(const QString& contact, bool senderIsSelf) {
 	QString wb_id = QString("wb%1").arg(P2QSTRING(boost::lexical_cast<std::string>(idCounter_++)));
 	QString htmlString;
 	QString actionText;
 	if (senderIsSelf) {
 		actionText = tr("Starting whiteboard chat");
 		htmlString = "<div id='" + wb_id + "'>" + actionText + "<br />"+
 				buildChatWindowButton(tr("Cancel"), ButtonWhiteboardSessionCancel, wb_id) +
 			"</div>";
 	} else {
 		actionText = tr("%1 would like to start a whiteboard chat");
 		htmlString = "<div id='" + wb_id + "'>" + actionText.arg(QtUtilities::htmlEscape(contact)) + ": <br/>" +
 				buildChatWindowButton(tr("Cancel"), ButtonWhiteboardSessionCancel, wb_id) +
 				buildChatWindowButton(tr("Accept"), ButtonWhiteboardSessionAcceptRequest, wb_id) +
 			"</div>";
 	}
 	QString qAvatarPath = "qrc:/icons/avatar.png";
 	std::string id = "wbmessage" + boost::lexical_cast<std::string>(idCounter_++);
 	addMessageBottom(boost::make_shared<MessageSnippet>(htmlString, QtUtilities::htmlEscape(contact), B2QDATE(boost::posix_time::second_clock::universal_time()), qAvatarPath, false, false, theme_, P2QSTRING(id), ChatSnippet::getDirection(actionText)));
 	previousMessageWasSelf_ = false;
 	previousSenderName_ = contact;
 	return Q2PSTRING(wb_id);
 }
 
 void QtWebKitChatView::setWhiteboardSessionStatus(const std::string& id, const ChatWindow::WhiteboardSessionState state) {
 	setWhiteboardSessionStatus(P2QSTRING(id), state);
 }
 
+static bool isFilePathWritable(const QString& path) {
+	QFileInfo fileInfo = QFileInfo(path);
+	if (fileInfo.exists()) {
+		return fileInfo.isWritable();
+	}
+	else {
+		bool writable = false;
+		QFile writeTestFile(path);
+		if (writeTestFile.open(QIODevice::ReadWrite)) {
+			writeTestFile.write("test");
+			if (writeTestFile.error() == QFileDevice::NoError) {
+				writable = true;
+			}
+		}
+		writeTestFile.close();
+		writeTestFile.remove();
+		return writable;
+	}
+}
+
+void QtWebKitChatView::setFileTransferWarning(QString id, QString warningText) {
+	QWebElement ftElement = findElementWithID(document_, "div", id);
+	if (ftElement.isNull()) {
+		SWIFT_LOG(debug) << "Tried to access FT UI via invalid id! id = " << Q2PSTRING(id) << std::endl;
+		return;
+	}
+
+	removeFileTransferWarning(id);
+	ftElement.appendInside(QString("<div class='ft_warning' style='color: red;'><br/>%1</div>").arg(QtUtilities::htmlEscape(warningText)));
+}
+
+void QtWebKitChatView::removeFileTransferWarning(QString id) {
+	QWebElement ftElement = findElementWithID(document_, "div", id);
+	if (ftElement.isNull()) {
+		SWIFT_LOG(debug) << "Tried to access FT UI via invalid id! id = " << Q2PSTRING(id) << std::endl;
+		return;
+	}
+
+	QWebElement warningElement = ftElement.findFirst(".ft_warning");
+	if (!warningElement.isNull()) {
+		warningElement.removeFromDocument();
+	}
+}
+
 void QtWebKitChatView::handleHTMLButtonClicked(QString id, QString encodedArgument1, QString encodedArgument2, QString encodedArgument3, QString encodedArgument4, QString encodedArgument5) {
 	QString arg1 = decodeButtonArgument(encodedArgument1);
 	QString arg2 = decodeButtonArgument(encodedArgument2);
 	QString arg3 = decodeButtonArgument(encodedArgument3);
 	QString arg4 = decodeButtonArgument(encodedArgument4);
 	QString arg5 = decodeButtonArgument(encodedArgument5);
 
 	if (id.startsWith(ButtonFileTransferCancel)) {
 		QString ft_id = arg1;
 		window_->onFileTransferCancel(Q2PSTRING(ft_id));
 	}
 	else if (id.startsWith(ButtonFileTransferSetDescription)) {
 		QString ft_id = arg1;
 		bool ok = false;
 		QString text = QInputDialog::getText(this, tr("File transfer description"),
 			tr("Description:"), QLineEdit::Normal, "", &ok);
 		if (ok) {
 			descriptions_[ft_id] = text;
 		}
 	}
 	else if (id.startsWith(ButtonFileTransferSendRequest)) {
 		QString ft_id = arg1;
 		QString text = descriptions_.find(ft_id) == descriptions_.end() ? QString() : descriptions_[ft_id];
 		window_->onFileTransferStart(Q2PSTRING(ft_id), Q2PSTRING(text));
 	}
 	else if (id.startsWith(ButtonFileTransferAcceptRequest)) {
 		QString ft_id = arg1;
 		QString filename = arg2;
 
 		QString path = QFileDialog::getSaveFileName(this, tr("Save File"), filename);
-		if (!path.isEmpty()) {
+		if (!path.isEmpty() && isFilePathWritable(path)) {
 			filePaths_[ft_id] = path;
 			window_->onFileTransferAccept(Q2PSTRING(ft_id), Q2PSTRING(path));
+			removeFileTransferWarning(ft_id);
+		}
+		else {
+			setFileTransferWarning(ft_id, tr("The chosen save location is not writable! Click the 'Accept' button to select a different save location."));
 		}
 	}
 	else if (id.startsWith(ButtonFileTransferOpenFile)) {
 		QString ft_id = arg1;
 		QString filename = arg2;
 		askDesktopToOpenFile(filename);
 	}
 	else if (id.startsWith(ButtonWhiteboardSessionAcceptRequest)) {
 		QString id = arg1;
 		setWhiteboardSessionStatus(id, ChatWindow::WhiteboardAccepted);
 		window_->onWhiteboardSessionAccept();
 	}
 	else if (id.startsWith(ButtonWhiteboardSessionCancel)) {
 		QString id = arg1;
 		setWhiteboardSessionStatus(id, ChatWindow::WhiteboardTerminated);
 		window_->onWhiteboardSessionCancel();
 	}
 	else if (id.startsWith(ButtonWhiteboardShowWindow)) {
 		QString id = arg1;
 		window_->onWhiteboardWindowShow();
 	}
 	else if (id.startsWith(ButtonMUCInvite)) {
 		QString roomJID = arg1;
 		QString password = arg2;
 		QString elementID = arg3;
 		QString isImpromptu = arg4;
 		QString isContinuation = arg5;
 		eventStream_->send(boost::make_shared<JoinMUCUIEvent>(Q2PSTRING(roomJID), Q2PSTRING(password), boost::optional<std::string>(), false, false, isImpromptu.contains("true"), isContinuation.contains("true")));
 		setMUCInvitationJoined(elementID);
 	}
diff --git a/Swift/QtUI/QtWebKitChatView.h b/Swift/QtUI/QtWebKitChatView.h
index 99375f7..173a05b 100644
--- a/Swift/QtUI/QtWebKitChatView.h
+++ b/Swift/QtUI/QtWebKitChatView.h
@@ -1,32 +1,32 @@
 /*
- * Copyright (c) 2010-2015 Isode Limited.
+ * Copyright (c) 2010-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #pragma once
 
 #include <boost/shared_ptr.hpp>
 
 #include <QList>
 #include <QString>
 #include <QWebElement>
 #include <QWidget>
 
 #include <Swiften/Base/Override.h>
 
 #include <Swift/Controllers/UIInterfaces/ChatWindow.h>
 
 #include <Swift/QtUI/ChatSnippet.h>
 #include <Swift/QtUI/QtChatView.h>
 
 class QWebPage;
 class QUrl;
 class QDate;
 
 namespace Swift {
 	class QtWebView;
 	class QtChatTheme;
 	class QtChatWindowJSBridge;
 	class UIEventStream;
 	class QtChatWindow;
@@ -68,60 +68,62 @@ namespace Swift {
 			
 			virtual std::string addFileTransfer(const std::string& senderName, bool senderIsSelf, const std::string& filename, const boost::uintmax_t sizeInBytes, const std::string& description) SWIFTEN_OVERRIDE;
 			virtual void setFileTransferProgress(std::string, const int percentageDone) SWIFTEN_OVERRIDE;
 			virtual void setFileTransferStatus(std::string, const ChatWindow::FileTransferState state, const std::string& msg = "") SWIFTEN_OVERRIDE;
 			virtual void addMUCInvitation(const std::string& senderName, const JID& jid, const std::string& reason, const std::string& password, bool direct, bool isImpromptu, bool isContinuation) SWIFTEN_OVERRIDE;
 			virtual std::string addWhiteboardRequest(const QString& contact, bool senderIsSelf) SWIFTEN_OVERRIDE;
 			virtual void setWhiteboardSessionStatus(const std::string& id, const ChatWindow::WhiteboardSessionState state) SWIFTEN_OVERRIDE;
 			virtual void setMessageReceiptState(const std::string& id, ChatWindow::ReceiptState state) SWIFTEN_OVERRIDE;
 
 			virtual void showEmoticons(bool show) SWIFTEN_OVERRIDE;
 			void addMessageTop(boost::shared_ptr<ChatSnippet> snippet);
 			void addMessageBottom(boost::shared_ptr<ChatSnippet> snippet);
 
 			int getSnippetPositionByDate(const QDate& date); // FIXME : This probably shouldn't have been public
 			virtual void addLastSeenLine() SWIFTEN_OVERRIDE;
 
 		private: // previously public, now private
 			void replaceLastMessage(const QString& newMessage, const ChatWindow::TimestampBehaviour timestampBehaviour);
 			void replaceLastMessage(const QString& newMessage, const QString& note);
 			void replaceMessage(const QString& newMessage, const QString& id, const QDateTime& time);
 			void replaceSystemMessage(const QString& newMessage, const QString&id, const ChatWindow::TimestampBehaviour timestampBehaviour);
 			void rememberScrolledToBottom();
 			void setAckXML(const QString& id, const QString& xml);
 			void setReceiptXML(const QString& id, const QString& xml);
 			void displayReceiptInfo(const QString& id, bool showIt);
 
 			QString getLastSentMessage();
 			void addToJSEnvironment(const QString&, QObject*);
 			void setFileTransferProgress(QString id, const int percentageDone);
 			void setFileTransferStatus(QString id, const ChatWindow::FileTransferState state, const QString& msg);
+			void setFileTransferWarning(QString id, QString warningText);
+			void removeFileTransferWarning(QString id);
 			void setWhiteboardSessionStatus(QString id, const ChatWindow::WhiteboardSessionState state);
 			void setMUCInvitationJoined(QString id);
 			void askDesktopToOpenFile(const QString& filename);
 			
 		signals:
 			void gotFocus();
 			void fontResized(int);
 			void logCleared();
 			void scrollRequested(int pos);
 			void scrollReachedTop();
 			void scrollReachedBottom();
 
 		public slots:
 			void copySelectionToClipboard();
 			void handleLinkClicked(const QUrl&);
 			void resetView();
 			void resetTopInsertPoint();
 			void increaseFontSize(int numSteps = 1);
 			void decreaseFontSize();
 			virtual void resizeFont(int fontSizeSteps) SWIFTEN_OVERRIDE;
 			virtual void scrollToBottom() SWIFTEN_OVERRIDE;
 			virtual void handleKeyPressEvent(QKeyEvent* event) SWIFTEN_OVERRIDE;
 
 		private slots:
 			void handleViewLoadFinished(bool);
 			void handleFrameSizeChanged();
 			void handleClearRequested();
 			void handleScrollRequested(int dx, int dy, const QRect& rectToScroll);
 			void handleHTMLButtonClicked(QString id, QString arg1, QString arg2, QString arg3, QString arg4, QString arg5);
 
diff --git a/Swiften/FileTransfer/ByteArrayWriteBytestream.h b/Swiften/FileTransfer/ByteArrayWriteBytestream.h
index 938b1d4..08c4d4b 100644
--- a/Swiften/FileTransfer/ByteArrayWriteBytestream.h
+++ b/Swiften/FileTransfer/ByteArrayWriteBytestream.h
@@ -1,30 +1,31 @@
 /*
- * Copyright (c) 2010-2015 Isode Limited.
+ * Copyright (c) 2010-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #pragma once
 
 #include <Swiften/Base/API.h>
 #include <Swiften/FileTransfer/WriteBytestream.h>
 
 namespace Swift {
 	class SWIFTEN_API ByteArrayWriteBytestream : public WriteBytestream {
 		public:
 			ByteArrayWriteBytestream() {
 			}
 
-			virtual void write(const std::vector<unsigned char>& bytes) {
+			virtual bool write(const std::vector<unsigned char>& bytes) {
 				data.insert(data.end(), bytes.begin(), bytes.end());
 				onWrite(bytes);
+				return true;
 			}
 
 			const std::vector<unsigned char>& getData() const {
 				return data;
 			}
 
 		private:
 			std::vector<unsigned char> data;
 	};
 }
diff --git a/Swiften/FileTransfer/FileTransferError.h b/Swiften/FileTransfer/FileTransferError.h
index 67e32f2..eff8ca9 100644
--- a/Swiften/FileTransfer/FileTransferError.h
+++ b/Swiften/FileTransfer/FileTransferError.h
@@ -1,30 +1,31 @@
 /*
- * Copyright (c) 2010-2015 Isode Limited.
+ * Copyright (c) 2010-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #pragma once
 
 #include <Swiften/Base/API.h>
 
 namespace Swift {
 	class SWIFTEN_API FileTransferError {
 		public:
 			enum Type {
 				UnknownError,
 				PeerError,
 				ReadError,
+				WriteError,
 				ClosedError
 			};
 
 			FileTransferError(Type type = UnknownError) : type(type) {}
 
 			Type getType() const {
 				return type;
 			}
 
 		private:
 			Type type;
 	};
 }
diff --git a/Swiften/FileTransfer/FileWriteBytestream.cpp b/Swiften/FileTransfer/FileWriteBytestream.cpp
index bbf3d51..c39d63a 100644
--- a/Swiften/FileTransfer/FileWriteBytestream.cpp
+++ b/Swiften/FileTransfer/FileWriteBytestream.cpp
@@ -1,47 +1,52 @@
 /*
- * Copyright (c) 2010-2015 Isode Limited.
+ * Copyright (c) 2010-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #include <Swiften/FileTransfer/FileWriteBytestream.h>
 
 #include <cassert>
 
 #include <boost/filesystem/fstream.hpp>
 #include <boost/numeric/conversion/cast.hpp>
 
 namespace Swift {
 
 FileWriteBytestream::FileWriteBytestream(const boost::filesystem::path& file) : file(file), stream(NULL) {
 }
 
 FileWriteBytestream::~FileWriteBytestream() {
 	if (stream) {
 		stream->close();
 		delete stream;
 		stream = NULL;
 	}
 }
 
-void FileWriteBytestream::write(const std::vector<unsigned char>& data) {
+bool FileWriteBytestream::write(const std::vector<unsigned char>& data) {
 	if (data.empty()) {
-		return;
+		return true;
 	}
 	if (!stream) {
 		stream = new boost::filesystem::ofstream(file, std::ios_base::out|std::ios_base::binary);
 	}
-	assert(stream->good());
-	stream->write(reinterpret_cast<const char*>(&data[0]), boost::numeric_cast<std::streamsize>(data.size()));
-	onWrite(data);
+	if (stream->good()) {
+		stream->write(reinterpret_cast<const char*>(&data[0]), boost::numeric_cast<std::streamsize>(data.size()));
+		if (stream->good()) {
+			onWrite(data);
+			return true;
+		}
+	}
+	return false;
 }
 
 void FileWriteBytestream::close() {
 	if (stream) {
 		stream->close();
 		delete stream;
 		stream = NULL;
 	}
 }
 
 }
diff --git a/Swiften/FileTransfer/FileWriteBytestream.h b/Swiften/FileTransfer/FileWriteBytestream.h
index b2d3347..02e1b46 100644
--- a/Swiften/FileTransfer/FileWriteBytestream.h
+++ b/Swiften/FileTransfer/FileWriteBytestream.h
@@ -1,28 +1,28 @@
 /*
  * Copyright (c) 2010-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #pragma once
 
 #include <boost/filesystem/fstream.hpp>
 #include <boost/filesystem/path.hpp>
 
 #include <Swiften/Base/API.h>
 #include <Swiften/FileTransfer/WriteBytestream.h>
 
 namespace Swift {
 	class SWIFTEN_API FileWriteBytestream : public WriteBytestream {
 		public:
 			FileWriteBytestream(const boost::filesystem::path& file);
 			virtual ~FileWriteBytestream();
 
-			virtual void write(const std::vector<unsigned char>&);
+			virtual bool write(const std::vector<unsigned char>&);
 			void close();
 
 		private:
 			boost::filesystem::path file;
 			boost::filesystem::ofstream* stream;
 	};
 }
diff --git a/Swiften/FileTransfer/SOCKS5BytestreamClientSession.cpp b/Swiften/FileTransfer/SOCKS5BytestreamClientSession.cpp
index a38501b..4fc0246 100644
--- a/Swiften/FileTransfer/SOCKS5BytestreamClientSession.cpp
+++ b/Swiften/FileTransfer/SOCKS5BytestreamClientSession.cpp
@@ -1,38 +1,38 @@
 /*
  * Copyright (c) 2011 Tobias Markmann
  * Licensed under the simplified BSD license.
  * See Documentation/Licenses/BSD-simplified.txt for more information.
  */
 
 /*
- * Copyright (c) 2013-2015 Isode Limited.
+ * Copyright (c) 2013-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #include "SOCKS5BytestreamClientSession.h"
 
 #include <boost/bind.hpp>
 #include <boost/numeric/conversion/cast.hpp>
 
 #include <Swiften/Base/Algorithm.h>
 #include <Swiften/Base/ByteArray.h>
 #include <Swiften/Base/Concat.h>
 #include <Swiften/Base/Log.h>
 #include <Swiften/Base/SafeByteArray.h>
 #include <Swiften/FileTransfer/BytestreamException.h>
 #include <Swiften/Network/TimerFactory.h>
 #include <Swiften/StringCodecs/Hexify.h>
 
 namespace Swift {
 
 SOCKS5BytestreamClientSession::SOCKS5BytestreamClientSession(
 		boost::shared_ptr<Connection> connection, 
 		const HostAddressPort& addressPort, 
 		const std::string& destination, 
 		TimerFactory* timerFactory) :
 			connection(connection), 
 			addressPort(addressPort), 
 			destination(destination), 
 			state(Initial), 
 			chunkSize(131072) {
@@ -139,61 +139,60 @@ void SOCKS5BytestreamClientSession::process() {
 			SWIFT_LOG(debug) << "Data: " << Hexify::hexify(unprocessedData) << std::endl;
 			unprocessedData.clear();
 			//assert(false);
 	}
 }
 
 void SOCKS5BytestreamClientSession::hello() {
 	// Version 5, 1 auth method, No authentication
 	const SafeByteArray hello = createSafeByteArray("\x05\x01\x00", 3);
 	connection->write(hello);
 	state = Hello;
 }
 
 void SOCKS5BytestreamClientSession::authenticate() {
 	SWIFT_LOG(debug) << std::endl;
 	SafeByteArray header = createSafeByteArray("\x05\x01\x00\x03", 4);
 	SafeByteArray message = header;
 	append(message, createSafeByteArray(boost::numeric_cast<char>(destination.size())));
 	authenticateAddress = createByteArray(destination);
 	append(message, authenticateAddress);
 	append(message, createSafeByteArray("\x00\x00", 2)); // 2 byte for port
 	connection->write(message);
 	state = Authenticating;
 }
 
 void SOCKS5BytestreamClientSession::startReceiving(boost::shared_ptr<WriteBytestream> writeStream) {
 	if (state == Ready) {
 		state = Reading;
 		writeBytestream = writeStream;
 		writeBytestream->write(unprocessedData);
-		//onBytesReceived(unprocessedData.size());
 		unprocessedData.clear();
 	} else {
 		SWIFT_LOG(debug) << "Session isn't ready for transfer yet!" << std::endl;
 	}
 }
 
 void SOCKS5BytestreamClientSession::startSending(boost::shared_ptr<ReadBytestream> readStream) {
 	if (state == Ready) {
 		state = Writing;
 		readBytestream = readStream;
 		dataWrittenConnection = connection->onDataWritten.connect(
 				boost::bind(&SOCKS5BytestreamClientSession::sendData, this));
 		sendData();
 	} else {
 		SWIFT_LOG(debug) << "Session isn't ready for transfer yet!" << std::endl;
 	}
 }
 
 HostAddressPort SOCKS5BytestreamClientSession::getAddressPort() const {
 	return addressPort;
 }
 
 void SOCKS5BytestreamClientSession::sendData() {
 	if (!readBytestream->isFinished()) {
 		try {
 			boost::shared_ptr<ByteArray> dataToSend = readBytestream->read(boost::numeric_cast<size_t>(chunkSize));
 			connection->write(createSafeByteArray(*dataToSend));
 			onBytesSent(dataToSend->size());
 		}
 		catch (const BytestreamException&) {
diff --git a/Swiften/FileTransfer/SOCKS5BytestreamServerSession.cpp b/Swiften/FileTransfer/SOCKS5BytestreamServerSession.cpp
index 7838dd1..0e1eb6b 100644
--- a/Swiften/FileTransfer/SOCKS5BytestreamServerSession.cpp
+++ b/Swiften/FileTransfer/SOCKS5BytestreamServerSession.cpp
@@ -1,199 +1,195 @@
 /*
- * Copyright (c) 2010-2013 Isode Limited.
+ * Copyright (c) 2010-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #include <Swiften/FileTransfer/SOCKS5BytestreamServerSession.h>
 
 #include <boost/bind.hpp>
 #include <boost/numeric/conversion/cast.hpp>
-#include <iostream>
 
-#include <Swiften/Base/ByteArray.h>
-#include <Swiften/Base/SafeByteArray.h>
 #include <Swiften/Base/Algorithm.h>
+#include <Swiften/Base/ByteArray.h>
 #include <Swiften/Base/Concat.h>
 #include <Swiften/Base/Log.h>
-#include <Swiften/Network/HostAddressPort.h>
-#include <Swiften/FileTransfer/SOCKS5BytestreamRegistry.h>
+#include <Swiften/Base/SafeByteArray.h>
 #include <Swiften/FileTransfer/BytestreamException.h>
+#include <Swiften/FileTransfer/SOCKS5BytestreamRegistry.h>
+#include <Swiften/Network/HostAddressPort.h>
 
 namespace Swift {
 
 SOCKS5BytestreamServerSession::SOCKS5BytestreamServerSession(
 		boost::shared_ptr<Connection> connection, 
 		SOCKS5BytestreamRegistry* bytestreams) : 
 			connection(connection), 
 			bytestreams(bytestreams), 
 			state(Initial), 
 			chunkSize(131072), 
 			waitingForData(false) {
 	disconnectedConnection = connection->onDisconnected.connect(boost::bind(&SOCKS5BytestreamServerSession::handleDisconnected, this, _1));
 }
 
 SOCKS5BytestreamServerSession::~SOCKS5BytestreamServerSession() {
 	if (state != Finished && state != Initial) {
-		std::cerr << "Warning: SOCKS5BytestreamServerSession unfinished" << std::endl;
-		finish(false);
+		SWIFT_LOG(warning) << "SOCKS5BytestreamServerSession unfinished" << std::endl;
+		finish();
 	}
 }
 
 void SOCKS5BytestreamServerSession::start() {
 	SWIFT_LOG(debug) << std::endl;
 	dataReadConnection = connection->onDataRead.connect(
 			boost::bind(&SOCKS5BytestreamServerSession::handleDataRead, this, _1));
 	state = WaitingForAuthentication;
 }
 
 void SOCKS5BytestreamServerSession::stop() {
-	finish(false);
+	finish();
 }
 
 void SOCKS5BytestreamServerSession::startSending(boost::shared_ptr<ReadBytestream> stream) {
 	if (state != ReadyForTransfer) { SWIFT_LOG(debug) << "Not ready for transfer!" << std::endl; return; }
 
 	readBytestream = stream;
 	state = WritingData;
 	dataAvailableConnection = readBytestream->onDataAvailable.connect(
 			boost::bind(&SOCKS5BytestreamServerSession::handleDataAvailable, this));
 	dataWrittenConnection = connection->onDataWritten.connect(
 			boost::bind(&SOCKS5BytestreamServerSession::sendData, this));
 	sendData();
 }
 
 void SOCKS5BytestreamServerSession::startReceiving(boost::shared_ptr<WriteBytestream> stream) {
 	if (state != ReadyForTransfer) { SWIFT_LOG(debug) << "Not ready for transfer!" << std::endl; return; }
 
 	writeBytestream = stream;
 	state = ReadingData;
 	writeBytestream->write(unprocessedData);
 	// onBytesReceived(unprocessedData.size());
 	unprocessedData.clear();
 }
 
 HostAddressPort SOCKS5BytestreamServerSession::getAddressPort() const {
 	return connection->getLocalAddress();
 }
 
 void SOCKS5BytestreamServerSession::handleDataRead(boost::shared_ptr<SafeByteArray> data) {
 	if (state != ReadingData) {
 		append(unprocessedData, *data);
 		process();
 	} else {
-		writeBytestream->write(createByteArray(vecptr(*data), data->size()));
-		// onBytesReceived(data->size());
+		if (!writeBytestream->write(createByteArray(vecptr(*data), data->size()))) {
+			finish(boost::optional<FileTransferError>(FileTransferError::WriteError));
+		}
 	}
 }
 
 void SOCKS5BytestreamServerSession::handleDataAvailable() {
 	if (waitingForData) {
 		sendData();
 	}
 }
 
 void SOCKS5BytestreamServerSession::handleDisconnected(const boost::optional<Connection::Error>& error) {
 	SWIFT_LOG(debug) << (error ? (error == Connection::ReadError ? "Read Error" : "Write Error") : "No Error") << std::endl;
-	finish(error ? true : false);
+	finish(error ? boost::optional<FileTransferError>(FileTransferError::PeerError) : boost::optional<FileTransferError>());
 }
 
 void SOCKS5BytestreamServerSession::process() {
 	if (state == WaitingForAuthentication) {
 		if (unprocessedData.size() >= 2) {
 			size_t authCount = unprocessedData[1];
 			size_t i = 2;
 			while (i < 2 + authCount && i < unprocessedData.size()) {
 				// Skip authentication mechanism
 				++i;
 			}
 			if (i == 2 + authCount) {
 				// Authentication message is complete
 				if (i != unprocessedData.size()) {
 					SWIFT_LOG(debug) << "Junk after authentication mechanism" << std::endl;
 				}
 				unprocessedData.clear();
 				connection->write(createSafeByteArray("\x05\x00", 2));
 				state = WaitingForRequest;
 			}
 		}
 	}
 	else if (state == WaitingForRequest) {
 		if (unprocessedData.size() >= 5) {
 			ByteArray requestID;
 			size_t i = 5;
 			size_t hostnameSize = unprocessedData[4];
 			while (i < 5 + hostnameSize && i < unprocessedData.size()) {
 				requestID.push_back(unprocessedData[i]);
 				++i;
 			}
 			// Skip the port: 2 byte large, one already skipped. Add one for comparison with size
 			i += 2;
 			if (i <= unprocessedData.size()) {
 				if (i != unprocessedData.size()) {
 					SWIFT_LOG(debug) << "Junk after authentication mechanism" << std::endl;
 				}
 				unprocessedData.clear();
 				streamID = byteArrayToString(requestID);
 				bool hasBytestream = bytestreams->hasBytestream(streamID);
 				SafeByteArray result = createSafeByteArray("\x05", 1);
 				result.push_back(hasBytestream ? 0x0 : 0x4);
 				append(result, createByteArray("\x00\x03", 2));
 				result.push_back(boost::numeric_cast<unsigned char>(requestID.size()));
 				append(result, concat(requestID, createByteArray("\x00\x00", 2)));
 				if (!hasBytestream) {
 					SWIFT_LOG(debug) << "Readstream or Wrtiestream with ID " << streamID << " not found!" << std::endl;
 					connection->write(result);
-					finish(true);
+					finish(boost::optional<FileTransferError>(FileTransferError::PeerError));
 				}
 				else {
 					SWIFT_LOG(debug) << "Found stream. Sent OK." << std::endl;
 					connection->write(result);
 					state = ReadyForTransfer;
 				}
 			}
 		}
 	}
 }
 
 void SOCKS5BytestreamServerSession::sendData() {
 	if (!readBytestream->isFinished()) {
 		try {
 			SafeByteArray dataToSend = createSafeByteArray(*readBytestream->read(boost::numeric_cast<size_t>(chunkSize)));
 			if (!dataToSend.empty()) {
 				connection->write(dataToSend);
 				onBytesSent(dataToSend.size());
 				waitingForData = false;
 			}
 			else {
 				waitingForData = true;
 			}
 		}
 		catch (const BytestreamException&) {
-			finish(true);
+			finish(boost::optional<FileTransferError>(FileTransferError::PeerError));
 		}
 	}
 	else {
-		finish(false);
+		finish();
 	}
 }
 
-void SOCKS5BytestreamServerSession::finish(bool error) {
-	SWIFT_LOG(debug) << error << " " << state << std::endl;
+void SOCKS5BytestreamServerSession::finish(const boost::optional<FileTransferError>& error) {
+	SWIFT_LOG(debug) << "state: " << state << std::endl;
 	if (state == Finished) {
 		return;
 	}
 
 	disconnectedConnection.disconnect();
 	dataReadConnection.disconnect();
 	dataWrittenConnection.disconnect();
 	dataAvailableConnection.disconnect();
 	readBytestream.reset();
 	state = Finished;
-	if (error) {
-		onFinished(boost::optional<FileTransferError>(FileTransferError::PeerError));
-	} else {
-		onFinished(boost::optional<FileTransferError>());
-	}
+	onFinished(error);
 }
 
 }
diff --git a/Swiften/FileTransfer/SOCKS5BytestreamServerSession.h b/Swiften/FileTransfer/SOCKS5BytestreamServerSession.h
index d8eea34..ed5272f 100644
--- a/Swiften/FileTransfer/SOCKS5BytestreamServerSession.h
+++ b/Swiften/FileTransfer/SOCKS5BytestreamServerSession.h
@@ -1,84 +1,83 @@
 /*
- * Copyright (c) 2010 Isode Limited.
+ * Copyright (c) 2010-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #pragma once
 
 #include <boost/shared_ptr.hpp>
 
 #include <Swiften/Base/API.h>
 #include <Swiften/Base/boost_bsignals.h>
-#include <Swiften/Network/Connection.h>
+#include <Swiften/FileTransfer/FileTransferError.h>
 #include <Swiften/FileTransfer/ReadBytestream.h>
 #include <Swiften/FileTransfer/WriteBytestream.h>
-#include <Swiften/FileTransfer/FileTransferError.h>
+#include <Swiften/Network/Connection.h>
 
 namespace Swift {
 	class SOCKS5BytestreamRegistry;
 
 	class SWIFTEN_API SOCKS5BytestreamServerSession {
 		public:
 		typedef boost::shared_ptr<SOCKS5BytestreamServerSession> ref;
 
 		public:
 			enum State {
 				Initial,
 				WaitingForAuthentication,
 				WaitingForRequest,
 				ReadyForTransfer,
 				ReadingData,
 				WritingData,
 				Finished
 			};
 
 			SOCKS5BytestreamServerSession(boost::shared_ptr<Connection> connection, SOCKS5BytestreamRegistry* registry);
 			~SOCKS5BytestreamServerSession();
 
 			void setChunkSize(int chunkSize) {
 				this->chunkSize = chunkSize;
 			}
 
 			void start();
 			void stop();
 
 			void startSending(boost::shared_ptr<ReadBytestream>);
 			void startReceiving(boost::shared_ptr<WriteBytestream>);
 
 			HostAddressPort getAddressPort() const;
 
 			boost::signal<void (boost::optional<FileTransferError>)> onFinished;
 			boost::signal<void (unsigned long long)> onBytesSent;
-			// boost::signal<void (unsigned long long)> onBytesReceived;
 
 			const std::string& getStreamID() const {
 				return streamID;
 			}
 
 		private:
-			void finish(bool error);
+			void finish(const boost::optional<FileTransferError>& error = boost::optional<FileTransferError>());
 			void process();
 			void handleDataRead(boost::shared_ptr<SafeByteArray>);
 			void handleDisconnected(const boost::optional<Connection::Error>&);
 			void handleDataAvailable();
 			void sendData();
 
 		private:
 			boost::shared_ptr<Connection> connection;
 			SOCKS5BytestreamRegistry* bytestreams;
 			ByteArray unprocessedData;
 			State state;
 			int chunkSize;
 			std::string streamID;
 			boost::shared_ptr<ReadBytestream> readBytestream;
 			boost::shared_ptr<WriteBytestream> writeBytestream;
 			bool waitingForData;
 
 			boost::bsignals::connection disconnectedConnection;
 			boost::bsignals::connection dataReadConnection;
 			boost::bsignals::connection dataWrittenConnection;
 			boost::bsignals::connection dataAvailableConnection;
 
 	};
 }
diff --git a/Swiften/FileTransfer/WriteBytestream.h b/Swiften/FileTransfer/WriteBytestream.h
index 3299620..76237c4 100644
--- a/Swiften/FileTransfer/WriteBytestream.h
+++ b/Swiften/FileTransfer/WriteBytestream.h
@@ -1,26 +1,32 @@
 /*
- * Copyright (c) 2010 Isode Limited.
+ * Copyright (c) 2010-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #pragma once
 
-#include <boost/shared_ptr.hpp>
 #include <vector>
 
+#include <boost/shared_ptr.hpp>
+
 #include <Swiften/Base/API.h>
 #include <Swiften/Base/boost_bsignals.h>
 
 namespace Swift {
 	class SWIFTEN_API WriteBytestream {
 		public:
 			typedef boost::shared_ptr<WriteBytestream> ref;
 
 			virtual ~WriteBytestream();
 
-			virtual void write(const std::vector<unsigned char>&) = 0;
+			/**
+			 * Write data from vector argument to bytestream.
+			 * 
+			 * On success true is returned and \ref onWrite is called. On failure false is returned.
+			 */
+			virtual bool write(const std::vector<unsigned char>&) = 0;
 
 			boost::signal<void (const std::vector<unsigned char>&)> onWrite;
 	};
 }
diff --git a/Swiften/QA/SConscript b/Swiften/QA/SConscript
index 2135ef4..2c588e5 100644
--- a/Swiften/QA/SConscript
+++ b/Swiften/QA/SConscript
@@ -1,13 +1,13 @@
 Import("swiften_env")
 
 SConscript(dirs = [
 		"NetworkTest",
 #		"ReconnectTest",
 		"ClientTest",
 #		"DNSSDTest",
-#		"StorageTest",
+		"StorageTest",
 		"TLSTest",
 		"ScriptedTests",
 		"ProxyProviderTest",
 		"FileTransferTest",
 	])
diff --git a/Swiften/QA/StorageTest/FileReadBytestreamTest.cpp b/Swiften/QA/StorageTest/FileReadBytestreamTest.cpp
index c36420a..d70d9c9 100644
--- a/Swiften/QA/StorageTest/FileReadBytestreamTest.cpp
+++ b/Swiften/QA/StorageTest/FileReadBytestreamTest.cpp
@@ -1,74 +1,75 @@
 /*
- * Copyright (c) 2010 Isode Limited.
+ * Copyright (c) 2010-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #include <cppunit/extensions/HelperMacros.h>
 #include <cppunit/extensions/TestFactoryRegistry.h>
 
 #include <Swiften/Base/ByteArray.h>
 #include <Swiften/FileTransfer/FileReadBytestream.h>
-#include "SwifTools/Application/PlatformApplicationPathProvider.h"
+
+#include <SwifTools/Application/PlatformApplicationPathProvider.h>
 
 using namespace Swift;
 
 class FileReadBytestreamTest : public CppUnit::TestFixture {
 		CPPUNIT_TEST_SUITE(FileReadBytestreamTest);
 		CPPUNIT_TEST(testRead);
 		CPPUNIT_TEST(testRead_Twice);
 		CPPUNIT_TEST(testIsFinished_NotFinished);
 		CPPUNIT_TEST(testIsFinished_IsFinished);
 		CPPUNIT_TEST_SUITE_END();
 
 	public:
 		void setUp() {
 			pathProvider = new PlatformApplicationPathProvider("FileReadBytestreamTest");
 		}
 
 		void tearDown() {
 			delete pathProvider;
 		}
 
 		void testRead() {
 			boost::shared_ptr<FileReadBytestream> testling(createTestling());
 
-			std::vector<unsigned char> result = testling->read(10);
+			boost::shared_ptr< std::vector<unsigned char> > result = testling->read(10);
 
-			CPPUNIT_ASSERT(ByteArray::create("/*\n * Copy") == result);
+			CPPUNIT_ASSERT(createByteArray("/*\n * Copy") == *result.get());
 		}
 
 		void testRead_Twice() {
 			boost::shared_ptr<FileReadBytestream> testling(createTestling());
 
 			testling->read(10);
-			ByteArray result(testling->read(10));
+			boost::shared_ptr< std::vector<unsigned char> > result = testling->read(10);
 
-			CPPUNIT_ASSERT_EQUAL(std::string("right (c) "), result.toString());
+			CPPUNIT_ASSERT_EQUAL(std::string("right (c) "), byteArrayToString(*result));
 		}
 
 		void testIsFinished_NotFinished() {
 			boost::shared_ptr<FileReadBytestream> testling(createTestling());
 
 			testling->read(10);
 
 			CPPUNIT_ASSERT(!testling->isFinished());
 		}
 
 		void testIsFinished_IsFinished() {
 			boost::shared_ptr<FileReadBytestream> testling(createTestling());
 
 			testling->read(4096);
 
 			CPPUNIT_ASSERT(testling->isFinished());
 		}
 
 	private:
 		FileReadBytestream* createTestling() {
 			return new FileReadBytestream(pathProvider->getExecutableDir() / "FileReadBytestreamTest.cpp");
 		}
 
 		PlatformApplicationPathProvider* pathProvider;
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(FileReadBytestreamTest);
diff --git a/Swiften/QA/StorageTest/FileWriteBytestreamTest.cpp b/Swiften/QA/StorageTest/FileWriteBytestreamTest.cpp
new file mode 100644
index 0000000..3686cf9
--- /dev/null
+++ b/Swiften/QA/StorageTest/FileWriteBytestreamTest.cpp
@@ -0,0 +1,58 @@
+/*
+ * Copyright (c) 2016 Isode Limited.
+ * All rights reserved.
+ * See the COPYING file for more information.
+ */
+
+#include <boost/bind.hpp>
+#include <boost/filesystem.hpp>
+
+#include <cppunit/extensions/HelperMacros.h>
+#include <cppunit/extensions/TestFactoryRegistry.h>
+
+#include <Swiften/Base/ByteArray.h>
+#include <Swiften/Base/boost_bsignals.h>
+#include <Swiften/FileTransfer/FileWriteBytestream.h>
+
+using namespace Swift;
+
+class FileWriteBytestreamTest : public CppUnit::TestFixture {
+		CPPUNIT_TEST_SUITE(FileWriteBytestreamTest);
+		CPPUNIT_TEST(testSuccessfulWrite);
+		CPPUNIT_TEST(testFailingWrite);
+		CPPUNIT_TEST_SUITE_END();
+
+	public:
+		void setUp() {
+			onWriteWasCalled = false;
+		}
+
+		void testSuccessfulWrite() {
+			boost::filesystem::path filename = boost::filesystem::unique_path("write_file_bytestream_test_%%%%%%%%%%%%%%%%.bin");
+			boost::shared_ptr<WriteBytestream> writeBytestream = boost::make_shared<FileWriteBytestream>(filename.string());
+			writeBytestream->onWrite.connect(boost::bind(&FileWriteBytestreamTest::handleOnWrite, this, _1));
+
+			CPPUNIT_ASSERT_EQUAL(true, writeBytestream->write(createByteArray("Some data.")));
+			CPPUNIT_ASSERT_EQUAL(true, onWriteWasCalled);
+
+			boost::filesystem::remove(filename);
+		}
+
+		void testFailingWrite() {
+			boost::shared_ptr<WriteBytestream> writeBytestream = boost::make_shared<FileWriteBytestream>("");
+			writeBytestream->onWrite.connect(boost::bind(&FileWriteBytestreamTest::handleOnWrite, this, _1));
+
+			CPPUNIT_ASSERT_EQUAL(false, writeBytestream->write(createByteArray("Some data.")));
+			CPPUNIT_ASSERT_EQUAL(false, onWriteWasCalled);
+		}
+
+
+		void handleOnWrite(const std::vector<unsigned char>& /*data*/) {
+			onWriteWasCalled = true;
+		}
+
+	private:
+		bool onWriteWasCalled;
+};
+
+CPPUNIT_TEST_SUITE_REGISTRATION(FileWriteBytestreamTest);
diff --git a/Swiften/QA/StorageTest/SConscript b/Swiften/QA/StorageTest/SConscript
index 6d65b30..fff2a2b 100644
--- a/Swiften/QA/StorageTest/SConscript
+++ b/Swiften/QA/StorageTest/SConscript
@@ -1,21 +1,22 @@
 import os
 
 Import("env")
 
 if env["TEST"] :
 	myenv = env.Clone()
 	myenv.MergeFlags(myenv["CHECKER_FLAGS"])
 	myenv.MergeFlags(myenv["SWIFTOOLS_FLAGS"])
 	myenv.MergeFlags(myenv["SWIFTEN_FLAGS"])
 	myenv.MergeFlags(myenv["CPPUNIT_FLAGS"])
 	myenv.MergeFlags(myenv["BOOST_FLAGS"])
 	myenv.MergeFlags(myenv["LIBIDN_FLAGS"])
 	myenv.MergeFlags(myenv.get("EXPAT_FLAGS", {}))
 	myenv.MergeFlags(myenv.get("LIBXML_FLAGS", {}))
 	myenv.MergeFlags(myenv["PLATFORM_FLAGS"])
 
 	tester = myenv.Program("StorageTest", [
-			"VCardFileStorageTest.cpp",
+			#"VCardFileStorageTest.cpp",
 			"FileReadBytestreamTest.cpp",
+			"FileWriteBytestreamTest.cpp",
 		])
 	myenv.Test(tester, "system", is_checker = True)
author	Tobias Markmann <tm@ayena.de>	2016-02-10 19:25:21 (GMT)
committer	Kevin Smith <kevin.smith@isode.com>	2016-02-15 12:31:10 (GMT)
commit	ca226e7bb019308db4bfc818d7e04422d9d28106 (patch)
tree	9f7ef065f20db3e04eb08560674eed16d10e95e5
parent	431eb62386101dc8fc1e7d346c49bd0d81fda70e (diff)
download	swift-ca226e7bb019308db4bfc818d7e04422d9d28106.zip swift-ca226e7bb019308db4bfc818d7e04422d9d28106.tar.bz2