Remove obsolete account data completion response

src/server/game/Handlers/MiscHandler.cpp

@@ -1635,35 +1635,37 @@ void WorldSession::HandleUpdateAccountData(WorldPacket& recvData)
     SF_LOG_DEBUG("network", "WORLD: Received CMSG_UPDATE_ACCOUNT_DATA");
 
     uint32 timestamp = 0, decompressedSize = 0, compCount = 0;
-    uint8 type = 0;
     recvData >> decompressedSize >> timestamp >> compCount;
 
-    std::size_t const typePosition = recvData.rpos() + compCount;
-    if (typePosition >= recvData.size())
+    auto readAccountDataType = [&recvData](AccountDataType& dataType) -> bool
     {
-        recvData.rfinish();
-        SF_LOG_DEBUG("network", "UAD: Account data packet missing type bits");
-        return;
-    }
+        if (recvData.rpos() >= recvData.size())
+        {
+            recvData.rfinish();
+            SF_LOG_DEBUG("network", "UAD: Account data packet missing type bits");
+            return false;
+        }
 
-    type = recvData[typePosition] >> 5;
-    AccountDataType UADType = AccountDataType(type);
-    if (UADType >= AccountDataType::NUM_ACCOUNT_DATA_TYPES)
-    {
-        recvData.rfinish();
-        SF_LOG_DEBUG("network", "UAD: Unknown account data type: %u", type);
-        return;
-    }
+        uint8 const type = uint8(recvData.ReadBits(3));
+        dataType = AccountDataType(type);
+        if (dataType >= AccountDataType::NUM_ACCOUNT_DATA_TYPES)
+        {
+            recvData.rfinish();
+            SF_LOG_DEBUG("network", "UAD: Unknown account data type: %u", type);
+            return false;
+        }
+
+        return true;
+    };
+
+    AccountDataType UADType = AccountDataType::NUM_ACCOUNT_DATA_TYPES;
 
     if (decompressedSize == 0)                               // erase
     {
+        if (!readAccountDataType(UADType))
+            return;
+
         SetAccountData(UADType, 0, "");
-
-        WorldPacket data(SMSG_UPDATE_ACCOUNT_DATA_COMPLETE, 4 + 4);
-        data << uint32(type);
-        data << uint32(0);
-        SendPacket(&data);
-
         return;
     }
 
@@ -1674,6 +1676,13 @@ void WorldSession::HandleUpdateAccountData(WorldPacket& recvData)
         return;
     }
 
+    if (compCount > recvData.size() - recvData.rpos())
+    {
+        recvData.rfinish();
+        SF_LOG_DEBUG("network", "UAD: Account data packet too short, compressed size %u", compCount);
+        return;
+    }
+
     ByteBuffer dest;
     dest.resize(decompressedSize);
 
@@ -1685,19 +1694,14 @@ void WorldSession::HandleUpdateAccountData(WorldPacket& recvData)
         return;
     }
 
-    recvData.rpos(recvData.rpos() + compCount);
+    std::string adata = dest.ReadString(decompressedSize);
 
-    recvData.ReadBits(3);
+    recvData.read_skip(compCount);
 
-    std::string adata;
-    dest >> adata;
+    if (!readAccountDataType(UADType))
+        return;
 
     SetAccountData(UADType, timestamp, adata);
-
-    WorldPacket data(SMSG_UPDATE_ACCOUNT_DATA_COMPLETE, 4 + 4);
-    data << uint32(UADType);
-    data << uint32(0);
-    SendPacket(&data);
 }
 
 void WorldSession::HandleRequestAccountData(WorldPacket& recvData)

src/server/game/Server/Protocol/Opcodes.cpp

@@ -1083,7 +1083,6 @@ void OpcodeTable::InitializeServerTable()
     DEFINE_OPCODE_HANDLER(SMSG_TUTORIAL_FLAGS,                                 0x1B90, STATUS_NEVER    ); // 5.4.8 18414
     DEFINE_OPCODE_HANDLER(SMSG_UI_TIME,                                        0x0027, STATUS_NEVER    ); // 5.4.8 18414
     DEFINE_OPCODE_HANDLER(SMSG_UPDATE_ACCOUNT_DATA,                            0x0AAE, STATUS_NEVER    ); // 5.4.8 18414
-    DEFINE_OPCODE_HANDLER(SMSG_UPDATE_ACCOUNT_DATA_COMPLETE,                   0x0000, STATUS_UNHANDLED); // 5.4.8 18414
     DEFINE_OPCODE_HANDLER(SMSG_UPDATE_ACTION_BUTTONS,                          0x081A, STATUS_NEVER    ); // 5.4.8 18414
     DEFINE_OPCODE_HANDLER(SMSG_UPDATE_COMBO_POINTS,                            0x082F, STATUS_NEVER    ); // 5.4.8 18414
     DEFINE_OPCODE_HANDLER(SMSG_UPDATE_CURRENCY,                                0x129E, STATUS_NEVER    ); // 5.4.8 18414

src/server/game/Server/Protocol/Opcodes.h

