Fix crash in client connection if the server is already gone bu data where pending, update monitor

libnymea-remoteproxy/server/tcpsocketserver.cpp

@@ -45,8 +45,7 @@ TcpSocketServer::~TcpSocketServer()
 
 void TcpSocketServer::sendData(const QUuid &clientId, const QByteArray &data)
 {
-    QTcpSocket *client = nullptr;
+    QTcpSocket *client = m_clientList.value(clientId);
-    client = m_clientList.value(clientId);
     if (!client) {
         qCWarning(dcTcpSocketServer()) << "Client" << clientId << "unknown to this transport";
         return;
@@ -67,6 +66,7 @@ void TcpSocketServer::killClientConnection(const QUuid &clientId, const QString
     }
 
     qCDebug(dcTcpSocketServer()) << "Killing client connection" << clientId.toString() << "Reason:" << killReason;
+    client->flush();
     client->close();
 }
 
@@ -83,31 +83,6 @@ bool TcpSocketServer::running() const
     return m_server->isListening();
 }
 
-void TcpSocketServer::onDataAvailable(QSslSocket *client, const QByteArray &data)
-{
-    //qCDebug(dcTcpSocketServerTraffic()) << "Emitting data available internal.";
-    QUuid clientId = m_clientList.key(qobject_cast<QTcpSocket *>(client));
-    emit dataAvailable(clientId, data);
-}
-
-void TcpSocketServer::onSocketConnected(QSslSocket *client)
-{
-    QUuid clientId = QUuid::createUuid();
-    qCDebug(dcTcpSocketServer()) << "New client connected:" << client << client->peerAddress().toString() << clientId.toString();
-    m_clientList.insert(clientId, client);
-    emit clientConnected(clientId, client->peerAddress());
-}
-
-void TcpSocketServer::onSocketDisconnected(QSslSocket *client)
-{
-    QUuid clientId = m_clientList.key(client);
-    qCDebug(dcTcpSocketServer()) << "Client disconnected:" << client << client->peerAddress().toString() << clientId.toString();
-    if (m_clientList.take(clientId)) {
-        // Note: the SslServer is deleting the socket object
-        emit clientDisconnected(clientId);
-    }
-}
-
 bool TcpSocketServer::startServer()
 {
     qCDebug(dcTcpSocketServer()) << "Starting TCP server" << m_serverUrl.toString();
@@ -128,21 +103,53 @@ bool TcpSocketServer::startServer()
 
 bool TcpSocketServer::stopServer()
 {
+    if (!m_server)
+        return true;
+
     // Clean up client connections
     foreach (const QUuid &clientId, m_clientList.keys()) {
         killClientConnection(clientId, "Stop server");
     }
 
-    if (!m_server)
-        return true;
-
     qCDebug(dcTcpSocketServer()) << "Stop server" << m_serverUrl.toString();
+
     m_server->close();
-    delete m_server;
+    m_server->deleteLater();
     m_server = nullptr;
     return true;
 }
 
+
+void TcpSocketServer::onDataAvailable(QSslSocket *client, const QByteArray &data)
+{
+    //qCDebug(dcTcpSocketServerTraffic()) << "Emitting data available internal.";
+    QUuid clientId = m_clientList.key(client);
+    emit dataAvailable(clientId, data);
+}
+
+void TcpSocketServer::onSocketConnected(QSslSocket *client)
+{
+    QUuid clientId = QUuid::createUuid();
+    qCDebug(dcTcpSocketServer()) << "New client connected:" << client << client->peerAddress().toString() << clientId.toString();
+    m_clientList.insert(clientId, client);
+    emit clientConnected(clientId, client->peerAddress());
+}
+
+void TcpSocketServer::onSocketDisconnected(QSslSocket *client)
+{
+    QUuid clientId = m_clientList.key(client);
+    if (clientId.isNull()) {
+        qCWarning(dcTcpSocketServer()) << "Socket disconnected but the uuid is null." << client << client->peerAddress().toString() << clientId.toString();
+        return;
+    }
+    qCDebug(dcTcpSocketServer()) << "Client disconnected:" << client << client->peerAddress().toString() << clientId.toString();
+    m_clientList.take(clientId);
+    // Note: the SslServer is deleting the socket object
+    emit clientDisconnected(clientId);
+}
+
+
+
 SslServer::SslServer(bool sslEnabled, const QSslConfiguration &config, QObject *parent) :
     QTcpServer(parent),
     m_sslEnabled(sslEnabled),
@@ -155,11 +162,11 @@ void SslServer::incomingConnection(qintptr socketDescriptor)
 {
     QSslSocket *sslSocket = new QSslSocket(this);
     qCDebug(dcTcpSocketServer()) << "Incomming connection" << sslSocket;
-    connect(sslSocket, &QSslSocket::readyRead, this, &SslServer::onSocketReadyRead);
     connect(sslSocket, &QSslSocket::disconnected, this, &SslServer::onSocketDisconnected);
 
     typedef void (QAbstractSocket:: *errorSignal)(QAbstractSocket::SocketError);
     connect(sslSocket, static_cast<errorSignal>(&QAbstractSocket::error), this, &SslServer::onSocketError);
+
     connect(sslSocket, &QSslSocket::encrypted, this, [this, sslSocket](){
         qCDebug(dcTcpSocketServer()) << "SSL encryption established for" << sslSocket;
         emit socketConnected(sslSocket);
@@ -167,12 +174,14 @@ void SslServer::incomingConnection(qintptr socketDescriptor)
 
     typedef void (QSslSocket:: *sslErrorsSignal)(const QList<QSslError> &);
     connect(sslSocket, static_cast<sslErrorsSignal>(&QSslSocket::sslErrors), this, [](const QList<QSslError> &errors) {
-        qCWarning(dcTcpSocketServer()) << "SSL Errors happened in the client connections:";
+        qCWarning(dcTcpSocketServer()) << "SSL error occurred in the client connection:";
         foreach (const QSslError &error, errors) {
-            qCWarning(dcTcpSocketServer()) << "SSL Error:" << error.error() << error.errorString();
+            qCWarning(dcTcpSocketServer()) << "SSL error:" << error.error() << error.errorString();
         }
     });
 
+    connect(sslSocket, &QSslSocket::readyRead, this, &SslServer::onSocketReadyRead);
+
     if (!sslSocket->setSocketDescriptor(socketDescriptor)) {
         qCWarning(dcTcpSocketServer()) << "Failed to set SSL socket descriptor.";
         delete sslSocket;
@@ -182,7 +191,6 @@ void SslServer::incomingConnection(qintptr socketDescriptor)
     if (m_sslEnabled) {
         qCDebug(dcTcpSocketServer()) << "Start SSL encryption for" << sslSocket;
         sslSocket->setSslConfiguration(m_config);
-        //addPendingConnection(sslSocket);
         sslSocket->startServerEncryption();
     } else {
         emit socketConnected(sslSocket);
@@ -191,7 +199,7 @@ void SslServer::incomingConnection(qintptr socketDescriptor)
 
 void SslServer::onSocketDisconnected()
 {
-    QSslSocket *sslSocket = qobject_cast<QSslSocket *>(sender());
+    QSslSocket *sslSocket = static_cast<QSslSocket *>(sender());
     qCDebug(dcTcpSocketServer()) << "Client socket disconnected:" << sslSocket << sslSocket->peerAddress().toString();;
     emit socketDisconnected(sslSocket);
     sslSocket->deleteLater();
@@ -199,7 +207,7 @@ void SslServer::onSocketDisconnected()
 
 void SslServer::onSocketReadyRead()
 {
-    QSslSocket *sslSocket = qobject_cast<QSslSocket *>(sender());
+    QSslSocket *sslSocket = static_cast<QSslSocket *>(sender());
     QByteArray data = sslSocket->readAll();
     qCDebug(dcTcpSocketServerTraffic()) << "Data from socket" << sslSocket->peerAddress().toString() << data;
     emit dataAvailable(sslSocket, data);
@@ -207,7 +215,26 @@ void SslServer::onSocketReadyRead()
 
 void SslServer::onSocketError(QAbstractSocket::SocketError error)
 {
-    qCWarning(dcTcpSocketServer()) << "Socket error occurred" << error;
+    QSslSocket *sslSocket = static_cast<QSslSocket *>(sender());
+    qCWarning(dcTcpSocketServer()) << "Socket error occurred" << error << sslSocket->errorString();
+    switch(error) {
+    case QAbstractSocket::SocketResourceError:
+    case QAbstractSocket::SocketTimeoutError:
+    case QAbstractSocket::DatagramTooLargeError:
+    case QAbstractSocket::NetworkError:
+    case QAbstractSocket::SslHandshakeFailedError:
+    case QAbstractSocket::UnfinishedSocketOperationError:
+    case QAbstractSocket::SslInternalError:
+    case QAbstractSocket::SslInvalidUserDataError:
+    case QAbstractSocket::TemporaryError:
+    case QAbstractSocket::UnknownSocketError:
+        qCWarning(dcTcpSocketServer()) << "Explitily closing the socket due to error.";
+        sslSocket->close();
+        break;
+    default:
+        break;
+    }
 }
 
 }
+

libnymea-remoteproxy/server/tcpsocketserver.h

@@ -45,10 +45,6 @@ public:
     explicit SslServer(bool sslEnabled, const QSslConfiguration &config, QObject *parent = nullptr);
     ~SslServer() override = default;
 
-private:
-    bool m_sslEnabled = false;
-    QSslConfiguration m_config;
-
 signals:
     void socketConnected(QSslSocket *socket);
     void socketDisconnected(QSslSocket *socket);
@@ -57,6 +53,10 @@ signals:
 protected:
     void incomingConnection(qintptr socketDescriptor) override;
 
+private:
+    bool m_sslEnabled = false;
+    QSslConfiguration m_config;
+
 private slots:
     void onSocketDisconnected();
     void onSocketReadyRead();
@@ -79,11 +79,15 @@ public:
 
     bool running() const override;
 
+public slots:
+    bool startServer() override;
+    bool stopServer() override;
+
 private:
     bool m_sslEnabled;
     QSslConfiguration m_sslConfiguration;
 
-    QHash<QUuid, QTcpSocket *> m_clientList;
+    QHash<QUuid, QSslSocket *> m_clientList;
 
     SslServer *m_server = nullptr;
 
@@ -92,9 +96,6 @@ private slots:
     void onSocketConnected(QSslSocket *client);
     void onSocketDisconnected(QSslSocket *client);
 
-public slots:
-    bool startServer() override;
-    bool stopServer() override;
 
 };
 

libnymea-remoteproxy/tunnelproxy/tunnelproxyserver.cpp

@@ -357,7 +357,11 @@ void TunnelProxyServer::onClientDataAvailable(const QUuid &clientId, const QByte
             return;
         }
 
-        Q_ASSERT_X(clientConnection->serverConnection(), "TunnelProxyServer", "The client has not been registered to a server connection");
+        if (!clientConnection->serverConnection()) {
+            qCWarning(dcTunnelProxyServer()) << "Valid client wants to send data to the server, but there is no server registered for this client.";
+            Q_ASSERT_X(clientConnection->serverConnection(), "TunnelProxyServer", "The client has not been registered to a server connection");
+            return;
+        }
 
         SlipDataProcessor::Frame frame;
         frame.socketAddress = clientConnection->socketAddress();

monitor/noninteractivemonitor.cpp

@@ -75,8 +75,9 @@ void NonInteractiveMonitor::onConnected()
                 serverLinePrint.prepend("├┬─");
             }
 
-            serverLinePrint += QString("%1 | %2 RX: %3 TX: %4 | %5")
+            serverLinePrint += QString("%1 | %2 | %3 RX: %4 TX: %5 | %6")
                     .arg(serverConnectionTime)
+                    .arg(serverMap.value("serverUuid").toString())
                     .arg(serverMap.value("address").toString(), - 15)
                     .arg(Utils::humanReadableTraffic(serverMap.value("rxDataCount").toInt()), - 9)
                     .arg(Utils::humanReadableTraffic(serverMap.value("txDataCount").toInt()), - 9)
@@ -93,8 +94,9 @@ void NonInteractiveMonitor::onConnected()
                     clientLinePrint.prepend("│├─");
                 }
 
-                clientLinePrint += QString("%1 | %2 RX: %3 TX: %4 | %5")
+                clientLinePrint += QString("%1 | %2 | %3 RX: %4 TX: %5 | %6")
                         .arg(QDateTime::fromTime_t(clientMap.value("timestamp").toUInt()).toString("dd.MM.yyyy hh:mm:ss"))
+                        .arg(clientMap.value("clientUuid").toString())
                         .arg(clientMap.value("address").toString(), - 15)
                         .arg(Utils::humanReadableTraffic(serverMap.value("rxDataCount").toInt()), - 9)
                         .arg(Utils::humanReadableTraffic(serverMap.value("txDataCount").toInt()), - 9)