Add smart chat resend/clear detection and intercept <actionbar> & <sound> tags by castledking · Pull Request #25 · ItsMyStudio/ItsMyConfig

castledking · 2025-12-13T20:13:32Z

Summary

Adds intelligent detection of chat clear/resend operations to prevent actionbar and sound tags from firing during chat floods.

Changes

ChatResendDetector — New utility class that detects:
- Rapid packet bursts (6+ packets in 100ms)
- Invisible unicode characters used by chat clear plugins (Allium, etc.)
PLibListener & PEventsListener — Integrated burst detection in both packet listeners
TagManager — Filters <actionbar> and <sound> tags during detected chat clears

Testing

Added ChatResendDetectorTest with unit tests for burst detection logic

Pull request overview

This PR introduces intelligent detection of chat clear/resend operations to prevent actionbar and sound tag spam when chat moderation plugins flood chat with blank lines to clear chat history. The implementation uses two detection mechanisms: timing-based burst detection (6+ packets within 100ms) and invisible unicode character detection commonly used by chat clear plugins.

Key Changes:

Added ChatResendDetector utility class with burst detection and invisible unicode detection
Integrated burst detection into PLibListener and PEventsListener packet processing
Modified TagManager to filter <actionbar> and <sound> tags during detected chat clears

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
ChatResendDetector.java	New utility class providing burst and invisible unicode detection for chat clear operations
ChatResendDetectorTest.java	Unit tests for burst detection timing logic (missing tests for unicode and combined detection)
TagManager.java	Filters actionbar and sound tags during detected burst states
PLibListener.java	Records packets for burst detection and detects invisible unicode in messages
PEventsListener.java	Records packets for burst detection, detects invisible unicode, and adds placeholder detection

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-14T19:23:07Z

core/src/main/java/to/itsme/itsmyconfig/processor/protocollib/PLibListener.java

+        // Record EVERY packet for burst detection (including blank lines)
+        final String playerIdentifier = event.getPlayer().getUniqueId().toString();
+        ChatResendDetector.recordPacket(playerIdentifier);


Incorrect packet recording: The detector records ALL packets processed by this listener, including KICK_DISCONNECT packets (line 38). Kick packets should not contribute to chat resend detection as they're not part of chat history floods. Move the recordPacket call after verifying the packet type is actually a chat packet, not a kick packet.

Copilot · 2025-12-14T19:23:07Z

core/src/main/java/to/itsme/itsmyconfig/processor/packetevents/PEventsListener.java

+        // Record EVERY packet for burst detection (including blank lines)
+        final Player player = event.getPlayer();
+        final String playerIdentifier = player.getUniqueId().toString();
+        ChatResendDetector.recordPacket(playerIdentifier);
+
+        final boolean isInBurst = ChatResendDetector.isInBurst(playerIdentifier);
+        Utilities.debug(() -> "################# CHAT PACKET #################\nProcessing packet " + server.name() + 
+            (isInBurst ? " (RESEND DETECTED)" : ""));

        final PacketProcessor<?> processor = packetTypeMap.get(server);
        if (processor == null) {
            return;
        }



Incorrect packet recording: The detector records packets before checking if the processor exists (line 66-68) and before verifying the packet type is actually a chat packet. This means ALL server packets (not just chat-related ones) are recorded, and DISCONNECT packets (line 34) are also recorded. Disconnect packets should not contribute to chat resend detection. Move the recordPacket call after confirming it's a chat-related packet.

Suggested change

// Record EVERY packet for burst detection (including blank lines)

final Player player = event.getPlayer();

final String playerIdentifier = player.getUniqueId().toString();

ChatResendDetector.recordPacket(playerIdentifier);

final boolean isInBurst = ChatResendDetector.isInBurst(playerIdentifier);

Utilities.debug(() -> "################# CHAT PACKET #################\nProcessing packet " + server.name() +

(isInBurst ? " (RESEND DETECTED)" : ""));

final PacketProcessor<?> processor = packetTypeMap.get(server);

if (processor == null) {

return;

}

final Player player = event.getPlayer();

final String playerIdentifier = player.getUniqueId().toString();

final PacketProcessor<?> processor = packetTypeMap.get(server);

if (processor == null) {

return;

}

