Core/PacketIO: Reduce the number of catch blocks in WorldSession::Update

5 files changed, 56 insertions, 42 deletions

diff --git a/src/server/game/Server/Packets/PacketUtilities.cpp b/src/server/game/Server/Packets/PacketUtilities.cpp
index 4227833e6b0..34fc9923379 100644
--- a/src/server/game/Server/Packets/PacketUtilities.cpp
+++ b/src/server/game/Server/Packets/PacketUtilities.cpp
@@ -20,20 +20,29 @@
 #include "StringFormat.h"
 #include <utf8.h>
 
-WorldPackets::InvalidStringValueException::InvalidStringValueException(std::string_view value) : ByteBufferInvalidValueException("string", value), _value(value)
+WorldPackets::InvalidStringValueException::InvalidStringValueException(char const* type, std::string_view value)
+    : ByteBufferInvalidValueException(type, value), _value(value)
 {
 }
 
-WorldPackets::InvalidUtf8ValueException::InvalidUtf8ValueException(std::string_view value) : InvalidStringValueException(value)
+WorldPackets::InvalidUtf8ValueException::InvalidUtf8ValueException(std::string_view value)
+    : InvalidStringValueException("utf8 string", value)
 {
 }
 
-WorldPackets::InvalidHyperlinkException::InvalidHyperlinkException(std::string_view value) : InvalidStringValueException(value)
+WorldPackets::InvalidHyperlinkException::InvalidHyperlinkException(std::string_view value, Reason reason)
+    : InvalidStringValueException(GetReasonText(reason), value), _reason(reason)
 {
 }
 
-WorldPackets::IllegalHyperlinkException::IllegalHyperlinkException(std::string_view value) : InvalidStringValueException(value)
+char const* WorldPackets::InvalidHyperlinkException::GetReasonText(Reason reason)
 {
+    switch (reason)
+    {
+        case Malformed: return "malformed hyperlink";
+        case NotAllowed: return "not allowed hyperlink";
+        default: return "hyperlink";
+    }
 }
 
 bool WorldPackets::Strings::Utf8::Validate(std::string_view value)
@@ -46,14 +55,14 @@ bool WorldPackets::Strings::Utf8::Validate(std::string_view value)
 bool WorldPackets::Strings::Hyperlinks::Validate(std::string_view value)
 {
     if (!Trinity::Hyperlinks::CheckAllLinks(value))
-        throw InvalidHyperlinkException(value);
+        throw InvalidHyperlinkException(value, InvalidHyperlinkException::Malformed);
     return true;
 }
 
 bool WorldPackets::Strings::NoHyperlinks::Validate(std::string_view value)
 {
     if (value.find('|') != std::string::npos)
-        throw IllegalHyperlinkException(value);
+        throw InvalidHyperlinkException(value, InvalidHyperlinkException::NotAllowed);
     return true;
 }
 
diff --git a/src/server/game/Server/Packets/PacketUtilities.h b/src/server/game/Server/Packets/PacketUtilities.h
index 3170b11b180..055df04cd90 100644
--- a/src/server/game/Server/Packets/PacketUtilities.h
+++ b/src/server/game/Server/Packets/PacketUtilities.h
@@ -30,7 +30,7 @@ namespace WorldPackets
     class InvalidStringValueException : public ByteBufferInvalidValueException
     {
     public:
-        InvalidStringValueException(std::string_view value);
+        explicit InvalidStringValueException(char const* type, std::string_view value);
 
         std::string const& GetInvalidValue() const { return _value; }
 
@@ -41,19 +41,26 @@ namespace WorldPackets
     class InvalidUtf8ValueException : public InvalidStringValueException
     {
     public:
-        InvalidUtf8ValueException(std::string_view value);
+        explicit InvalidUtf8ValueException(std::string_view value);
     };
 
     class InvalidHyperlinkException : public InvalidStringValueException
     {
     public:
-        InvalidHyperlinkException(std::string_view value);
-    };
+        enum Reason : uint8
+        {
+            Malformed,
+            NotAllowed
+        };
 
-    class IllegalHyperlinkException : public InvalidStringValueException
-    {
-    public:
-        IllegalHyperlinkException(std::string_view value);
+        explicit InvalidHyperlinkException(std::string_view value, Reason reason);
+
+        Reason GetReason() const { return _reason; }
+
+    private:
+        static char const* GetReasonText(Reason reason);
+
+        Reason _reason;
     };
 
     namespace Strings
diff --git a/src/server/game/Server/WorldSession.cpp b/src/server/game/Server/WorldSession.cpp
index 99d72cb2b43..0c5898a8dc3 100644
--- a/src/server/game/Server/WorldSession.cpp
+++ b/src/server/game/Server/WorldSession.cpp
@@ -458,29 +458,28 @@ bool WorldSession::Update(uint32 diff, PacketFilter& updater)
         }
         catch (WorldPackets::InvalidHyperlinkException const& ihe)
         {
-            TC_LOG_ERROR("network", "{} sent {} with an invalid link:\n{}", GetPlayerInfo(),
-                GetOpcodeNameForLogging(static_cast<OpcodeClient>(packet->GetOpcode())), ihe.GetInvalidValue());
+            TC_LOG_ERROR("network", "WorldSession::Update ByteBufferException {} occured while parsing a packet (opcode: {}) from {} address {}. Skipped packet.",
+                ihe.what(), GetOpcodeNameForLogging(static_cast<OpcodeClient>(packet->GetOpcode())), GetPlayerInfo(), GetRemoteAddress());
 
             if (sWorld->getIntConfig(CONFIG_CHAT_STRICT_LINK_CHECKING_KICK))
-                KickPlayer("WorldSession::Update Invalid chat link");
-        }
-        catch (WorldPackets::IllegalHyperlinkException const& ihe)
-        {
-            TC_LOG_ERROR("network", "{} sent {} which illegally contained a hyperlink:\n{}", GetPlayerInfo(),
-                GetOpcodeNameForLogging(static_cast<OpcodeClient>(packet->GetOpcode())), ihe.GetInvalidValue());
-
-            if (sWorld->getIntConfig(CONFIG_CHAT_STRICT_LINK_CHECKING_KICK))
-                KickPlayer("WorldSession::Update Illegal chat link");
-        }
-        catch (WorldPackets::PacketArrayMaxCapacityException const& pamce)
-        {
-            TC_LOG_ERROR("network", "PacketArrayMaxCapacityException: {} while parsing {} from {}.",
-                pamce.what(), GetOpcodeNameForLogging(static_cast<OpcodeClient>(packet->GetOpcode())), GetPlayerInfo());
+            {
+                switch (ihe.GetReason())
+                {
+                    case WorldPackets::InvalidHyperlinkException::Malformed:
+                        KickPlayer("WorldSession::Update Invalid chat link");
+                        break;
+                    case WorldPackets::InvalidHyperlinkException::NotAllowed:
+                        KickPlayer("WorldSession::Update Illegal chat link");
+                        break;
+                    default:
+                        break;
+                }
+            }
         }
