irssi-import / bugs.irssi.org

bugs.irssi.org archive
https://github.com/irssi/irssi/issues
0 stars 0 forks source link

Parse FLOOD= information from ISUPPORT #796

Open irssibot opened 13 years ago

irssibot commented 13 years ago

Background: BitlBee doesn't care about message floods at all and in fact it's good if the IRC client does not rate limit pastes so they can be converted to multiline messages on the IM side. ATM it takes some effort to configure one's well-behaved IRC client to be willing to flood BitlBee.

To make this easier, I added a little piece of info to BitlBee's 005:

:localhost 005 wilmer ... FLOOD=0/9999 :are supported by this server

The format is FLOOD=cmd_queue_speed/max_cmds_at_once. One would maybe consider this rather biased against irssi's implementation of rate limiting but it seems generic to me.

I have a patch to add support for this to irssi. It's pretty simple and will let the user override whatever is in ISUPPORT with what s/he configured for an IRCNet specifically. The info will override irssi-wide limits though.

I'm flooring the time to 10ms, since I remember seeing somewhere that this is the magic "flood me" value. I could be wrong though.

With the little testing I've done, this seems to work well. Of course it may be fully ignorant of irssi coding best practices though. Suggestions for improvement are welcome.

But most of all, are you willing to accept this feature?

irssibot commented 13 years ago

irssi-isupport-flood.diff

Index: src/irc/core/irc-servers.c
===================================================================
--- src/irc/core/irc-servers.c  (revision 5203)
+++ src/irc/core/irc-servers.c  (working copy)
@@ -28,6 +28,7 @@
 #include "channels.h"
 #include "queries.h"

+#include "irc-chatnets.h"
 #include "irc-nicklist.h"
 #include "irc-queries.h"
 #include "irc-servers-setup.h"
@@ -860,6 +861,19 @@
        if (server->max_msgs_in_cmd <= 0)
            server->max_msgs_in_cmd = 1;
    }
+   
+   if ((sptr = g_hash_table_lookup(server->isupport, "FLOOD"))) {
+       IRC_CHATNET_REC *net;
+       int cmd_queue_speed, max_cmds_at_once;
+       if ((!server->connrec->chatnet ||
+            !(net = irc_chatnet_find(server->connrec->chatnet)) ||
+            (net->cmd_queue_speed == 0 && net->max_cmds_at_once == 0)) &&
+           sscanf(sptr, "%d/%d", &cmd_queue_speed, &max_cmds_at_once) == 2) {
+           server->cmd_queue_speed = cmd_queue_speed > 10 ?
+                                     cmd_queue_speed : 10;
+           server->max_cmds_at_once = max_cmds_at_once;
+       }
+   }
 }

 void irc_servers_init(void)
irssibot commented 13 years ago

Makes sense to me.

irssibot commented 13 years ago

I'm not sure I agree with this. I don't think we should be adding custom ISUPPORT functionality to support what is basically a local patch to bitlbee, especially given that this can already be easily done via /network add -cmdspeed 0 -cmdmax 9999 bitlbee .

irssibot commented 13 years ago

This is not a local patch. This is in bzr and will be in the next release, and if I intended this to be BitlBee-specific I'd just look at IRCNET=BitlBee. This stuff can be much more generic so any IRC network can specify what message rate will trigger their flood detection so IRC clients no longer have to be overly conservative to be on the safe side everywhere.

irssibot commented 13 years ago

It would have helped if you stated that it had been committed to bitlbee's repo (I wasn't aware that you're a bitlbee dev) ;) As it has been committed, that pretty much removes my reason for objecting to this request.

I would however like to see this patch respecting the global limits if they've been changed from default.

Edit: Or perhaps the lower of the out of the two values (ISupport vs global)?

irssibot commented 13 years ago

D'oh. I now notice that I forgot to include a link to the commit on bugs.bitlbee.org. I really thought it was there. See http://gaa.st/aJ . Sorry about that.

I'll try to adapt my patch to not blindly override global limits either. To clarify, with lower do you mean isupport should be able to make irssi more floody or less floody? And the per-ircnet setting should still "win"?

While you're here BTW, at some point IIRC I'm pretty sure managed to get irssi to send all lines of a flood in one packet. Not anymore now, they're sent one by one, even with a 0ms delay. Not a huge deal but I wonder if I'm on crack and imagined something that the code can't technically do or if I just forgot with what setting it did that. I saw this in tcpdump, so it's also possible that somehow multiple writes once ended up in one packet.

irssibot commented 13 years ago

I wonder if there is a nicer way to check if something is changed from the default than just manually comparing the setting against the default value?