diff --git a/forge-gui-desktop/src/test/java/forge/net/ChatRateLimitTest.java b/forge-gui-desktop/src/test/java/forge/net/ChatRateLimitTest.java new file mode 100644 index 000000000000..f2cdc097e288 --- /dev/null +++ b/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"); + } + } + +} diff --git a/forge-gui/src/main/java/forge/gamemodes/net/server/FServerManager.java b/forge-gui/src/main/java/forge/gamemodes/net/server/FServerManager.java index 3642e6c9ee9e..224eaad16b47 100644 --- a/forge-gui/src/main/java/forge/gamemodes/net/server/FServerManager.java +++ b/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) { diff --git a/forge-gui/src/main/java/forge/gamemodes/net/server/RemoteClient.java b/forge-gui/src/main/java/forge/gamemodes/net/server/RemoteClient.java index afc860a9457d..87e7315ea857 100644 --- a/forge-gui/src/main/java/forge/gamemodes/net/server/RemoteClient.java +++ b/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;