Fix file transfer under SSL

Introduces a big fat buffer :(
Actually test the trasfers :)
Takes QSslSocket causistic into account, for some reason QNAM refuses to
mark as finished when the QSslSocket (through QIODevice) closes.
It would be good to look into dropping the QBuffer, doing so with the test
in place will help.

core/backends/lan/downloadjob.cpp

@@ -37,9 +37,13 @@ DownloadJob::DownloadJob(const QHostAddress &address, const QVariantMap &transfe
     : KJob()
     , mAddress(address)
     , mPort(transferInfo["port"].toInt())
-    , mSocket(new QSslSocket(this))
+    , mSocket(new QSslSocket)
+    , mBuffer(new QBuffer)
 {
     LanLinkProvider::configureSslSocket(mSocket.data(), transferInfo.value("deviceId").toString(), true);
+
+    connect(mSocket.data(), SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(socketFailed(QAbstractSocket::SocketError)));
+//     connect(mSocket.data(), &QAbstractSocket::stateChanged, [](QAbstractSocket::SocketState state){ qDebug() << "statechange" << state; });
 }
 
 DownloadJob::~DownloadJob()
@@ -50,23 +54,28 @@ DownloadJob::~DownloadJob()
 void DownloadJob::start()
 {
     //TODO: Timeout?
-    connect(mSocket.data(), &QAbstractSocket::disconnected, this, &DownloadJob::emitResult);
-    connect(mSocket.data(), SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(socketFailed(QAbstractSocket::SocketError)));
-//     connect(mSocket.data(), &QAbstractSocket::stateChanged, [](QAbstractSocket::SocketState state){ qDebug() << "statechange" << state; });
-
     // Cannot use read only, might be due to ssl handshake, getting QIODevice::ReadOnly error and no connection
     mSocket->connectToHostEncrypted(mAddress.toString(), mPort, QIODevice::ReadWrite);
+
+    bool b = mBuffer->open(QBuffer::ReadWrite);
+    Q_ASSERT(b);
 }
 
 void DownloadJob::socketFailed(QAbstractSocket::SocketError error)
 {
-    qWarning(KDECONNECT_CORE) << "error..." << mSocket->errorString();
-    setError(error + 1);
-    setErrorText(mSocket->errorString());
+    if (error != QAbstractSocket::RemoteHostClosedError) { //remote host closes when finishes
+        qWarning(KDECONNECT_CORE) << "error..." << mSocket->errorString();
+        setError(error + 1);
+        setErrorText(mSocket->errorString());
+    } else {
+        auto ba = mSocket->readAll();
+        mBuffer->write(ba);
+        mBuffer->seek(0);
+    }
     emitResult();
 }
 
 QSharedPointer<QIODevice> DownloadJob::getPayload()
 {
-    return mSocket.staticCast<QIODevice>();
+    return mBuffer.staticCast<QIODevice>();
 }

core/backends/lan/downloadjob.h

@@ -29,6 +29,7 @@
 #include <QTcpSocket>
 #include <QSharedPointer>
 #include <QSslSocket>
+#include <QBuffer>
 #include <kdeconnectcore_export.h>
 
 
@@ -46,6 +47,7 @@ private:
     QHostAddress mAddress;
     qint16 mPort;
     QSharedPointer<QSslSocket> mSocket;
+    QSharedPointer<QBuffer> mBuffer;
 
 private Q_SLOTS:
     void socketFailed(QAbstractSocket::SocketError error);

core/backends/lan/uploadjob.cpp

@@ -27,7 +27,8 @@
 
 #include "core_debug.h"
 
-UploadJob::UploadJob(const QSharedPointer<QIODevice>& source, const QString& deviceId): KJob()
+UploadJob::UploadJob(const QSharedPointer<QIODevice>& source, const QString& deviceId)
+    : KJob()
 {
 //  TODO: initialize in constructor
     mInput = source;
@@ -38,7 +39,7 @@ UploadJob::UploadJob(const QSharedPointer<QIODevice>& source, const QString& dev
     // We will use this info if link is on ssl, to send encrypted payload
     this->mDeviceId = deviceId;
 
-    connect(mInput.data(), SIGNAL(readyRead()), this, SLOT(readyRead()));
+    connect(mInput.data(), SIGNAL(readyRead()), this, SLOT(startUploading()));
     connect(mInput.data(), SIGNAL(aboutToClose()), this, SLOT(aboutToClose()));
 }
 
@@ -50,6 +51,7 @@ void UploadJob::start()
         if (mPort > 1764) { //No ports available?
             qCWarning(KDECONNECT_CORE) << "Error opening a port in range 1739-1764 for file transfer";
             mPort = 0;
+            emitResult();
             return;
         }
     }
@@ -68,17 +70,19 @@ void UploadJob::newConnection()
     disconnect(mServer, SIGNAL(newConnection()), this, SLOT(newConnection()));
 
     mSocket = server->nextPendingConnection();
-    connect(mSocket, SIGNAL(disconnected()), mSocket, SLOT(deleteLater()));
+    mSocket->setParent(this);
+    connect(mSocket, &QSslSocket::disconnected, this, &UploadJob::cleanup);
+    connect(mSocket, SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(socketFailed(QAbstractSocket::SocketError)));
+    connect(mSocket, SIGNAL(sslErrors(QList<QSslError>)), this, SLOT(sslErrors(QList<QSslError>)));
+    connect(mSocket, &QSslSocket::encrypted, this, &UploadJob::startUploading);
+//     connect(mSocket, &QAbstractSocket::stateChanged, [](QAbstractSocket::SocketState state){ qDebug() << "statechange" << state; });
 
     LanLinkProvider::configureSslSocket(mSocket, mDeviceId, true);
 
     mSocket->startServerEncryption();
-    mSocket->waitForEncrypted();
-
-    readyRead();
 }
 
