Rate limit and bound chat from remote clients

forge-gui-desktop/src/test/java/forge/net/ChatRateLimitTest.java

@@ -0,0 +1,53 @@
+package forge.net;
+
+import forge.gamemodes.net.server.RemoteClient;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+/**
+ * Chat is accepted before login and rebroadcast to every peer, so a client
+ * that sends it in a loop costs the host and everyone else. The limit is a
+ * token bucket sized for a person typing, and switchable off.
+ */
+public class ChatRateLimitTest {
+
+    /** Far above any legitimate burst, low enough to fail fast. */
+    private static final int DRAIN_CEILING = 10_000;
+
+    @Test(timeOut = 30_000)
+    public void testChatIsRateLimitedAndRefills() throws Exception {
+        final RemoteClient client = new RemoteClient(null);
+
+        // Bounded, not "while (allowChatMessage())". An unbounded drain loop
+        // terminates only if the limiter works, so with the limiter removed --
+        // the case this exists to detect -- it spins forever and the run hangs
+        // instead of failing. A hang reads as "still running", which is worse.
+        int drained = 0;
+        while (drained < DRAIN_CEILING && client.allowChatMessage()) {
+            drained++;
+        }
+        Assert.assertTrue(drained > 0, "A burst should be allowed - people do type");
+        Assert.assertTrue(drained < DRAIN_CEILING,
+                "Bucket never emptied after " + DRAIN_CEILING + " messages; not rate limiting");
+
+        // One token per refill interval, so this cannot be shortened much.
+        Thread.sleep(1100);
+        Assert.assertTrue(client.allowChatMessage(),
+                "Allowance must come back - a rate limit that never refills is a mute");
+    }
+
+    @Test(timeOut = 30_000)
+    public void testLimitCanBeSwitchedOff() {
+        System.setProperty("forge.net.chatBurst", "0");
+        try {
+            final RemoteClient client = new RemoteClient(null);
+            for (int i = 0; i < DRAIN_CEILING; i++) {
+                Assert.assertTrue(client.allowChatMessage(),
+                        "Disabled limit must never refuse (refused at " + i + ")");
+            }
+        } finally {
+            System.clearProperty("forge.net.chatBurst");
+        }
+    }
+
+}

forge-gui/src/main/java/forge/gamemodes/net/server/FServerManager.java

@@ -71,6 +71,15 @@ public final class FServerManager implements IHasForgeLog {
 
     static final int HEARTBEAT_TIMEOUT_SECONDS = Integer.getInteger("forge.net.heartbeatTimeout", 45);
 
+    /**
+     * Chat is rebroadcast to every peer, so one very long message costs
+     * everyone. Read per call for the same reason as the burst in
+     * {@link RemoteClient}; zero or less switches truncation off.
+     */
+    private static int maxChatLength() {
+        return Integer.getInteger("forge.net.maxChatLength", 512);
+    }
+
     private static final int OUTBOUND_BUFFER_LOW_WATER = 64 * 1024;
     private static final int OUTBOUND_BUFFER_HIGH_WATER = 1024 * 1024;
     private static final int RECONNECT_TIMEOUT_SECONDS = 300;
@@ -1008,10 +1017,17 @@ public final void channelRead(final ChannelHandlerContext ctx, final Object msg)
                 if (client == null) {
                     return;
                 }
+                if (!client.allowChatMessage()) {
+                    return; // Over its rate; dropped rather than queued
+                }
                 // Strip control characters before echoing to other players: a
                 // carriage return lets one player paint fake system lines in
-                // everyone else's chat pane.
-                final String text = LogSafe.forDisplay(raw);
+                // everyone else's chat pane. Stripping is unconditional; only
+                // the length cap is policy, so zero or less strips without
+                // truncating rather than skipping the scrub.
+                final String text = maxChatLength() > 0
+                        ? LogSafe.forDisplay(raw, maxChatLength())
+                        : LogSafe.forDisplay(raw, Integer.MAX_VALUE);
                 String username = client.getUsername();
                 // Append (Host) indicator for the host player
                 if (client.getIndex() == 0) {

forge-gui/src/main/java/forge/gamemodes/net/server/RemoteClient.java

@@ -18,8 +18,25 @@ public final class RemoteClient implements IToClient, IHasForgeLog {
     /** Special value indicating the client hasn't been assigned a slot yet. */
     public static final int UNASSIGNED_SLOT = -1;
 
+    /**
+     * Token-bucket allowance for chat, sized for a person typing rather than a
+     * script holding the key down. Read per call rather than into static final
+     * fields, because Integer.getInteger in a static initialiser is fixed at
+     * class-load and surefire shares one JVM, so a test could not vary them.
+     * A burst of zero or less switches the limit off.
+     */
+    private static int chatBurst() {
+        return Integer.getInteger("forge.net.chatBurst", 10);
+    }
+
+    private static int chatRefillMillis() {
+        return Integer.getInteger("forge.net.chatRefillMillis", 1000);
+    }
+
     private volatile Channel channel;
     private String username;
+    private double chatTokens = chatBurst();
+    private long chatLastRefill = System.currentTimeMillis();
     private int index = UNASSIGNED_SLOT;
     private boolean libgdx;
     private volatile ReplyPool replies = new ReplyPool();
@@ -67,6 +84,29 @@ public SocketAddress getRemoteAddress() {
         return ch == null ? null : ch.remoteAddress();
     }
 
+    /**
+     * Consume one chat allowance; false when the peer is over its rate.
+     * Always true when the limit is switched off.
+     */
+    public synchronized boolean allowChatMessage() {
+        final int burst = chatBurst();
+        if (burst <= 0) {
+            return true;
+        }
+        final long refill = Math.max(1, chatRefillMillis());
+        final long now = System.currentTimeMillis();
+        final long elapsed = now - chatLastRefill;
+        if (elapsed > 0) {
+            chatTokens = Math.min(burst, chatTokens + (double) elapsed / refill);
+            chatLastRefill = now;
+        }
+        if (chatTokens < 1) {
+            return false;
+        }
+        chatTokens -= 1;
+        return true;
+    }
+
     /** Encodes synchronously on the caller's thread. Returns null on failure (logged). */
     private ByteBuf encodeOnCallingThread(final NetEvent event) {
         final Channel ch = channel;