sopel
sopel copied to clipboard
[Proposal] Update flood protection logic
Flood-prevention measures need to be refactored.
Proposed minimum changes:
- Move logic into
irc.Bot.write
- Calculate writing backoff (penalty) based on an adequately massaged message (e.g., post-truncation)
Background
RFC 1459 spec on flood control of clients:
8.10 Flood control of clients
With a large network of interconnected IRC servers, it is quite easy
for any single client attached to the network to supply a continuous
stream of messages that result in not only flooding the network, but
also degrading the level of service provided to others. Rather than
require every 'victim' to be provide their own protection, flood
protection was written into the server and is applied to all clients
except services. The current algorithm is as follows:
* check to see if client's `message timer' is less than
current time (set to be equal if it is);
* read any data present from the client;
* while the timer is less than ten seconds ahead of the current
time, parse any present messages and penalize the client by
2 seconds for each message;
which in essence means that the client may send 1 message every 2
seconds without being adversely affected.
Current bot layout: (I may have missed some :hushed:)
** = flood-prevention logic is here
+----------------+ +---------------+ +-------------+
| Sopel.action +--->+ **Sopel.say** +--->+ |
| Sopel.msg | +---------------+ | | +---------------+ +--------------------------+
| Sopel.reply | | Sopel.write +--->+ irc.Bot.write +--->+ asynchat.async_chat.send |
+----------------+ +---------------+ | | +---------------+ +--------------------------+
| Sopel.notice +--->+ | .
| Sopel.part | +-------------+ /|\
| Sopel.join | |
| Sopel.kick ? | |
+---------------+ +-----------+
| CAP,NICK, |
| PASS,... |
+-----------+
Rationale
A few reasons this change is needed:
- The current logic calculates a penalty based on the untruncated message. This led to the fiasco of https://github.com/sopel-irc/sopel/issues/1550#issuecomment-481937234 and #1551.
- The penalty should be based only on the actual part of the message that is being sent to the server.
- All the methods that don't use
Sopel.say
currently, bypass flood-prevention logic.
Future plans
Potential additional changes:
- Add configurable backoff
- Add a configurable maximum penalty
- Allow message burst (with a configurable number and subsequent delays)
I'm working on that, but I need #1518 to be merged before I can publish anything.
@Exirel Do you still want to work on this for 7.0, or should we defer it to 7.1?
Ok, after looking for it, I think it's best to postpone to 7.1 (if not later).
@HumorBaby I'm about to look at making a small PR for 7.1 that changes the flood penalty calculation to use the truncated message, but regardless of whether that ships or not… I think most of your "Future plans" section is already done, actually.
We have flood_burst_lines
(burst quota before triggering Sopel's send-limits), flood_empty_rate
(delay between messages when flood-limited), and flood_refill_rate
(burst message quota recovery per second) in 7.0 (from #1518); flood_max_wait
takes care of a configurable maximum wait time for 7.1 (from #1929). The only thing that might not be present yet is "configurable backoff", but you might consider it partially/wholly obviated by one of the above or remaining new options (flood_penalty_ratio
or flood_text_length
).
It would benefit Sopel most in the long term to move its flood prevention logic into the IRC backend (so it applies to everything sent, not just what the bot object sends)—a step further than your proposed move into bot.write()
. I'm just looking at the other stuff in this issue to see if it's still relevant, or already done in 7.x; those main changes will still wait until 8.0.