Rate limit and bound chat from remote clients - #11457
Conversation
|
No problem in principle, but what was the situation that led to the need for this? |
Chat is accepted from a remote client before it has logged in and is rebroadcast to every peer, so a client sending it in a loop costs the host's CPU, everyone's bandwidth and everyone's scrollback at once. Nothing bounds the rate, and the length bound is fixed. Adds a per-client token bucket sized for a person typing (10 message burst, one refilled per second). Over-rate messages are dropped rather than queued, since queuing is what the sender wants. The existing LogSafe length bound on the rebroadcast becomes configurable at the same time. Both are policy rather than correctness, so both are tunable and both can be switched off outright: forge.net.chatBurst, forge.net.chatRefillMillis and forge.net.maxChatLength, alongside the existing forge.net.heartbeatTimeout. Zero or less on the burst disables rate limiting; zero or less on the length disables truncation. The defaults are a guess at "generous for a human" -- if they are wrong for how people actually use the lobby, they are one property away rather than a rebuild. Note that switching the cap off still strips control characters, since that is a correctness property of the rebroadcast rather than an anti-spam measure -- only the truncation is optional. Values are 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 otherwise vary them. Deliberately not included: a general per-message rate limit covering all inbound traffic. That risks throttling legitimate high-rate game traffic, and there is no measured baseline here for what a busy turn sends -- guessing a number and stalling a real game is how this kind of change gets reverted. Portions authored with an AI assistant (Claude), reviewed by a human. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1781c01 to
66f258c
Compare
|
No inciting incident. I was looking at ways clients could overload each other (for 2df8aaa) and noticed there were no spam protections. Spam isn't much of an issue with no public rooms or matchmaking, but it seemed like an easy fix, so here it is if you're interested. The limits are off-switchable if the defaults turn out wrong for how people actually use the lobby. |
|
@tool4ever I don't have a hard view against this, but it does seem to be solving a problem that doesn't actually exist in practice. |
|
ok thanks, yea this could become more interesting if lobbies were more broadly available like on some forks |
Summary
Chat from a remote client is accepted before login and rebroadcast to every peer, and nothing bounds how fast those messages arrive. A client sending chat in a loop costs the host's CPU, everyone's bandwidth and everyone's scrollback at once.
This adds a per-client token bucket, and makes the existing length bound on the rebroadcast configurable.
Behaviour
LogSafe's default.Both are policy rather than correctness, so both are tunable and both can be switched off outright:
forge.net.chatBurst0forge.net.chatRefillMillisforge.net.maxChatLength0These sit alongside the existing
forge.net.heartbeatTimeout. The defaults are a guess at "generous for a human" — if they are wrong for how people actually use the lobby, they are one property away rather than a rebuild.Switching the cap off still strips control characters from the rebroadcast: that part is a correctness property, not an anti-spam measure, so only the truncation is optional.
Values are read per call rather than into
static finalfields, becauseInteger.getIntegerin a static initialiser is fixed at class-load and surefire shares one JVM, so a test could not otherwise vary them.Deliberately not included
A general per-message rate limit covering all inbound traffic. That risks throttling legitimate high-rate game traffic, and there is no measured baseline here for what a busy turn sends — guessing a number and stalling a real game is how this kind of change gets reverted.
Tests
ChatRateLimitTest, 2 unit tests, no server and no socket: the bucket empties and refills, and the disable switch never refuses.Portions authored with an AI assistant (Claude), reviewed by a human.