From c26319701e8d2502f8bcdf892633dfc5f1b36101 Mon Sep 17 00:00:00 2001 From: Salanto <62221668+Salanto@users.noreply.github.com> Date: Thu, 2 Jun 2022 14:52:50 -0700 Subject: [PATCH] Rework AOPacket (#24) --- core/include/aopacket.h | 100 ++++++++++++++-- core/src/aoclient.cpp | 27 ++--- core/src/aopacket.cpp | 112 +++++++++++++----- core/src/packets.cpp | 25 ++-- core/src/server.cpp | 2 +- tests/tests.pro | 3 +- .../tst_unittest_aopacket.cpp | 86 ++++++++++++++ tests/unittest_aopacket/unittest_aopacket.pro | 5 + 8 files changed, 296 insertions(+), 64 deletions(-) create mode 100644 tests/unittest_aopacket/tst_unittest_aopacket.cpp create mode 100644 tests/unittest_aopacket/unittest_aopacket.pro diff --git a/core/include/aopacket.h b/core/include/aopacket.h index 41f9932..3d0404e 100644 --- a/core/include/aopacket.h +++ b/core/include/aopacket.h @@ -41,35 +41,115 @@ class AOPacket AOPacket(QString p_header, QStringList p_contents); /** - * @brief AOPacket Interprets a string of a full (header + content) packet into an AOPacket. + * @brief Create an AOPacket from an incoming network message. * - * @param packet The string to interpret. + * @param f_packet An escaped string with header and content. */ - AOPacket(QString packet); + AOPacket(QString f_packet); /** - * @brief Returns the string representation of the packet. + * @brief Destructor for the AOPacket + */ + ~AOPacket(){}; + + /** + * @brief Returns the current content of the packet * - * @return See brief description. + * @return The content of the packet. + */ + const QStringList getContent(); + + /** + * @brief Returns the header of the packet. + * + * @return The packets header. + */ + QString getHeader(); + + /** + * @brief Converts the header and content into a single string. + * + * @return String converted packet. */ QString toString(); /** - * @brief Convenience function over AOPacket::toString() + QString::toUtf8(). + * @brief Converts the entire packet, header and content, to a UTF8 formatted ByteArray. * * @return A UTF-8 representation of the packet. */ QByteArray toUtf8(); /** - * @brief The string that indentifies the type of the packet. + * @brief Allows editing of the content inside the packet on a per-field basis. */ - QString header; + void setContentField(int f_content_index, QString f_content_data); /** - * @brief The list of parameters for the packet. Can be empty. + * @brief Escapes the content of the packet using AO2's escape codes. + * + * @see https://github.com/AttorneyOnline/docs/blob/master/AO%20Documentation/docs/development/network.md#escape-codes */ - QStringList contents; + void escapeContent(); + + /** + * @brief Unescapes the content of the packet using AO2's escape codes. + * + * @see https://github.com/AttorneyOnline/docs/blob/master/AO%20Documentation/docs/development/network.md#escape-codes + */ + void unescapeContent(); + + /** + * @brief Due to the way AO's netcode actively fights you, you have to do some specific considerations when escaping evidence. + */ + void escapeEvidence(); + + /** + * @brief Sets the state if a packet has already been escaped or not. + * + * @details This is partially a workaround to make edge case behaviour possible while maintaining a + * mostly unified escape/unescape path. + * + * @param Boolean value of the current state. + * + */ + void setPacketEscaped(bool f_packet_state); + + /** + * @brief Returns if the packet is currently escaped or not. + * + * @details If a packet is escaped, it likely has either just been received by the server or is about to be written + * to a network socket. There should **NEVER** be an instance where an unescaped packet is processed inside the server. + * + * @return If true, the packet is escaped. If false, it is unescaped and plain text. + */ + bool isPacketEscaped(); + + private: + /** + * @brief The header of the packet. + * + * @see https://github.com/AttorneyOnline/docs/blob/master/AO%20Documentation/docs/development/network.md#network-protocol + * for a general explanation on Attorney Online 2's network protocl. + */ + QString m_header; + + /** + * @brief The contents of the packet. + */ + QStringList m_content; + + /** + * @brief Wether the packet is currently escaped or not. If false, the packet is unescaped. + */ + bool m_escaped; + + /** + * @brief According to AO documentation a complete packet is finished using the percent symbol. + * + * @details Note : This is due to AOs inability to determine the packet length, making it read forever otherwise. + */ + const QString packetFinished = "%"; }; #endif // PACKET_MANAGER_H diff --git a/core/src/aoclient.cpp b/core/src/aoclient.cpp index 29103bd..178b4a4 100644 --- a/core/src/aoclient.cpp +++ b/core/src/aoclient.cpp @@ -171,6 +171,10 @@ void AOClient::clientData() QStringList l_all_packets = l_data.split("%"); l_all_packets.removeLast(); // Remove the entry after the last delimiter + if (l_all_packets.value(0).startsWith("MC", Qt::CaseInsensitive)) { + l_all_packets = QStringList{l_all_packets.value(0)}; + } + for (const QString &l_single_packet : qAsConst(l_all_packets)) { AOPacket l_packet(l_single_packet); handlePacket(l_packet); @@ -180,7 +184,7 @@ void AOClient::clientData() void AOClient::clientDisconnected() { #ifdef NET_DEBUG - qDebug() << remote_ip.toString() << "disconnected"; + qDebug() << m_remote_ip.toString() << "disconnected"; #endif if (m_joined) { server->getAreaById(m_current_area)->clientLeftArea(server->getCharID(m_current_char), m_id); @@ -208,12 +212,12 @@ void AOClient::clientDisconnected() void AOClient::handlePacket(AOPacket packet) { #ifdef NET_DEBUG - qDebug() << "Received packet:" << packet.header << ":" << packet.contents << "args length:" << packet.contents.length(); + qDebug() << "Received packet:" << packet.getHeader() << ":" << packet.getContent() << "args length:" << packet.getContent().length(); #endif AreaData *l_area = server->getAreaById(m_current_area); - PacketInfo l_info = packets.value(packet.header, {ACLRole::NONE, 0, &AOClient::pktDefault}); + PacketInfo l_info = packets.value(packet.getHeader(), {ACLRole::NONE, 0, &AOClient::pktDefault}); - if (packet.contents.join("").size() > 16384) { + if (packet.getContent().join("").size() > 16384) { return; } @@ -221,21 +225,21 @@ void AOClient::handlePacket(AOPacket packet) return; } - if (packet.header != "CH") { + if (packet.getHeader() != "CH") { if (m_is_afk) sendServerMessage("You are no longer AFK."); m_is_afk = false; m_afk_timer->start(ConfigManager::afkTimeout() * 1000); } - if (packet.contents.length() < l_info.minArgs) { + if (packet.getContent().length() < l_info.minArgs) { #ifdef NET_DEBUG - qDebug() << "Invalid packet args length. Minimum is" << info.minArgs << "but only" << packet.contents.length() << "were given."; + qDebug() << "Invalid packet args length. Minimum is" << l_info.minArgs << "but only" << packet.getContent().length() << "were given."; #endif return; } - (this->*(l_info.action))(l_area, packet.contents.length(), packet.contents, packet); + (this->*(l_info.action))(l_area, packet.getContent().length(), packet.getContent(), packet); } void AOClient::changeArea(int new_area) @@ -430,13 +434,8 @@ void AOClient::fullArup() void AOClient::sendPacket(AOPacket packet) { #ifdef NET_DEBUG - qDebug() << "Sent packet:" << packet.header << ":" << packet.contents; + qDebug() << "Sent packet:" << packet.getHeader() << ":" << packet.getContent(); #endif - packet.contents.replaceInStrings("#", "") - .replaceInStrings("%", "") - .replaceInStrings("$", ""); - if (packet.header != "LE") - packet.contents.replaceInStrings("&", ""); m_socket->write(packet.toUtf8()); m_socket->flush(); } diff --git a/core/src/aopacket.cpp b/core/src/aopacket.cpp index 9eb703b..25c05da 100644 --- a/core/src/aopacket.cpp +++ b/core/src/aopacket.cpp @@ -17,47 +17,103 @@ ////////////////////////////////////////////////////////////////////////////////////// #include "include/aopacket.h" -AOPacket::AOPacket(QString p_header, QStringList p_contents) +AOPacket::AOPacket(QString p_header, QStringList p_contents) : + m_header(p_header), + m_content(p_contents), + m_escaped(false) { - header = p_header; - contents = p_contents; } -AOPacket::AOPacket(QString p_packet) +AOPacket::AOPacket(QString f_packet) { - if (p_packet.isEmpty()) + QString l_packet = f_packet; + if (l_packet.isEmpty() || l_packet.at(0) == '#' || l_packet.contains("%")) { +#if NET_DEBUG + qDebug() << "Invalid or fantacrypt packet received."; +#endif + m_header = "Unknown"; + m_content = QStringList{"Unknown"}; return; + } - QStringList packet_contents = p_packet.split("#"); - if (p_packet.at(0) == '#') { - // The header is encrypted with FantaCrypt - // This should never happen with AO2 2.4.3 or newer - qDebug() << "FantaCrypt packet received"; - header = "Unknown"; - packet_contents.append("Unknown"); - return; - } - else { - header = packet_contents[0]; - } - packet_contents.removeFirst(); // Remove header - packet_contents.removeLast(); // Remove anything trailing after delimiter - contents = packet_contents; + QStringList l_split_packet = l_packet.split("#"); + m_header = l_split_packet.value(0); + + // Remove header and trailing packetFinished + l_split_packet.removeFirst(); + l_split_packet.removeLast(); + m_content = l_split_packet; + + // All incoming data has to be escaped after being split. + this->unescapeContent(); +} + +const QStringList AOPacket::getContent() +{ + return m_content; +} + +QString AOPacket::getHeader() +{ + return m_header; } QString AOPacket::toString() { - QString ao_packet = header; - for (int i = 0; i < contents.length(); i++) { - ao_packet += "#" + contents[i]; + if (!isPacketEscaped() && !(m_header == "LE")) { + // We will never send unescaped data to a client, unless its evidence. + this->escapeContent(); } - ao_packet += "#%"; - - return ao_packet; + else { + // Of course AO has SOME expection to the rule. + this->escapeEvidence(); + } + return QString("%1#%2#%3").arg(m_header, m_content.join("#"), packetFinished); } QByteArray AOPacket::toUtf8() { - QString packet_string = toString(); - return packet_string.toUtf8(); + QString l_packet = this->toString(); + return l_packet.toUtf8(); +} + +void AOPacket::setContentField(int f_content_index, QString f_content_data) +{ + m_content[f_content_index] = f_content_data; +} + +void AOPacket::escapeContent() +{ + m_content.replaceInStrings("#", "") + .replaceInStrings("%", "") + .replaceInStrings("$", "") + .replaceInStrings("&", ""); + this->setPacketEscaped(true); +} + +void AOPacket::unescapeContent() +{ + m_content.replaceInStrings("", "#") + .replaceInStrings("", "%") + .replaceInStrings("", "$") + .replaceInStrings("", "&"); + this->setPacketEscaped(false); +} + +void AOPacket::escapeEvidence() +{ + m_content.replaceInStrings("#", "") + .replaceInStrings("%", "") + .replaceInStrings("$", ""); + this->setPacketEscaped(true); +} + +void AOPacket::setPacketEscaped(bool f_packet_state) +{ + m_escaped = f_packet_state; +} + +bool AOPacket::isPacketEscaped() +{ + return m_escaped; } diff --git a/core/src/packets.cpp b/core/src/packets.cpp index 8f63d64..dcdcb81 100644 --- a/core/src/packets.cpp +++ b/core/src/packets.cpp @@ -33,7 +33,7 @@ void AOClient::pktDefault(AreaData *area, int argc, QStringList argv, AOPacket p Q_UNUSED(argc); Q_UNUSED(argv); #ifdef NET_DEBUG - qDebug() << "Unimplemented packet:" << packet.header << packet.contents; + qDebug() << "Unimplemented packet:" << packet.getHeader() << packet.getContent(); #else Q_UNUSED(packet); #endif @@ -104,7 +104,7 @@ void AOClient::pktSoftwareId(AreaData *area, int argc, QStringList argv, AOPacke if (m_version.release != 2) { // No valid ID packet resolution. - sendPacket(AOPacket("BD", {"A protocol error has been encountered. Packet : ID"})); + sendPacket(AOPacket("BD", {"A protocol error has been encountered. Packet : ID\nMajor version not recognised."})); m_socket->close(); return; } @@ -224,6 +224,11 @@ void AOClient::pktSelectChar(AreaData *area, int argc, QStringList argv, AOPacke l_selected_char_id = SPECTATOR_ID; } + if (l_selected_char_id < -1 || l_selected_char_id > server->getCharacters().size() - 1) { + sendPacket(AOPacket("KK", {"A protocol error has been encountered.Packet : CC\nCharacter ID out of range."})); + m_socket->close(); + } + if (changeCharacter(l_selected_char_id)) m_char_id = l_selected_char_id; @@ -247,15 +252,15 @@ void AOClient::pktIcChat(AreaData *area, int argc, QStringList argv, AOPacket pa } AOPacket validated_packet = validateIcPacket(packet); - if (validated_packet.header == "INVALID") + if (validated_packet.getHeader() == "INVALID") return; if (m_pos != "") - validated_packet.contents[5] = m_pos; + validated_packet.setContentField(5, m_pos); server->broadcast(validated_packet, m_current_area); emit logIC((m_current_char + " " + m_showname), m_ooc_name, m_ipid, server->getAreaById(m_current_area)->name(), m_last_message); - area->updateLastICMessage(validated_packet.contents); + area->updateLastICMessage(validated_packet.getContent()); area->startMessageFloodguard(ConfigManager::messageFloodguard()); server->startMessageFloodguard(ConfigManager::globalMessageFloodguard()); @@ -502,8 +507,8 @@ void AOClient::pktModCall(AreaData *area, int argc, QStringList argv, AOPacket p QString l_modcallNotice = "!!!MODCALL!!!\nArea: " + l_areaName + "\nCaller: " + l_name + "\n"; - if (!packet.contents[0].isEmpty()) - l_modcallNotice.append("Reason: " + packet.contents[0]); + if (!packet.getContent()[0].isEmpty()) + l_modcallNotice.append("Reason: " + packet.getContent()[0]); else l_modcallNotice.append("No reason given."); @@ -520,7 +525,7 @@ void AOClient::pktModCall(AreaData *area, int argc, QStringList argv, AOPacket p l_name = m_current_char; QString l_areaName = area->name(); - emit server->modcallWebhookRequest(l_name, l_areaName, packet.contents[0], server->getAreaBuffer(l_areaName)); + emit server->modcallWebhookRequest(l_name, l_areaName, packet.getContent().value(0), server->getAreaBuffer(l_areaName)); } } @@ -694,7 +699,7 @@ AOPacket AOClient::validateIcPacket(AOPacket packet) return l_invalid; QList l_incoming_args; - for (const QString &l_arg : qAsConst(packet.contents)) { + for (const QString &l_arg : packet.getContent()) { l_incoming_args.append(QVariant(l_arg)); } @@ -882,7 +887,7 @@ AOPacket AOClient::validateIcPacket(AOPacket packet) QString l_other_offset = "0"; QString l_other_flip = "0"; for (int l_client_id : area->joinedIDs()) { - AOClient* l_client = server->getClientByID(l_client_id); + AOClient *l_client = server->getClientByID(l_client_id); if (l_client->m_pairing_with == m_char_id && l_other_charid != m_char_id && l_client->m_char_id == m_pairing_with && l_client->m_pos == m_pos) { l_other_name = l_client->m_current_iniswap; l_other_emote = l_client->m_emote; diff --git a/core/src/server.cpp b/core/src/server.cpp index 6a446f6..a1358e1 100644 --- a/core/src/server.cpp +++ b/core/src/server.cpp @@ -221,7 +221,7 @@ void Server::clientConnected() client->sendPacket(decryptor); hookupAOClient(client); #ifdef NET_DEBUG - qDebug() << client->remote_ip.toString() << "connected"; + qDebug() << client->m_remote_ip.toString() << "connected"; #endif } diff --git a/tests/tests.pro b/tests/tests.pro index 16a13f3..c7132ca 100644 --- a/tests/tests.pro +++ b/tests/tests.pro @@ -4,4 +4,5 @@ SUBDIRS += \ unittest_area \ unittest_music_manager \ unittest_acl_roles_handler \ - unittest_command_extension + unittest_command_extension \ + unittest_aopacket \ No newline at end of file diff --git a/tests/unittest_aopacket/tst_unittest_aopacket.cpp b/tests/unittest_aopacket/tst_unittest_aopacket.cpp new file mode 100644 index 0000000..018b982 --- /dev/null +++ b/tests/unittest_aopacket/tst_unittest_aopacket.cpp @@ -0,0 +1,86 @@ +#include +#include + +#include "include/aopacket.h" + +namespace tests { +namespace unittests { + +/** + * @brief Unit Tester class for the area-related functions. + */ +class Packet : public QObject +{ + Q_OBJECT + + public: + AOPacket m_packet = AOPacket{"", {}}; + + private slots: + /** + * @brief Creates a packet from a defined header and content. + */ + void createPacket(); + + /** + * @brief The data function for createPacketFromString(); + */ + void createPacketFromString_data(); + + /** + * @brief Tests the creation of AOPackets from incoming string formatted packets. + */ + void createPacketFromString(); +}; + +void Packet::createPacket() +{ + AOPacket packet = AOPacket("HI", {"HDID"}); + QCOMPARE(packet.getHeader(), "HI"); + QCOMPARE(packet.getContent(), {"HDID"}); +} + +void Packet::createPacketFromString_data() +{ + QTest::addColumn("incoming_packet"); + QTest::addColumn("expected_header"); + QTest::addColumn("expected_content"); + + QTest::newRow("No Escaped fields") << "HI#1234#" + << "HI" + << QStringList{"1234"}; + + QTest::newRow("Multiple fields") << "ID#34#Akashi#" + << "ID" + << QStringList{"34", "Akashi"}; + + QTest::newRow("Encoded fields") << "MC#[TT]Objection.opus#0#oldmud0#-1#0#0#" + << "MC" + << QStringList{"[T&T]Objection.opus", "0", "oldmud0", "-1", "0", "0"}; + + QTest::newRow("Sequence of encoded characters") << "UNIT##" + << "UNIT" + << QStringList{"&&%#%$"}; + + QTest::newRow("Unescaped characters") << "MC#20% Cooler#" + << "Unknown" + << QStringList{"Unknown"}; // This should be impossible. +} + +void Packet::createPacketFromString() +{ + QFETCH(QString, incoming_packet); + QFETCH(QString, expected_header); + QFETCH(QStringList, expected_content); + + AOPacket packet = AOPacket(incoming_packet); + QCOMPARE(packet.getHeader(), expected_header); + QCOMPARE(packet.getContent(), expected_content); +} + +} +} + +QTEST_APPLESS_MAIN(tests::unittests::Packet) + +#include "tst_unittest_aopacket.moc" diff --git a/tests/unittest_aopacket/unittest_aopacket.pro b/tests/unittest_aopacket/unittest_aopacket.pro new file mode 100644 index 0000000..d8f3ba0 --- /dev/null +++ b/tests/unittest_aopacket/unittest_aopacket.pro @@ -0,0 +1,5 @@ +QT -= gui + +include(../tests_common.pri) + +SOURCES += tst_unittest_aopacket.cpp