Fixed issues pointed out on CR

core/backends/lan/downloadjob.cpp

@@ -44,12 +44,10 @@ void DownloadJob::start()
 {
     //kDebug(kdeconnect_kded()) << "DownloadJob Start";
     if (useSsl) {
-        qDebug() << "Connecting to host encrypted";
         // Cannot use read only, might be due to ssl handshake, getting QIODevice::ReadOnly error and no connection
         mSocket->connectToHostEncrypted(mAddress.toString(), mPort, QIODevice::ReadWrite);
         mSocket->waitForEncrypted();
     } else {
-        qDebug() << "Connectiong to host unencrypted";
         mSocket->connectToHost(mAddress, mPort, QIODevice::ReadOnly);
         mSocket->waitForConnected();
     }

core/backends/lan/landevicelink.cpp

@@ -57,7 +57,7 @@ void LanDeviceLink::setOnSsl(bool value) {
 bool LanDeviceLink::sendPackageEncrypted(QCA::PublicKey& key, NetworkPackage& np)
 {
     if (np.hasPayload()) {
-        np.setPayloadTransferInfo(sendPayload(np)->getTransferInfo());
+        np.setPayloadTransferInfo(sendPayload(np)->transferInfo());
     }
 
     if (!onSsl) {
@@ -75,7 +75,7 @@ bool LanDeviceLink::sendPackageEncrypted(QCA::PublicKey& key, NetworkPackage& np
 bool LanDeviceLink::sendPackage(NetworkPackage& np)
 {
     if (np.hasPayload()) {
-        np.setPayloadTransferInfo(sendPayload(np)->getTransferInfo());
+        np.setPayloadTransferInfo(sendPayload(np)->transferInfo());
     }
 
     int written = mSocketLineReader->write(np.serialize());
@@ -107,13 +107,12 @@ void LanDeviceLink::dataReceived()
     NetworkPackage::unserialize(package, &unserialized);
     if (unserialized.isEncrypted()) {
         //mPrivateKey should always be set when device link is added to device, no null-checking done here
-        // TODO : Check this with old device since package thorough ssl in unencrypted
+        // TODO : Check this with old device since package through ssl in unencrypted
         unserialized.decrypt(mPrivateKey, &unserialized);
-        qDebug() << "Serialized " << unserialized.serialize();
     }
 
     if (unserialized.hasPayloadTransferInfo()) {
-//        qCDebug(KDECONNECT_CORE) << "HasPayloadTransferInfo";
+        //qCDebug(KDECONNECT_CORE) << "HasPayloadTransferInfo";
         QVariantMap transferInfo = unserialized.payloadTransferInfo();
         if (onSsl) {
             transferInfo.insert("useSsl", true);

core/backends/lan/lanlinkprovider.cpp

@@ -51,7 +51,7 @@ LanLinkProvider::LanLinkProvider()
     mServer = new Server(this);
     connect(mServer,SIGNAL(newConnection()),this, SLOT(newConnection()));
 
-    pairingHandler = new LanPairingHandler();
+    m_pairingHandler = new LanPairingHandler();
 
     //Detect when a network interface changes status, so we announce ourelves in the new network
     QNetworkConfigurationManager* networkManager;
@@ -114,7 +114,6 @@ void LanLinkProvider::onNetworkChange()
 void LanLinkProvider::newUdpConnection()
 {
     while (mUdpServer->hasPendingDatagrams()) {
-        qCDebug(KDECONNECT_CORE) << "Udp package received";
 
         QByteArray datagram;
         datagram.resize(mUdpServer->pendingDatagramSize());
@@ -158,8 +157,6 @@ void LanLinkProvider::connectError()
     disconnect(socket, SIGNAL(connected()), this, SLOT(connected()));
     disconnect(socket, SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(connectError()));
 
-    qCDebug(KDECONNECT_CORE) << socket->errorString();
-
     qCDebug(KDECONNECT_CORE) << "Fallback (1), try reverse connection (send udp packet)";
     NetworkPackage np("");
     NetworkPackage::createIdentityPackage(&np);
@@ -168,8 +165,7 @@ void LanLinkProvider::connectError()
 
     //The socket we created didn't work, and we didn't manage
     //to create a LanDeviceLink from it, deleting everything.
-    delete receivedIdentityPackages[socket].np;
+    delete receivedIdentityPackages.take(socket).np;
-    receivedIdentityPackages.remove(socket);
     delete socket;
 }
 
@@ -177,7 +173,6 @@ void LanLinkProvider::connected()
 {
     qCDebug(KDECONNECT_CORE) << "Socket connected";
 
-    // TODO : Change the behaviour of these disconnects
     QSslSocket* socket = qobject_cast<QSslSocket*>(sender());
     if (!socket) return;
     disconnect(socket, SIGNAL(connected()), this, SLOT(connected()));
@@ -244,7 +239,8 @@ void LanLinkProvider::connected()
     //We don't delete the socket because now it's owned by the LanDeviceLink
 }
 
-void LanLinkProvider::encrypted() {
+void LanLinkProvider::encrypted()
+{
 
     qCDebug(KDECONNECT_CORE) << "Socket encrypted";
 
@@ -267,7 +263,7 @@ void LanLinkProvider::encrypted() {
 
 }
 
-void LanLinkProvider::sslErrors(QList<QSslError> errors)
+void LanLinkProvider::sslErrors(const QList<QSslError>& errors)
 {
     QSslSocket* socket = qobject_cast<QSslSocket*>(sender());
     if (!socket) return;
@@ -293,8 +289,7 @@ void LanLinkProvider::sslErrors(QList<QSslError> errors)
         }
     }
 
-    delete receivedIdentityPackages[socket].np;
+    delete receivedIdentityPackages.take(socket).np;
-    receivedIdentityPackages.remove(socket);
     // Socket disconnects itself on ssl error and will be deleted by deleteLater slot, no need to delete manually
 }
 
@@ -303,7 +298,7 @@ void LanLinkProvider::sslErrors(QList<QSslError> errors)
 //I'm the new device and this is the answer to my UDP identity package (no data received yet)
 void LanLinkProvider::newConnection()
 {
-    qDebug() << "LanLinkProvider newConnection " ;
+    //qCDebug(KDECONNECT_CORE) << "LanLinkProvider newConnection";
 
     while (mServer->hasPendingConnections()) {
         QSslSocket* socket = mServer->nextPendingConnection();
@@ -331,14 +326,14 @@ void LanLinkProvider::dataReceived()
     NetworkPackage* np = new NetworkPackage("");
     bool success = NetworkPackage::unserialize(data, np);
 
-    receivedIdentityPackages[socket].np = np;
-//     receivedIdentityPackages[socket].sender = sender;
-
     if (!success || np->type() != PACKAGE_TYPE_IDENTITY) {
         qCDebug(KDECONNECT_CORE) << "LanLinkProvider/newConnection: Not an identification package (wuh?)";
         return;
     }
 
+    // Needed in "encrypted" if ssl is used, similar to "connected"
+    receivedIdentityPackages[socket].np = np;
+
     const QString& deviceId = np->get<QString>("deviceId");
     //qCDebug(KDECONNECT_CORE) << "Handshaking done (i'm the new device)";
 
@@ -369,11 +364,14 @@ void LanLinkProvider::dataReceived()
         connect(socket, SIGNAL(encrypted()), this, SLOT(encrypted()));
 
         socket->startClientEncryption();
-
+        return;
     } else {
         addLink(deviceId, socket, np);
     }
 
+    delete np;
+    receivedIdentityPackages.remove(socket);
+
 }
 
 void LanLinkProvider::deviceLinkDestroyed(QObject* destroyedDeviceLink)

core/backends/lan/lanlinkprovider.h

@@ -58,7 +58,7 @@ private Q_SLOTS:
     void newConnection();
     void dataReceived();
     void deviceLinkDestroyed(QObject* destroyedDeviceLink);
-    void sslErrors(QList<QSslError> errors);
+    void sslErrors(const QList<QSslError>& errors);
 
 private:
     static void configureSocket(QSslSocket* socket);

core/backends/lan/lanpairinghandler.cpp

@@ -22,20 +22,20 @@
 #include "lanpairinghandler.h"
 #include "networkpackagetypes.h"
 
-LanPairingHandler::LanPairingHandler() {
+LanPairingHandler::LanPairingHandler()
-
+{
 }
 
-NetworkPackage LanPairingHandler::createPairPackage() {
+void LanPairingHandler::createPairPackage(NetworkPackage& np)
-    NetworkPackage np(PACKAGE_TYPE_PAIR);
+{
     np.set("pair", true);
     np.set("publicKey", KdeConnectConfig::instance()->publicKey().toPEM());
-    return np;
 }
 
-bool LanPairingHandler::packageReceived(Device *device, NetworkPackage np) {
+bool LanPairingHandler::packageReceived(Device *device,const NetworkPackage& np)
+{
     //Retrieve their public key
-    const QString& keyString = np.get<QString>("publicKey");
+    QString keyString = np.get<QString>("publicKey");
     device->setPublicKey(QCA::RSAPublicKey::fromPEM(keyString));
     if (device->publicKey().isNull()) {
         return false;
@@ -43,28 +43,32 @@ bool LanPairingHandler::packageReceived(Device *device, NetworkPackage np) {
     return true;
 }
 
-bool LanPairingHandler::requestPairing(Device *device) {
+bool LanPairingHandler::requestPairing(Device *device)
-    NetworkPackage np = createPairPackage();
+{
+    NetworkPackage np(PACKAGE_TYPE_PAIR);
+    createPairPackage(np);
     bool success = device->sendPackage(np);
     return success;
 }
 
-bool LanPairingHandler::acceptPairing(Device *device) {
+bool LanPairingHandler::acceptPairing(Device *device)
-    NetworkPackage np = createPairPackage();
+{
+    NetworkPackage np(PACKAGE_TYPE_PAIR);
+    createPairPackage(np);
     bool success = device->sendPackage(np);
     return success;
 }
 
-void LanPairingHandler::rejectPairing(Device *device) {
+void LanPairingHandler::rejectPairing(Device *device)
+{
     // TODO : check status of reject pairing
     NetworkPackage np(PACKAGE_TYPE_PAIR);
     np.set("pair", false);
     device->sendPackage(np);
 }
 
-void LanPairingHandler::pairingDone(Device *device) {
+void LanPairingHandler::pairingDone(Device *device)
-    // TODO : Save certificate and public key here
+{
-
     // No need to worry, if either of certificate or public key is null an empty qstring will be returned
     KdeConnectConfig::instance()->setDeviceProperty(device->id(), "key", device->publicKey().toPEM());
     KdeConnectConfig::instance()->setDeviceProperty(device->id(), "certificate", QString(device->certificate().toPem()));
@@ -74,4 +78,5 @@ void LanPairingHandler::unpair(Device *device) {
     NetworkPackage np(PACKAGE_TYPE_PAIR);
     np.set("pair", false);
     bool success = device->sendPackage(np);
+    Q_UNUSED(success);
 }

core/backends/lan/lanpairinghandler.h

@@ -30,8 +30,8 @@ public:
     LanPairingHandler();
     virtual ~LanPairingHandler() { }
 
-    virtual NetworkPackage createPairPackage() Q_DECL_OVERRIDE;
+    virtual void createPairPackage(NetworkPackage& np) Q_DECL_OVERRIDE;
-    virtual bool packageReceived(Device *device, NetworkPackage np) Q_DECL_OVERRIDE;
+    virtual bool packageReceived(Device *device,const NetworkPackage& np) Q_DECL_OVERRIDE;
     virtual bool requestPairing(Device *device) Q_DECL_OVERRIDE;
     virtual bool acceptPairing(Device *device) Q_DECL_OVERRIDE;
     virtual void rejectPairing(Device *device) Q_DECL_OVERRIDE;

core/backends/lan/server.cpp

@@ -34,9 +34,9 @@ Server::Server(QObject * parent)
 }
 
 void Server::incomingConnection(qintptr socketDescriptor) {
-    QSslSocket *serverSocket = new QSslSocket;
+    QSslSocket *serverSocket = new QSslSocket(parent());
     if (serverSocket->setSocketDescriptor(socketDescriptor)) {
-        pendingConnections.push_back(serverSocket);
+        pendingConnections.append(serverSocket);
         Q_EMIT newConnection();
     } else {
         delete serverSocket;
@@ -44,15 +44,13 @@ void Server::incomingConnection(qintptr socketDescriptor) {
 }
 
 QSslSocket* Server::nextPendingConnection() {
-    if (pendingConnections.size() == 0) {
+    if (pendingConnections.isEmpty()) {
         return Q_NULLPTR;
     } else {
-        QSslSocket *socket = pendingConnections.first();
+        return pendingConnections.takeFirst();
-        pendingConnections.removeFirst();
-        return socket;
     }
 }
 
 bool Server::hasPendingConnections() const {
-    return pendingConnections.size() != 0;
+    return !pendingConnections.isEmpty();
 }

core/backends/lan/server.h

@@ -26,7 +26,7 @@
 #include <QtNetwork/qsslsocket.h>
 
 class Server
-    : public QTcpServers
+    : public QTcpServer
 {
 
     Q_OBJECT
@@ -34,7 +34,7 @@ private:
     QList<QSslSocket*> pendingConnections;
 
 public:
-    Server(QObject* parent = 0);
+    Server(QObject* parent);
     virtual ~Server() {}
 
     QSslSocket* nextPendingConnection() Q_DECL_OVERRIDE;

core/backends/lan/uploadjob.cpp

@@ -25,7 +25,7 @@
 #include "uploadjob.h"
 #include "core_debug.h"
 
-UploadJob::UploadJob(const QSharedPointer<QIODevice>& source, QVariantMap transferInfo): KJob()
+UploadJob::UploadJob(const QSharedPointer<QIODevice>& source, const QVariantMap& transferInfo): KJob()
 {
     mInput = source;
     mServer = new Server(this);
@@ -33,7 +33,7 @@ UploadJob::UploadJob(const QSharedPointer<QIODevice>& source, QVariantMap transf
     mPort = 0;
 
     // We will use this info if link is on ssl, to send encrypted payload
-    this->transferInfo = transferInfo;
+    this->mTransferInfo = transferInfo;
 
     connect(mInput.data(), SIGNAL(readyRead()), this, SLOT(readyRead()));
     connect(mInput.data(), SIGNAL(aboutToClose()), this, SLOT(aboutToClose()));
@@ -50,24 +50,29 @@ void UploadJob::start()
             return;
         }
     }
-    connect(mServer, SIGNAL(newConnection(QSslSocket*)), this, SLOT(newConnection(QSslSocket*)));
+    connect(mServer, SIGNAL(newConnection()), this, SLOT(newConnection()));
 }
 
-void UploadJob::newConnection(QSslSocket* socket)
+void UploadJob::newConnection()
 {
     if (!mInput->open(QIODevice::ReadOnly)) {
         qWarning() << "error when opening the input to upload";
         return; //TODO: Handle error, clean up...
     }
 
-    mSocket = socket;
+    Server* server = qobject_cast<Server*>(sender());
+    // FIXME : It is called again when payload sending is finished. Unsolved mystery :(
+    disconnect(mServer, SIGNAL(newConnection()), this, SLOT(newConnection()));
 
-    if (transferInfo.value("useSsl", false).toBool()) {
+    mSocket = server->nextPendingConnection();
+    connect(mSocket, SIGNAL(disconnected()), mSocket, SLOT(deleteLater()));
+
+    if (mTransferInfo.value("useSsl", false).toBool()) {
         mSocket->setLocalCertificate(KdeConnectConfig::instance()->certificate());
         mSocket->setPrivateKey(KdeConnectConfig::instance()->privateKeyPath());
         mSocket->setProtocol(QSsl::TlsV1_2);
-        mSocket->setPeerVerifyName(transferInfo.value("deviceId").toString());
+        mSocket->setPeerVerifyName(mTransferInfo.value("deviceId").toString());
-        mSocket->addCaCertificate(QSslCertificate(KdeConnectConfig::instance()->getTrustedDevice(transferInfo.value("deviceId").toString()).certificate.toLatin1()));
+        mSocket->addCaCertificate(QSslCertificate(KdeConnectConfig::instance()->getTrustedDevice(mTransferInfo.value("deviceId").toString()).certificate.toLatin1()));
         mSocket->startServerEncryption();
         mSocket->waitForEncrypted();
     }
@@ -101,7 +106,7 @@ void UploadJob::aboutToClose()
     emitResult();
 }
 
-QVariantMap UploadJob::getTransferInfo()
+QVariantMap UploadJob::transferInfo()
 {
     Q_ASSERT(mPort != 0);
 

core/backends/lan/uploadjob.h

@@ -34,20 +34,20 @@ class UploadJob
 {
     Q_OBJECT
 public:
-    UploadJob(const QSharedPointer<QIODevice>& source, QVariantMap sslInfo);
+    UploadJob(const QSharedPointer<QIODevice>& source,const QVariantMap& sslInfo);
     virtual void start();
-    QVariantMap getTransferInfo();
+    QVariantMap transferInfo();
 
 private:
     QSharedPointer<QIODevice> mInput;
     Server* mServer;
     QSslSocket* mSocket;
     quint16 mPort;
-    QVariantMap transferInfo;
+    QVariantMap mTransferInfo;
 
 private Q_SLOTS:
     void readyRead();
-    void newConnection(QSslSocket*);
+    void newConnection();
     void aboutToClose();
 };
 

core/backends/linkprovider.h

@@ -35,9 +35,6 @@ class LinkProvider
 {
     Q_OBJECT
 
-protected:
-    PairingHandler* pairingHandler;
-
 public:
 
     const static int PRIORITY_LOW = 0;      //eg: 3g internet
@@ -49,7 +46,10 @@ public:
 
     virtual QString name() = 0;
     virtual int priority() = 0;
-    PairingHandler* getPairingHandler() { return pairingHandler;}
+    PairingHandler* pairingHandler() { return m_pairingHandler;}
+
+protected:
+    PairingHandler* m_pairingHandler;
 
 
 public Q_SLOTS:

core/backends/pairinghandler.h

@@ -31,8 +31,8 @@ public:
     PairingHandler();
     virtual ~PairingHandler() { }
 
-    virtual NetworkPackage createPairPackage() = 0;
+    virtual void createPairPackage(NetworkPackage& np) = 0;
-    virtual bool packageReceived(Device *device, NetworkPackage np) = 0;
+    virtual bool packageReceived(Device *device,const NetworkPackage& np) = 0;
     virtual bool requestPairing(Device *device) = 0;
     virtual bool acceptPairing(Device *device) = 0;
     virtual void rejectPairing(Device *device) = 0;

core/daemon.cpp

@@ -133,7 +133,7 @@ void Daemon::onNewDeviceLink(const NetworkPackage& identityPackage, DeviceLink*
 {
     const QString& id = identityPackage.get<QString>("deviceId");
 
-    qCDebug(KDECONNECT_CORE) << "Device discovered" << id << "via" << dl->provider()->name();
+    //qCDebug(KDECONNECT_CORE) << "Device discovered" << id << "via" << dl->provider()->name();
 
     if (d->mDevices.contains(id)) {
         qCDebug(KDECONNECT_CORE) << "It is a known device " << identityPackage.get<QString>("deviceName");

core/device.cpp

@@ -210,7 +210,7 @@ void Device::requestPair()
 
     //Send our own public key
     Q_FOREACH(DeviceLink* dl, m_deviceLinks) {
-        bool success = dl->provider()->getPairingHandler()->requestPairing(this);
+        bool success = dl->provider()->pairingHandler()->requestPairing(this);
 
         if (!success) {
             m_pairStatus = Device::NotPaired;
@@ -230,7 +230,7 @@ void Device::unpair()
 {
 
     Q_FOREACH(DeviceLink* dl, m_deviceLinks) {
-        dl->provider()->getPairingHandler()->unpair(this);
+        dl->provider()->pairingHandler()->unpair(this);
     }
 
     unpairInternal();
@@ -282,9 +282,7 @@ void Device::addLink(const NetworkPackage& identityPackage, DeviceLink* link)
     // Set certificate if the link is on ssl, and it is added to identity package
     // This is always sets certificate when link is added to device
     if (identityPackage.has("certificate")) {
-        qDebug() << "Got certificate" ;
         m_certificate = QSslCertificate(identityPackage.get<QByteArray>("certificate"));
-//        qDebug() << m_certificate.toText();
     }
 
     //Theoretically we will never add two links from the same provider (the provider should destroy
@@ -356,7 +354,7 @@ void Device::privateReceivedPackage(const NetworkPackage& np)
             } else if (m_pairStatus == Device::Paired) {
                 // If other request's pairing, and we have pair status Paired, send accept pairing
                 Q_FOREACH(DeviceLink* dl, m_deviceLinks) {
-                       dl->provider()->getPairingHandler()->acceptPairing(this);
+                       dl->provider()->pairingHandler()->acceptPairing(this);
                 }
             }
             return;
@@ -365,7 +363,7 @@ void Device::privateReceivedPackage(const NetworkPackage& np)
         if (wantsPair) {
 
             Q_FOREACH(DeviceLink* dl, m_deviceLinks) {
-                bool success = dl->provider()->getPairingHandler()->packageReceived(this, np);
+                bool success = dl->provider()->pairingHandler()->packageReceived(this, np);
                 if (!success) {
                     if (m_pairStatus == Device::Requested) {
                         m_pairStatus = Device::NotPaired;
@@ -434,7 +432,7 @@ void Device::rejectPairing()
     m_pairStatus = Device::NotPaired;
 
     Q_FOREACH(DeviceLink* link, m_deviceLinks) {
-        link->provider()->getPairingHandler()->rejectPairing(this);
+        link->provider()->pairingHandler()->rejectPairing(this);
     }
 
     Q_EMIT pairingFailed(i18n("Canceled by the user"));
@@ -449,7 +447,7 @@ void Device::acceptPairing()
 
     bool success;
     Q_FOREACH(DeviceLink* link, m_deviceLinks) {
-        success = link->provider()->getPairingHandler()->acceptPairing(this);
+        success = link->provider()->pairingHandler()->acceptPairing(this);
     }
 
     if (!success) {

core/kdeconnectconfig.cpp

@@ -33,7 +33,7 @@
 #include <QCoreApplication>
 #include <QHostInfo>
 #include <QSettings>
-#include <QtNetwork/qsslcertificate.h>
+#include <QSslCertificate>
 
 #include "core_debug.h"
 #include "dbushelper.h"

core/networkpackage.cpp

@@ -47,14 +47,6 @@ NetworkPackage::NetworkPackage(const QString& type)
     mPayloadSize = 0;
 }
 
-NetworkPackage::NetworkPackage(const NetworkPackage &np) {
-    mId = np.id();
-    mType = np.type();
-    mBody = np.body();
-    mPayload = np.payload();
-    mPayloadSize = np.payloadSize();
-}
-
 void NetworkPackage::createIdentityPackage(NetworkPackage* np)
 {
     KdeConnectConfig* config = KdeConnectConfig::instance();

tests/testsocketlinereader.cpp

@@ -79,7 +79,7 @@ void TestSocketLineReader::socketLineReader()
         QTest::qSleep(1000);
     }
 
-    QSslSocket *sock = (QSslSocket*) mServer->nextPendingConnection();
+    QSslSocket *sock = mServer->nextPendingConnection();
 
     QVERIFY2(sock != 0, "Could not open a connection to the client");