// Record only chat-related packets for burst detection

ChatResendDetector.recordPacket(playerIdentifier);

final boolean isInBurst = ChatResendDetector.isInBurst(playerIdentifier);

Utilities.debug(() -> "################# CHAT PACKET #################\nProcessing packet " + server.name() +

(isInBurst ? " (RESEND DETECTED)" : ""));

Copilot · 2025-12-14T19:23:07Z

core/src/main/java/to/itsme/itsmyconfig/util/ChatResendDetector.java

+            if ((System.currentTimeMillis() - windowStart.get()) > BURST_WINDOW_MS) {
+                inBurst = false;
+                count.set(0);


Race condition: The window expiration check and state reset are not atomic. Between checking the window expiration (line 115) and resetting the state (lines 116-117), another thread could call addPacket and modify the count or windowStart. This could lead to inconsistent state where inBurst is reset but count remains non-zero, or vice versa. Consider using synchronized methods or AtomicReference to wrap the BurstTracker state.

Copilot · 2025-12-14T19:23:08Z

core/src/main/java/to/itsme/itsmyconfig/processor/protocollib/PLibListener.java

        final Optional<String> parsed = Strings.parsePrefixedMessage(message);
-        if (parsed.isEmpty()) {
-            Utilities.debug(() -> "Message doesn't start w/ the symbol-prefix: " + message + "\n" + Strings.DEBUG_HYPHEN);
-            return;
-        }
-
+
        final Player player = event.getPlayer();
-        final Component translated = Utilities.translate(parsed.get(), player);
+        // Use the parsed message if available, otherwise use original message
+        final String messageToProcess = parsed.isPresent() ? parsed.get() : message;
+        final Component translated = Utilities.translate(messageToProcess, player);


Logic error: The code now processes ALL messages (even those without the symbol prefix) by falling back to the original message when parsed.isEmpty(). This changes the original behavior which would return early if the message didn't have the prefix. Messages without the prefix should not be processed for placeholder replacement unless there's a specific reason documented in the PR description. This could lead to unintended processing of regular chat messages.

Copilot · 2025-12-14T19:23:08Z

core/src/main/java/to/itsme/itsmyconfig/processor/packetevents/PEventsListener.java

-            Utilities.debug(() -> "Message doesn't start w/ the symbol-prefix: " + message + "\n" + Strings.DEBUG_HYPHEN);
+
+        // Also check if message contains ItsMyConfig placeholders even without prefix (like console does)
+        final boolean hasPlaceholders = message.contains("<p:");


The placeholder check using message.contains("<p:") is fragile and inconsistent with the tag detection pattern used elsewhere in the codebase. This hard-coded string check doesn't account for escaped tags or tags within quotes, and doesn't match the ARG_TAG_PATTERN regex used in TagManager. Consider using a consistent approach for detecting placeholders across the codebase.

Copilot · 2025-12-14T19:23:09Z

core/src/main/java/to/itsme/itsmyconfig/processor/packetevents/PEventsListener.java

+        final boolean isChatClear = isInBurst || hasInvisibleUnicode;
+
+        Utilities.debug(() -> "Found message: " + message + 
+            (hasInvisibleUnicode ? " [INVISIBLE UNICODE]" : "") +
+            (isChatClear ? " [CHAT CLEAR DETECTED]" : ""));


Unused variable: The variable 'isChatClear' is computed but never used. It appears to be calculated for debugging purposes but isn't referenced anywhere in the method logic. Either use it for some purpose or remove it to avoid confusion.

Suggested change

final boolean isChatClear = isInBurst || hasInvisibleUnicode;

Utilities.debug(() -> "Found message: " + message +

(hasInvisibleUnicode ? " [INVISIBLE UNICODE]" : "") +

(isChatClear ? " [CHAT CLEAR DETECTED]" : ""));

Utilities.debug(() -> "Found message: " + message +

(hasInvisibleUnicode ? " [INVISIBLE UNICODE]" : "") +

((isInBurst || hasInvisibleUnicode) ? " [CHAT CLEAR DETECTED]" : ""));

Copilot · 2025-12-14T19:23:10Z

core/src/main/java/to/itsme/itsmyconfig/processor/protocollib/PLibListener.java

+        ChatResendDetector.recordPacket(playerIdentifier);
+
+        final boolean isInBurst = ChatResendDetector.isInBurst(playerIdentifier);
+        Utilities.debug(() -> "################# CHAT PACKET #################\nProccessing packet " + type.name() + 


Typo in debug message: "Proccessing" should be "Processing" (missing one 'c').

Suggested change

Utilities.debug(() -> "################# CHAT PACKET #################\nProccessing packet " + type.name() +

Utilities.debug(() -> "################# CHAT PACKET #################\nProcessing packet " + type.name() +

Copilot · 2025-12-14T19:23:10Z

core/src/main/java/to/itsme/itsmyconfig/util/ChatResendDetector.java

+    }
+
+    /**
+     * Forcefully ends burst tracking for testing or manual intervention.


Incomplete documentation: The Javadoc comment for endBurst mentions "testing or manual intervention" but doesn't warn about the memory leak implications if this method is not called in production. Given that this is the only way to clean up tracker entries, the documentation should clarify when/how this should be called in production environments (e.g., on player disconnect).

Suggested change

* Forcefully ends burst tracking for testing or manual intervention.

* Forcefully ends burst tracking for the given identifier.

* 

* Warning: If this method is not called when a player (or other identifier) disconnects or is no longer being tracked,

* the internal tracker map will retain entries indefinitely, leading to a memory leak.

* 

* This method must be called in production environments (e.g., on player disconnect) to clean up tracker entries.

* While it can also be used for testing or manual intervention, proper cleanup is required to prevent unbounded memory growth.

*

* @param identifier Unique identifier (usually player UUID)

Copilot · 2025-12-14T19:23:10Z

core/src/test/java/to/itsme/itsmyconfig/util/ChatResendDetectorTest.java

+package to.itsme.itsmyconfig.util;
+
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.*;
+
+public class ChatResendDetectorTest {
+
+    @Test
+    public void testNoBurstOnSinglePacket() {
+        final String testId = "test-player-single";
+
+        // Clean up any existing state
+        ChatResendDetector.endBurst(testId);
+
+        // Single packet should not trigger burst
+        ChatResendDetector.recordPacket(testId);
+        assertFalse(ChatResendDetector.isInBurst(testId));
+    }
+
+    @Test
+    public void testNoBurstOnFewPackets() {
+        final String testId = "test-player-few";
+
+        // Clean up any existing state
+        ChatResendDetector.endBurst(testId);
+
+        // Few packets (under threshold of 6) should not trigger burst
+        for (int i = 0; i < 4; i++) {
+            ChatResendDetector.recordPacket(testId);
+            assertFalse(ChatResendDetector.isInBurst(testId));
+        }
+    }
+
+    @Test
+    public void testBurstOnManyRapidPackets() {
+        final String testId = "test-player-burst";
+
+        // Clean up any existing state
+        ChatResendDetector.endBurst(testId);
+
+        // 6+ packets in rapid succession should trigger burst
+        for (int i = 0; i < 5; i++) {
+            ChatResendDetector.recordPacket(testId);
+            assertFalse(ChatResendDetector.isInBurst(testId)); // 1-5
+        }
+        ChatResendDetector.recordPacket(testId); // 6th
+        assertTrue(ChatResendDetector.isInBurst(testId)); // Should now be in burst
+    }
+
+    @Test
+    public void testBurstResetAfterDelay() throws InterruptedException {
+        final String testId = "test-player-reset";
+
+        // Clean up any existing state
+        ChatResendDetector.endBurst(testId);
+
+        // Send rapid packets to trigger burst
+        for (int i = 0; i < 6; i++) {
+            ChatResendDetector.recordPacket(testId);
+        }
+        assertTrue(ChatResendDetector.isInBurst(testId));
+
+        // Wait for window to expire
+        Thread.sleep(120); // Longer than 100ms threshold
+
+        // Should no longer be in burst
+        assertFalse(ChatResendDetector.isInBurst(testId));
+    }
+
+    @Test
+    public void testIndependentPlayerTracking() {
+        final String player1 = "test-player-1";
+        final String player2 = "test-player-2";
+
+        // Clean up
+        ChatResendDetector.endBurst(player1);
+        ChatResendDetector.endBurst(player2);
+
+        // Trigger burst for player1
+        for (int i = 0; i < 6; i++) {
+            ChatResendDetector.recordPacket(player1);
+        }
+
+        // Player1 should be in burst, player2 should not
+        assertTrue(ChatResendDetector.isInBurst(player1));
+        assertFalse(ChatResendDetector.isInBurst(player2));
+    }
+}


Missing test coverage for isChatClearPacket method: This public method combines burst detection with invisible unicode detection and is used by the listeners, but there are no tests covering it. Add tests to verify it correctly returns true when either condition is met (burst OR invisible unicode), and false when neither is present.

Copilot · 2025-12-14T19:23:10Z

core/src/main/java/to/itsme/itsmyconfig/processor/protocollib/PLibListener.java

+        final boolean isChatClear = isInBurst || hasInvisibleUnicode;
+
+        Utilities.debug(() -> "Found message: " + message + 
+            (hasInvisibleUnicode ? " [INVISIBLE UNICODE]" : "") +
+            (isChatClear ? " [CHAT CLEAR DETECTED]" : ""));


Unused variable: The variable 'isChatClear' is computed but never used. It appears to be calculated for debugging purposes but isn't referenced anywhere in the method logic. Either use it for some purpose or remove it to avoid confusion.

Suggested change

final boolean isChatClear = isInBurst || hasInvisibleUnicode;

Utilities.debug(() -> "Found message: " + message +

(hasInvisibleUnicode ? " [INVISIBLE UNICODE]" : "") +

(isChatClear ? " [CHAT CLEAR DETECTED]" : ""));

Utilities.debug(() -> "Found message: " + message +

(hasInvisibleUnicode ? " [INVISIBLE UNICODE]" : "") +

((isInBurst || hasInvisibleUnicode) ? " [CHAT CLEAR DETECTED]" : ""));

castledking added 2 commits December 13, 2025 12:10

Add smart chat resend/clear detection with invisible unicode and burs…

6d76662

…t detection

fix: Lower burst threshold to 6 to match test expectations

80ba978

itsme-to requested a review from iiAhmedYT December 14, 2025 16:31

iiAhmedYT requested review from Copilot and removed request for iiAhmedYT December 14, 2025 19:18

Copilot started reviewing on behalf of iiAhmedYT December 14, 2025 19:18 View session

castledking closed this Dec 14, 2025

castledking deleted the feat/clear-chat-intercept branch December 14, 2025 19:18

Copilot AI reviewed Dec 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add smart chat resend/clear detection and intercept <actionbar> & <sound> tags#25

Add smart chat resend/clear detection and intercept <actionbar> & <sound> tags#25
castledking wants to merge 2 commits intoItsMyStudio:mainfrom
castledking:feat/clear-chat-intercept

castledking commented Dec 13, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 14, 2025

Uh oh!

Copilot AI Dec 14, 2025

Uh oh!

Copilot AI Dec 14, 2025

Uh oh!

Copilot AI Dec 14, 2025

Uh oh!

Copilot AI Dec 14, 2025

Uh oh!

Copilot AI Dec 14, 2025

Uh oh!

Copilot AI Dec 14, 2025

Uh oh!

Copilot AI Dec 14, 2025

Uh oh!

Copilot AI Dec 14, 2025

Uh oh!

Copilot AI Dec 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	Utilities.debug(() -> "################# CHAT PACKET #################\nProccessing packet " + type.name() +
	Utilities.debug(() -> "################# CHAT PACKET #################\nProcessing packet " + type.name() +

-     * Forcefully ends burst tracking for testing or manual intervention.
+     * Forcefully ends burst tracking for the given identifier.
+     * <p>
+     * <b>Warning:</b> If this method is not called when a player (or other identifier) disconnects or is no longer being tracked,
+     * the internal tracker map will retain entries indefinitely, leading to a memory leak.
+     * <p>
+     * This method <b>must</b> be called in production environments (e.g., on player disconnect) to clean up tracker entries.
+     * While it can also be used for testing or manual intervention, proper cleanup is required to prevent unbounded memory growth.
+     *
+     * @param identifier Unique identifier (usually player UUID)

Conversation

castledking commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Testing

Related

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

castledking commented Dec 13, 2025 •

edited

Loading