Core/Chat: Some hyperlink follow-ups:

* Ignore messages containing invalid links again instead of trying to sanitize them. Closes #22451.
* No longer filter messages on the addon channel. Closes #22450.

7 files changed, 59 insertions, 95 deletions

diff --git a/src/server/game/Chat/Hyperlinks.cpp b/src/server/game/Chat/Hyperlinks.cpp
index b2cc0aa9ff9..9822254f3fd 100644
--- a/src/server/game/Chat/Hyperlinks.cpp
+++ b/src/server/game/Chat/Hyperlinks.cpp
@@ -341,77 +341,46 @@ static bool ValidateLinkInfo(HyperlinkInfo const& info)
 }
 
 // Validates all hyperlinks and control sequences contained in str
-bool Trinity::Hyperlinks::ValidateLinks(std::string& str)
+bool Trinity::Hyperlinks::CheckAllLinks(std::string const& str)
 {
-    bool allValid = true;
-    std::string::size_type pos = std::string::npos;
-    // Step 1: Strip all control sequences except ||, |H, |h, |c and |r
-    do
+    // Step 1: Disallow all control sequences except ||, |H, |h, |c and |r
     {
-        if ((pos = str.rfind('|', pos)) == std::string::npos)
-            break;
-        if (pos && str[pos - 1] == '|')
+        std::string::size_type pos = 0;
+        while ((pos = str.find('|', pos)) != std::string::npos)
         {
-            --pos;
-            continue;
+            char next = str[pos + 1];
+            if (next == 'H' || next == 'h' || next == 'c' || next == 'r' || next == '|')
+                pos += 2;
+            else
+                return false;
         }
-        char next = str[pos + 1];
-        if (next == 'H' || next == 'h' || next == 'c' || next == 'r')
-            continue;
-
-        allValid = false;
-        str.erase(pos, 2);
-    } while (pos--);
-
+    }
+    
     // Step 2: Parse all link sequences
     // They look like this: |c<color>|H<linktag>:<linkdata>|h[<linktext>]|h|r
     // - <color> is 8 hex characters AARRGGBB
     // - <linktag> is arbitrary length [a-z_]
     // - <linkdata> is arbitrary length, no | contained
     // - <linktext> is printable
-    pos = 0;
-    while (pos < str.size() && (pos = str.find('|', pos)) != std::string::npos)
     {
-        if (str[pos + 1] == '|') // this is an escaped pipe character (||)
+        std::string::size_type pos = 0;
+        while ((pos = str.find('|', pos)) != std::string::npos)
         {
-            pos += 2;
-            continue;
-        }
-
-        HyperlinkInfo info = ParseHyperlink(str.c_str() + pos);
-        if (!info)
-        {   // cannot be parsed at all, so we'll need to cut it out entirely
-            // find the next start of a link
-            std::string::size_type next = str.find("|c", pos + 1);
-            // then backtrack to the previous return control sequence
-            std::string::size_type end = str.rfind("|r", next);
-            if (end == std::string::npos || end <= pos) // there is no potential end tag, remove everything after pos (up to next, if available)
+            if (str[pos + 1] == '|') // this is an escaped pipe character (||)
             {
-                if (next == std::string::npos)
-                    str.erase(pos);
-                else
-                    str.erase(pos, next - pos);
+                pos += 2;
+                continue;
             }
-            else
-                str.erase(pos, end - pos + 2);
 
-            allValid = false;
-            continue;
-        }
+            HyperlinkInfo info = ParseHyperlink(str.c_str() + pos);
+            if (!info || !ValidateLinkInfo(info))
+                return false;
 
-        // ok, link parsed successfully - now validate it based on the tag
-        if (!ValidateLinkInfo(info))
-        {
-            // invalid link info, replace with just text
-            str.replace(pos, (info.next - str.c_str()) - pos, str, info.text.first - str.c_str(), info.text.second);
-            allValid = false;
-            continue;
+            // tag is fine, find the next one
+            pos = info.next - str.c_str();
         }
-
-        // tag is fine, find the next one
-        pos = info.next - str.c_str();
     }
 
-    // all tags validated
-    return allValid;
+    // all tags are valid
+    return true;
 }
diff --git a/src/server/game/Chat/Hyperlinks.h b/src/server/game/Chat/Hyperlinks.h
index 5808cbe9447..8b078688272 100644
--- a/src/server/game/Chat/Hyperlinks.h
+++ b/src/server/game/Chat/Hyperlinks.h
@@ -234,7 +234,7 @@ struct HyperlinkInfo
     std::pair<char const*, size_t> const text;
 };
 HyperlinkInfo TC_GAME_API ParseHyperlink(char const* pos);
