From 8ee90aaccd8d83b20cc959b6ab6565ae4961658e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=BCrz?= Date: Sat, 18 Feb 2023 00:11:51 +0100 Subject: [PATCH] Fix crash in client connection if the server is already gone bu data where pending, update monitor --- .../server/tcpsocketserver.cpp | 103 +++++++++++------- libnymea-remoteproxy/server/tcpsocketserver.h | 17 +-- .../tunnelproxy/tunnelproxyserver.cpp | 6 +- monitor/noninteractivemonitor.cpp | 6 +- 4 files changed, 83 insertions(+), 49 deletions(-) diff --git a/libnymea-remoteproxy/server/tcpsocketserver.cpp b/libnymea-remoteproxy/server/tcpsocketserver.cpp index 39cf2ec..ba623c9 100644 --- a/libnymea-remoteproxy/server/tcpsocketserver.cpp +++ b/libnymea-remoteproxy/server/tcpsocketserver.cpp @@ -45,8 +45,7 @@ TcpSocketServer::~TcpSocketServer() void TcpSocketServer::sendData(const QUuid &clientId, const QByteArray &data) { - QTcpSocket *client = nullptr; - client = m_clientList.value(clientId); + QTcpSocket *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(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(&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 &); connect(sslSocket, static_cast(&QSslSocket::sslErrors), this, [](const QList &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(sender()); + QSslSocket *sslSocket = static_cast(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(sender()); + QSslSocket *sslSocket = static_cast(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(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; + } } } + diff --git a/libnymea-remoteproxy/server/tcpsocketserver.h b/libnymea-remoteproxy/server/tcpsocketserver.h index 900b70a..f312402 100644 --- a/libnymea-remoteproxy/server/tcpsocketserver.h +++ b/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 m_clientList; + QHash 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; }; diff --git a/libnymea-remoteproxy/tunnelproxy/tunnelproxyserver.cpp b/libnymea-remoteproxy/tunnelproxy/tunnelproxyserver.cpp index 080f2a4..986e814 100644 --- a/libnymea-remoteproxy/tunnelproxy/tunnelproxyserver.cpp +++ b/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(); diff --git a/monitor/noninteractivemonitor.cpp b/monitor/noninteractivemonitor.cpp index 90c1f6e..de456a4 100644 --- a/monitor/noninteractivemonitor.cpp +++ b/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)