-void UploadJob::readyRead()
+void UploadJob::startUploading()
 {
     while ( mInput->bytesAvailable() > 0 )
     {
@@ -93,14 +97,19 @@ void UploadJob::readyRead()
             while ( mSocket->flush() );
         }
     }
-
     mInput->close();
 }
 
 void UploadJob::aboutToClose()
 {
+//     qDebug() << "closing...";
     mSocket->disconnectFromHost();
+}
+
+void UploadJob::cleanup()
+{
     mSocket->close();
+//     qDebug() << "closed!";
     emitResult();
 }
 
@@ -110,3 +119,18 @@ QVariantMap UploadJob::transferInfo()
     return {{"port", mPort}};
 }
 
+void UploadJob::socketFailed(QAbstractSocket::SocketError error)
+{
+    qWarning() << "error uploading" << error;
+    setError(2);
+    emitResult();
+    mSocket->close();
+}
+
+void UploadJob::sslErrors(const QList<QSslError>& errors)
+{
+    qWarning() << "ssl errors" << errors;
+    setError(1);
+    emitResult();
+    mSocket->close();
+}

core/backends/lan/uploadjob.h

@@ -29,7 +29,7 @@
 #include <QSslSocket>
 #include "server.h"
 
-class UploadJob
+class KDECONNECTCORE_EXPORT UploadJob
     : public KJob
 {
     Q_OBJECT
@@ -48,9 +48,13 @@ private:
     QString mDeviceId;
 
 private Q_SLOTS:
-    void readyRead();
+    void startUploading();
     void newConnection();
     void aboutToClose();
+    void cleanup();
+
+    void socketFailed(QAbstractSocket::SocketError);
+    void sslErrors(const QList<QSslError> &errors);
 };
 
 #endif // UPLOADJOB_H

core/filetransferjob.cpp

@@ -72,7 +72,7 @@ void FileTransferJob::doStart()
         return;
     }
 
-    startTransfer();
+    connect(mOrigin.data(), &QIODevice::readyRead, this, &FileTransferJob::startTransfer);
 }
 
 void FileTransferJob::startTransfer()

tests/sendfiletest.cpp

@@ -19,6 +19,10 @@
  */
 
 #include <QSocketNotifier>
+#include <backends/lan/downloadjob.h>
+#include <kdeconnectconfig.h>
+#include <backends/lan/uploadjob.h>
+#include <core/filetransferjob.h>
 #include <QApplication>
 #include <QNetworkAccessManager>
 #include <QTest>
@@ -83,6 +87,58 @@ class TestSendFile : public QObject
             QCOMPARE(file.readAll(), content);
         }
 
+        void testSslJobs()
+        {
+            const QString aFile = QFINDTESTDATA("sendfiletest.cpp");
+            const QString destFile = QDir::tempPath() + "/kdeconnect-test-sentfile";
+            QFile(destFile).remove();
+
+            const QString deviceId = KdeConnectConfig::instance()->deviceId()
+                        , deviceName = "testdevice"
+                        , deviceType = KdeConnectConfig::instance()->deviceType();
+
+            KdeConnectConfig* kcc = KdeConnectConfig::instance();
+            kcc->addTrustedDevice(deviceId, deviceName, deviceType);
+            kcc->setDeviceProperty(deviceId, QString("certificate"), QString::fromLatin1(kcc->certificate().toPem())); // Using same certificate from kcc, instead of generating
+
+            QSharedPointer<QFile> f(new QFile(aFile));
+            UploadJob* uj = new UploadJob(f, deviceId);
+            QSignalSpy spyUpload(uj, &KJob::result);
+            uj->start();
+
+            auto info = uj->transferInfo();
+            info.insert("deviceId", deviceId);
+            info.insert("size", aFile.size());
+
+            DownloadJob* dj = new DownloadJob(QHostAddress::LocalHost, info);
+
+            QVERIFY(dj->getPayload()->open(QIODevice::ReadOnly));
+
+            FileTransferJob* ft = new FileTransferJob(dj->getPayload(), uj->transferInfo()["size"].toInt(), QUrl::fromLocalFile(destFile));
+
+            QSignalSpy spyDownload(dj, &KJob::result);
+            QSignalSpy spyTransfer(ft, &KJob::result);
+
+            ft->start();
+            dj->start();
+
+            QVERIFY(spyTransfer.count() || spyTransfer.wait(10000000000000000000));
+
+            if (ft->error()) qWarning() << "fterror" << ft->errorString();
+
+            QCOMPARE(ft->error(), 0);
+            QCOMPARE(spyDownload.count(), 1);
+            QCOMPARE(spyUpload.count(), 1);
+
+            QFile resultFile(destFile), originFile(aFile);
+            QVERIFY(resultFile.open(QIODevice::ReadOnly));
+            QVERIFY(originFile.open(QIODevice::ReadOnly));
+
+            const QByteArray resultContents = resultFile.readAll(), originContents = originFile.readAll();
+            QCOMPARE(resultContents.size(), originContents.size());
+            QCOMPARE(resultFile.readAll(), originFile.readAll());
+        }
+
     private:
         TestDaemon* mDaemon;
 };