-        catch (ByteBufferException const&)
+        catch (ByteBufferException const& bbe)
         {
-            TC_LOG_ERROR("network", "WorldSession::Update ByteBufferException occured while parsing a packet (opcode: {}) from client {}, accountid={}. Skipped packet.",
-                GetOpcodeNameForLogging(static_cast<OpcodeClient>(packet->GetOpcode())), GetRemoteAddress(), GetAccountId());
+            TC_LOG_ERROR("network", "WorldSession::Update ByteBufferException {} occured while parsing a packet (opcode: {}) from {} address {}. Skipped packet.",
+                bbe.what(), GetOpcodeNameForLogging(static_cast<OpcodeClient>(packet->GetOpcode())), GetPlayerInfo(), GetRemoteAddress());
             packet->hexlike();
         }
 
@@ -714,16 +713,15 @@ void WorldSession::LogoutPlayer(bool save)
 }
 
 /// Kick a player out of the World
-void WorldSession::KickPlayer(std::string const& reason)
+void WorldSession::KickPlayer(std::string_view reason)
 {
-    TC_LOG_INFO("network.kick", "Account: {} Character: '{}' {} kicked with reason: {}", GetAccountId(), _player ? _player->GetName() : "<none>",
-        _player ? _player->GetGUID().ToString() : "", reason);
+    TC_LOG_INFO("network.kick", "{} kicked with reason: {}", GetPlayerInfo(), reason);
 
-    for (uint8 i = 0; i < 2; ++i)
+    for (std::shared_ptr<WorldSocket> const& socket : m_Socket)
     {
-        if (m_Socket[i])
+        if (socket)
         {
-            m_Socket[i]->CloseSocket();
+            socket->CloseSocket();
             forceExit = true;
         }
     }
diff --git a/src/server/game/Server/WorldSession.h b/src/server/game/Server/WorldSession.h
index 409696dafe4..5134c09af19 100644
--- a/src/server/game/Server/WorldSession.h
+++ b/src/server/game/Server/WorldSession.h
@@ -1047,7 +1047,7 @@ class TC_GAME_API WorldSession
         }
 
         void LogoutPlayer(bool save);
-        void KickPlayer(std::string const& reason);
+        void KickPlayer(std::string_view reason);
         // Returns true if all contained hyperlinks are valid
         // May kick player on false depending on world config (handler should abort)
         bool ValidateHyperlinksAndMaybeKick(std::string const& str);
diff --git a/src/server/shared/Packets/ByteBuffer.cpp b/src/server/shared/Packets/ByteBuffer.cpp
index f5d6a5ab4b5..5712160949f 100644
--- a/src/server/shared/Packets/ByteBuffer.cpp
+++ b/src/server/shared/Packets/ByteBuffer.cpp
@@ -65,7 +65,7 @@ std::string_view ByteBuffer::ReadCString(bool requireValidUtf8 /*= true*/)
     std::string_view value(begin, stringEnd);
     _rpos += value.length() + 1;
     if (requireValidUtf8 && !utf8::is_valid(value.begin(), value.end()))
-        throw ByteBufferInvalidValueException("string", value);
+        throw ByteBufferInvalidValueException("utf8 string", value);
     return value;
 }
 
@@ -81,7 +81,7 @@ std::string_view ByteBuffer::ReadString(uint32 length, bool requireValidUtf8 /*=
     std::string_view value(reinterpret_cast<char const*>(&_storage[_rpos]), length);
     _rpos += length;
     if (requireValidUtf8 && !utf8::is_valid(value.begin(), value.end()))
-        throw ByteBufferInvalidValueException("string", value);
+        throw ByteBufferInvalidValueException("utf8 string", value);
     return value;
 }