-bool TC_GAME_API ValidateLinks(std::string&);
+bool TC_GAME_API CheckAllLinks(std::string const&);
 
 }
 }
diff --git a/src/server/game/Handlers/ChatHandler.cpp b/src/server/game/Handlers/ChatHandler.cpp
index ff0b0c2214c..bab3afae48f 100644
--- a/src/server/game/Handlers/ChatHandler.cpp
+++ b/src/server/game/Handlers/ChatHandler.cpp
@@ -30,7 +30,6 @@
 #include "Group.h"
 #include "Guild.h"
 #include "GuildMgr.h"
-#include "Hyperlinks.h"
 #include "Language.h"
 #include "Log.h"
 #include "ObjectAccessor.h"
@@ -240,6 +239,9 @@ void WorldSession::HandleMessagechatOpcode(WorldPacket& recvData)
             break;
     }
 
+    if (msg.size() > 255)
+        return;
+
     // Strip invisible characters for non-addon messages
     if (sWorld->getBoolConfig(CONFIG_CHAT_FAKE_MESSAGE_PREVENTING) && lang != LANG_ADDON)
         StripInvisibleChars(msg);
@@ -261,20 +263,7 @@ void WorldSession::HandleMessagechatOpcode(WorldPacket& recvData)
         }
     }
 
-    bool validMessage = Trinity::Hyperlinks::ValidateLinks(msg);
-    if (!validMessage)
-    {
-        TC_LOG_ERROR("network", "Player %s (GUID: %u) sent a chatmessage with an invalid link - corrected", GetPlayer()->GetName().c_str(),
-            GetPlayer()->GetGUID().GetCounter());
-
-        if (sWorld->getIntConfig(CONFIG_CHAT_STRICT_LINK_CHECKING_KICK))
-        {
-            KickPlayer();
-            return;
-        }
-    }
-
-    if (msg.length() > 255)
+    if ((lang != LANG_ADDON) && !ValidateHyperlinksAndMaybeKick(msg))
         return;
 
     switch (type)
diff --git a/src/server/game/Handlers/TicketHandler.cpp b/src/server/game/Handlers/TicketHandler.cpp
index 426795cd7f0..b7c19c31cbe 100644
--- a/src/server/game/Handlers/TicketHandler.cpp
+++ b/src/server/game/Handlers/TicketHandler.cpp
@@ -20,7 +20,6 @@
 #include "Common.h"
 #include "DatabaseEnv.h"
 #include "GameTime.h"
-#include "Hyperlinks.h"
 #include "Language.h"
 #include "Log.h"
 #include "ObjectMgr.h"
@@ -33,21 +32,6 @@
 #include "WorldPacket.h"
 #include <zlib.h>
 
-#define ValidateLinksAndMaybeKick(str)                                                                              \
-{                                                                                                                   \
-    if (!Trinity::Hyperlinks::ValidateLinks(str))                                                                   \
-    {                                                                                                               \
-        TC_LOG_ERROR("network", "Player %s (GUID: %u) tried to add an invalid link to a GM ticket - corrected",     \
-            GetPlayer()->GetName().c_str(), GetPlayer()->GetGUID().GetCounter());                                   \
-                                                                                                                    \
-        if (sWorld->getIntConfig(CONFIG_CHAT_STRICT_LINK_CHECKING_KICK))                                            \
-        {                                                                                                           \
-            KickPlayer();                                                                                           \
-            return;                                                                                                 \
-        }                                                                                                           \
-    }                                                                                                               \
-}
-
 void WorldSession::HandleGMTicketCreateOpcode(WorldPacket& recvData)
 {
     // Don't accept tickets if the ticket queue is disabled. (Ticket UI is greyed out but not fully dependable)
@@ -83,7 +67,8 @@ void WorldSession::HandleGMTicketCreateOpcode(WorldPacket& recvData)
         recvData >> x >> y >> z;
         recvData >> message;
 
-        ValidateLinksAndMaybeKick(message);
+        if (!ValidateHyperlinksAndMaybeKick(message))
+            return;
 
         recvData >> needResponse;
         recvData >> needMoreHelp;
@@ -120,16 +105,15 @@ void WorldSession::HandleGMTicketCreateOpcode(WorldPacket& recvData)
             recvData.rfinish(); // Will still have compressed data in buffer.
         }
 
+        if (!chatLog.empty() && !ValidateHyperlinksAndMaybeKick(chatLog))
+            return;
+
         ticket = new GmTicket(GetPlayer());
         ticket->SetPosition(mapId, x, y, z);
         ticket->SetMessage(message);
         ticket->SetGmAction(needResponse, needMoreHelp);
-
         if (!chatLog.empty())
