From 2fe0569bd6213205949bf5a1f49600958dfe0e24 Mon Sep 17 00:00:00 2001 From: Octol1ttle Date: Thu, 9 Apr 2026 17:41:25 +0500 Subject: [PATCH 1/2] fix(McClient): do not use unsigned type for response length Signed-off-by: Octol1ttle --- launcher/ui/pages/instance/McClient.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launcher/ui/pages/instance/McClient.h b/launcher/ui/pages/instance/McClient.h index 633e7aaed..f0a6808fa 100644 --- a/launcher/ui/pages/instance/McClient.h +++ b/launcher/ui/pages/instance/McClient.h @@ -20,7 +20,7 @@ class McClient : public QObject { // 1: read the response length, still reading the response // 2: finished reading the response unsigned m_responseReadState = 0; - unsigned m_wantedRespLength = 0; + int32_t m_wantedRespLength = 0; QByteArray m_resp; public: From 91616ae9b6379d18dec5131870554bfacc5f7543 Mon Sep 17 00:00:00 2001 From: Octol1ttle Date: Thu, 9 Apr 2026 18:57:33 +0500 Subject: [PATCH 2/2] refactor: McClient Signed-off-by: Octol1ttle --- launcher/ui/pages/instance/McClient.cpp | 90 +++++++++++++------------ launcher/ui/pages/instance/McClient.h | 59 ++++++++-------- 2 files changed, 78 insertions(+), 71 deletions(-) diff --git a/launcher/ui/pages/instance/McClient.cpp b/launcher/ui/pages/instance/McClient.cpp index f110e1fe8..0a719431d 100644 --- a/launcher/ui/pages/instance/McClient.cpp +++ b/launcher/ui/pages/instance/McClient.cpp @@ -1,18 +1,17 @@ +#include "McClient.h" + #include #include #include #include +#include -#include +#include "Exception.h" #include "Json.h" -#include "McClient.h" -// 7 first bits -#define SEGMENT_BITS 0x7F -// last bit -#define CONTINUE_BIT 0x80 - -McClient::McClient(QObject* parent, QString domain, QString ip, short port) : QObject(parent), m_domain(domain), m_ip(ip), m_port(port) {} +McClient::McClient(QObject* parent, QString domain, QString ip, const uint16_t port) + : QObject(parent), m_domain(std::move(domain)), m_ip(std::move(ip)), m_port(port) +{} void McClient::getStatusData() { @@ -33,13 +32,12 @@ void McClient::getStatusData() void McClient::sendRequest() { QByteArray data; - writeVarInt(data, 0x00); // packet ID - writeVarInt(data, 763); // hardcoded protocol version (763 = 1.20.1) - writeVarInt(data, m_domain.size()); // server address length - writeString(data, m_domain.toStdString()); // server address - writeFixedInt(data, m_port, 2); // server port - writeVarInt(data, 0x01); // next state - writePacketToSocket(data); // send handshake packet + writeVarInt(data, 0x00); // packet ID + writeVarInt(data, 763); // hardcoded protocol version (763 = 1.20.1) + writeString(data, m_domain); // server address + writeUInt16(data, m_port); // server port + writeVarInt(data, 0x01); // next state + writePacketToSocket(data); // send handshake packet writeVarInt(data, 0x00); // packet ID writePacketToSocket(data); // send status packet @@ -47,17 +45,17 @@ void McClient::sendRequest() void McClient::readRawResponse() { - if (m_responseReadState == 2) { + if (m_responseReadState == ResponseReadState::Finished) { return; } m_resp.append(m_socket.readAll()); - if (m_responseReadState == 0 && m_resp.size() >= 5) { + if (m_responseReadState == ResponseReadState::Waiting && m_resp.size() >= 5) { m_wantedRespLength = readVarInt(m_resp); - m_responseReadState = 1; + m_responseReadState = ResponseReadState::GotLength; } - if (m_responseReadState == 1 && m_resp.size() >= m_wantedRespLength) { + if (m_responseReadState == ResponseReadState::GotLength && m_resp.size() >= m_wantedRespLength) { if (m_resp.size() > m_wantedRespLength) { qDebug().nospace() << "Warning: Packet length doesn't match actual packet size (" << m_wantedRespLength << " expected vs " << m_resp.size() << " received)"; @@ -67,7 +65,7 @@ void McClient::readRawResponse() } catch (const Exception& e) { emitFail(e.cause()); } - m_responseReadState = 2; + m_responseReadState = ResponseReadState::Finished; } } @@ -75,7 +73,7 @@ void McClient::parseResponse() { qDebug() << "Received response successfully"; - int packetID = readVarInt(m_resp); + const int packetID = readVarInt(m_resp); if (packetID != 0x00) { throw Exception(QString("Packet ID doesn't match expected value (0x00 vs 0x%1)").arg(packetID, 0, 16)); } @@ -84,7 +82,7 @@ void McClient::parseResponse() // 'resp' should now be the JSON string QJsonParseError parseError; - QJsonDocument doc = Json::parseUntilGarbage(m_resp, &parseError); + const QJsonDocument doc = Json::parseUntilGarbage(m_resp, &parseError); if (parseError.error != QJsonParseError::NoError) { qDebug() << "Failed to parse JSON:" << parseError.errorString(); emitFail(parseError.errorString()); @@ -93,18 +91,23 @@ void McClient::parseResponse() emitSucceed(doc.object()); } +// NOLINTBEGIN(*-signed-bitwise) + // From https://wiki.vg/Protocol#VarInt_and_VarLong +constexpr uint8_t g_varIntValueMask = 0x7F; +constexpr uint8_t g_varIntContinue = 0x80; + void McClient::writeVarInt(QByteArray& data, int value) { - while ((value & ~SEGMENT_BITS)) { // check if the value is too big to fit in 7 bits + while ((value & ~g_varIntValueMask) != 0) { // check if the value is too big to fit in 7 bits // Write 7 bits - data.append((value & SEGMENT_BITS) | CONTINUE_BIT); + data.append(static_cast((value & ~g_varIntValueMask) | g_varIntContinue)); // NOLINT(*-narrowing-conversions) // Erase theses 7 bits from the value to write // Note: >>> means that the sign bit is shifted with the rest of the number rather than being left alone value >>= 7; } - data.append(value); + data.append(static_cast(value)); // NOLINT(*-narrowing-conversions) } // From https://wiki.vg/Protocol#VarInt_and_VarLong @@ -112,53 +115,56 @@ int McClient::readVarInt(QByteArray& data) { int value = 0; int position = 0; - char currentByte; while (position < 32) { - currentByte = readByte(data); - value |= (currentByte & SEGMENT_BITS) << position; + const uint8_t currentByte = readByte(data); + value |= (currentByte & g_varIntValueMask) << position; - if ((currentByte & CONTINUE_BIT) == 0) + if ((currentByte & g_varIntContinue) == 0) { break; + } position += 7; } - if (position >= 32) + if (position >= 32) { throw Exception("VarInt is too big"); + } return value; } -char McClient::readByte(QByteArray& data) +// NOLINTEND(*-signed-bitwise) + +uint8_t McClient::readByte(QByteArray& data) { if (data.isEmpty()) { throw Exception("No more bytes to read"); } - char byte = data.at(0); + const uint8_t byte = data.at(0); data.remove(0, 1); return byte; } -// write number with specified size in big endian format -void McClient::writeFixedInt(QByteArray& data, int value, int size) +void McClient::writeUInt16(QByteArray& data, const uint16_t value) { - for (int i = size - 1; i >= 0; i--) { - data.append((value >> (i * 8)) & 0xFF); - } + QDataStream stream(&data, QIODeviceBase::Append); + stream.setByteOrder(QDataStream::BigEndian); + stream << value; } -void McClient::writeString(QByteArray& data, const std::string& value) +void McClient::writeString(QByteArray& data, const QString& value) { - data.append(value.c_str()); + writeVarInt(data, static_cast(value.size())); + data.append(value.toUtf8()); } void McClient::writePacketToSocket(QByteArray& data) { // we prefix the packet with its length QByteArray dataWithSize; - writeVarInt(dataWithSize, data.size()); + writeVarInt(dataWithSize, static_cast(data.size())); dataWithSize.append(data); // write it to the socket @@ -168,7 +174,7 @@ void McClient::writePacketToSocket(QByteArray& data) data.clear(); } -void McClient::emitFail(QString error) +void McClient::emitFail(const QString& error) { qDebug() << "Minecraft server ping for status error:" << error; emit failed(error); @@ -177,6 +183,6 @@ void McClient::emitFail(QString error) void McClient::emitSucceed(QJsonObject data) { - emit succeeded(data); + emit succeeded(std::move(data)); emit finished(); } diff --git a/launcher/ui/pages/instance/McClient.h b/launcher/ui/pages/instance/McClient.h index f0a6808fa..c1cb3d748 100644 --- a/launcher/ui/pages/instance/McClient.h +++ b/launcher/ui/pages/instance/McClient.h @@ -1,53 +1,54 @@ #pragma once + #include -#include #include #include #include -#include - // Client for the Minecraft protocol class McClient : public QObject { Q_OBJECT - QString m_domain; - QString m_ip; - short m_port; - QTcpSocket m_socket; - - // 0: did not start reading the response yet - // 1: read the response length, still reading the response - // 2: finished reading the response - unsigned m_responseReadState = 0; - int32_t m_wantedRespLength = 0; - QByteArray m_resp; - public: - explicit McClient(QObject* parent, QString domain, QString ip, short port); + explicit McClient(QObject* parent, QString domain, QString ip, uint16_t port); //! Read status data of the server, and calls the succeeded() signal with the parsed JSON data void getStatusData(); + signals: + void succeeded(QJsonObject data); + void failed(QString error); + void finished(); + + private: + static uint8_t readByte(QByteArray& data); + static int readVarInt(QByteArray& data); + static void writeUInt16(QByteArray& data, uint16_t value); + static void writeString(QByteArray& data, const QString& value); + static void writeVarInt(QByteArray& data, int value); + private: void sendRequest(); //! Accumulate data until we have a full response, then call parseResponse() once void readRawResponse(); void parseResponse(); - - void writeVarInt(QByteArray& data, int value); - int readVarInt(QByteArray& data); - char readByte(QByteArray& data); - //! write number with specified size in big endian format - void writeFixedInt(QByteArray& data, int value, int size); - void writeString(QByteArray& data, const std::string& value); - void writePacketToSocket(QByteArray& data); - void emitFail(QString error); + void emitFail(const QString& error); void emitSucceed(QJsonObject data); - signals: - void succeeded(QJsonObject data); - void failed(QString error); - void finished(); + private: + enum class ResponseReadState : uint8_t { + Waiting, + GotLength, + Finished + }; + + QString m_domain; + QString m_ip; + uint16_t m_port; + QTcpSocket m_socket; + + ResponseReadState m_responseReadState = ResponseReadState::Waiting; + int32_t m_wantedRespLength = 0; + QByteArray m_resp; };