@@ -1051,7 +1051,6 @@ enum Opcodes
     SMSG_TUTORIAL_FLAGS,
     SMSG_UI_TIME,
     SMSG_UPDATE_ACCOUNT_DATA,
-    SMSG_UPDATE_ACCOUNT_DATA_COMPLETE,
     SMSG_UPDATE_ACTION_BUTTONS,
     SMSG_UPDATE_COMBO_POINTS,
     SMSG_UPDATE_CURRENCY,

src/tests/AccountDataUpdateHandlerTests.cpp

@@ -0,0 +1,96 @@
+/*
+* This file is part of Project SkyFire https://www.projectskyfire.org.
+* See LICENSE.md file for Copyright information
+*/
+
+#include <cstdint>
+#include <fstream>
+#include <iostream>
+#include <sstream>
+#include <string>
+
+namespace
+{
+    bool Expect(bool condition, char const* message)
+    {
+        if (!condition)
+            std::cerr << message << '\n';
+
+        return condition;
+    }
+
+    std::string ReadFile(std::string const& path)
+    {
+        std::ifstream input(path.c_str());
+        if (!input)
+        {
+            std::cerr << "Could not open file: " << path << '\n';
+            return "";
+        }
+
+        std::ostringstream buffer;
+        buffer << input.rdbuf();
+        return buffer.str();
+    }
+
+    std::string ExtractFunction(std::string const& source, char const* signature)
+    {
+        size_t const start = source.find(signature);
+        if (start == std::string::npos)
+            return "";
+
+        size_t const openBrace = source.find('{', start);
+        if (openBrace == std::string::npos)
+            return "";
+
+        uint32_t depth = 0;
+        for (size_t i = openBrace; i < source.size(); ++i)
+        {
+            if (source[i] == '{')
+                ++depth;
+            else if (source[i] == '}')
+            {
+                if (--depth == 0)
+                    return source.substr(start, i - start + 1);
+            }
+        }
+
+        return "";
+    }
+
+    bool Contains(std::string const& haystack, char const* needle)
+    {
+        return haystack.find(needle) != std::string::npos;
+    }
+}
+
+int main()
+{
+    std::string const miscHandler =
+        ReadFile(std::string(SKYFIRE_SOURCE_DIR) + "/src/server/game/Handlers/MiscHandler.cpp");
+    std::string const handleUpdateAccountData =
+        ExtractFunction(miscHandler, "void WorldSession::HandleUpdateAccountData(WorldPacket& recvData)");
+    std::string const opcodes =
+        ReadFile(std::string(SKYFIRE_SOURCE_DIR) + "/src/server/game/Server/Protocol/Opcodes.cpp");
+    std::string const opcodeHeader =
+        ReadFile(std::string(SKYFIRE_SOURCE_DIR) + "/src/server/game/Server/Protocol/Opcodes.h");
+
+    bool passed = true;
+    passed &= Expect(!handleUpdateAccountData.empty(),
+        "HandleUpdateAccountData should exist in MiscHandler.cpp.");
+    passed &= Expect(!Contains(handleUpdateAccountData, "SMSG_UPDATE_ACCOUNT_DATA_COMPLETE"),
+        "HandleUpdateAccountData should not send an account data completion response.");
+    passed &= Expect(!Contains(opcodes, "DEFINE_OPCODE_HANDLER(SMSG_UPDATE_ACCOUNT_DATA_COMPLETE"),
+        "The unused account data completion response should not be registered in the opcode table.");
+    passed &= Expect(!Contains(opcodeHeader, "SMSG_UPDATE_ACCOUNT_DATA_COMPLETE"),
+        "The unused account data completion response should not remain in the server opcode enum.");
+    passed &= Expect(Contains(handleUpdateAccountData, "recvData.read_skip(compCount)") &&
+        Contains(handleUpdateAccountData, "recvData.ReadBits(3)"),
+        "HandleUpdateAccountData should read the account data type after the compressed payload.");
+    passed &= Expect(Contains(handleUpdateAccountData, "SetAccountData(UADType, timestamp, adata)"),
+        "HandleUpdateAccountData should still persist non-empty account data updates.");
+    passed &= Expect(Contains(handleUpdateAccountData, "SetAccountData(UADType, 0, \"\")"),
+        "HandleUpdateAccountData should still erase empty account data updates.");
+
+    return passed ? 0 : 1;
+}

src/tests/CMakeLists.txt

@@ -150,6 +150,14 @@ target_include_directories(account_data_utils_tests PRIVATE
 
 add_test(NAME account_data_utils_tests COMMAND account_data_utils_tests)
 
+add_executable(account_data_update_handler_tests
+  AccountDataUpdateHandlerTests.cpp
+)
+
+target_compile_definitions(account_data_update_handler_tests PRIVATE SKYFIRE_SOURCE_DIR=\"${CMAKE_SOURCE_DIR}\")
+
+add_test(NAME account_data_update_handler_tests COMMAND account_data_update_handler_tests)
+
 add_executable(world_auth_identity_tests
   WorldAuthIdentityTests.cpp
 )