-        {
-            ValidateLinksAndMaybeKick(chatLog);
             ticket->SetChatLog(times, chatLog);
-        }
 
         sTicketMgr->AddTicket(ticket);
         sTicketMgr->UpdateLastChange();
@@ -149,7 +133,8 @@ void WorldSession::HandleGMTicketUpdateOpcode(WorldPacket& recvData)
     std::string message;
     recvData >> message;
 
-    ValidateLinksAndMaybeKick(message);
+    if (!ValidateHyperlinksAndMaybeKick(message))
+        return;
 
     GMTicketResponse response = GMTICKET_RESPONSE_UPDATE_ERROR;
     if (GmTicket* ticket = sTicketMgr->GetTicketByPlayer(GetPlayer()->GetGUID()))
@@ -233,7 +218,8 @@ void WorldSession::HandleGMSurveySubmit(WorldPacket& recvData)
         if (!surveyIds.insert(subSurveyId).second)
             continue;
 
-        ValidateLinksAndMaybeKick(comment);
+        if (!ValidateHyperlinksAndMaybeKick(comment))
+            return;
 
         PreparedStatement* stmt = CharacterDatabase.GetPreparedStatement(CHAR_INS_GM_SUBSURVEY);
         stmt->setUInt32(0, nextSurveyID);
@@ -246,7 +232,8 @@ void WorldSession::HandleGMSurveySubmit(WorldPacket& recvData)
     std::string comment; // just a guess
     recvData >> comment;
 
-    ValidateLinksAndMaybeKick(comment);
+    if (!ValidateHyperlinksAndMaybeKick(comment))
+        return;
 
     PreparedStatement* stmt = CharacterDatabase.GetPreparedStatement(CHAR_INS_GM_SURVEY);
     stmt->setUInt32(0, GetPlayer()->GetGUID().GetCounter());
diff --git a/src/server/game/Server/WorldSession.cpp b/src/server/game/Server/WorldSession.cpp
index 9b2ce6fe4a9..e49ee044dd8 100644
--- a/src/server/game/Server/WorldSession.cpp
+++ b/src/server/game/Server/WorldSession.cpp
@@ -32,6 +32,7 @@
 #include "Group.h"
 #include "Guild.h"
 #include "GuildMgr.h"
+#include "Hyperlinks.h"
 #include "Log.h"
 #include "Map.h"
 #include "Metric.h"
@@ -591,6 +592,20 @@ void WorldSession::KickPlayer()
     }
 }
 
+bool WorldSession::ValidateHyperlinksAndMaybeKick(std::string const& str)
+{
+    if (Trinity::Hyperlinks::CheckAllLinks(str))
+        return true;
+
+    TC_LOG_ERROR("network", "Player %s (GUID: %u) sent a message with an invalid link:\n%s", GetPlayer()->GetName().c_str(),
+        GetPlayer()->GetGUID().GetCounter(), str.c_str());
+
+    if (sWorld->getIntConfig(CONFIG_CHAT_STRICT_LINK_CHECKING_KICK))
+        KickPlayer();
+
+    return false;
+}
+
 void WorldSession::SendNotification(const char *format, ...)
 {
     if (format)
diff --git a/src/server/game/Server/WorldSession.h b/src/server/game/Server/WorldSession.h
index 03e80c7e69e..540bc239128 100644
--- a/src/server/game/Server/WorldSession.h
+++ b/src/server/game/Server/WorldSession.h
@@ -30,6 +30,7 @@
 #include "Packet.h"
 #include "QueryCallbackProcessor.h"
 #include "SharedDefines.h"
+#include <string>
 #include <unordered_map>
 
 class BigNumber;
@@ -335,6 +336,9 @@ class TC_GAME_API WorldSession
 
         void LogoutPlayer(bool save);
         void KickPlayer();
+        // 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);
 
         void QueuePacket(WorldPacket* new_packet);
         bool Update(uint32 diff, PacketFilter& updater);
diff --git a/src/server/worldserver/worldserver.conf.dist b/src/server/worldserver/worldserver.conf.dist
index b5f6aa6aa16..5463affdb5b 100644
--- a/src/server/worldserver/worldserver.conf.dist
+++ b/src/server/worldserver/worldserver.conf.dist
@@ -1898,7 +1898,7 @@ ChatStrictLinkChecking.Severity = 0
 #    ChatStrictLinkChecking.Kick
 #        Description: Defines what should be done if a message containing invalid control characters
 #                     is received.
-#        Default:     0 - (Process sanitized message normally)
+#        Default:     0 - (Silently ignore message)
 #                     1 - (Ignore message and kick player)
 
 ChatStrictLinkChecking.Kick = 0