Repository: gawel/irc3

Issue:
Split long messages to avoid being kicked for excess flood
Hi,

My bot sends messages to a channel whose length can sometimes be too long for the server to accept.

Would it be possible for `irc3` (maybe via an opt-in mechanism) to split long message queries into several ones, to avoid the bot being kicked for excess flood? The message itself could be split on word boundaries.

This could theoretically be handled on the bot's side, but I don't think it would be very robust, as the bot doesn't know exactly what the resulting IRC query will look like (I assume something like `PRIVMSG recipient :message`), and thus not be able to know how many characters should be subtracted to the theoretical maximum message size (512 I believe).

HTH.

Discussion:
@gawel: Hi,

You can set `max_length` in the config file. Default value is 512: https://github.com/gawel/irc3/blob/master/irc3/base.py#L75
@lenormf: Do servers communicate what maximum message length they accept? In that case, I would like `irc3` to detect that, and split messages according to it.@gawel: It's possible. But the rfc say 512 is fine: https://github.com/gawel/irc3/blob/2d7d65ca753fdd13b686d0ba5d4c180131ada6f6/irc3/rfc2812.txt#L300

You may be able to track this for your server by checking `bot.config['server_config']`

See https://github.com/gawel/irc3/blob/2d7d65ca753fdd13b686d0ba5d4c180131ada6f6/irc3/plugins/core.py#L109@lenormf: So, the server I'm connecting to is GameSurge.

Here is the configuration the server sends to the bot:

```
:NuclearFallout.WA.US.GameSurge.net 005 _nick WHOX WALLCHOPS WALLVOICES USERIP CPRIVMSG CNOTICE SILENCE=25 MODES=6 MAXCHANNELS=75 MAXBANS=100 NICKLEN=30 :are supported by this server
:NuclearFallout.WA.US.GameSurge.net 005 _nick MAXNICKLEN=30 TOPICLEN=300 AWAYLEN=200 KICKLEN=300 CHANNELLEN=200 MAXCHANNELLEN=200 CHANTYPES=#& PREFIX=(ov)@+ STATUSMSG=@+ CHANMODES=b,k,l,imnpstrDdRcC CASEMAPPING=rfc1459 NETWORK=GameSurge :are supported by this server
```

The bot subsequently prints these values as:

```
Server config: {'TOPICLEN': '300', 'MAXCHANNELS': '75', 'PREFIX': '(ov)@+', 'MAXCHANNELLEN': '200', 'CHANNELLEN': '200', 'MAXBANS': '100', 'CASEMAPPING': 'rfc1459', 'NETWORK': 'GameSurge', 'USERIP': True, 'WHOX': True, 'CHANMODES': 'b,k,l,imnpstrDdRcC', 'AWAYLEN': '200', 'MODES': '6', 'SILENCE': '25', 'STATUSMSG': '@+', 'MAXNICKLEN': '30', 'KICKLEN': '300', 'WALLVOICES': True, 'NICKLEN': '30', 'CPRIVMSG': True, 'WALLCHOPS': True, 'CHANTYPES': '#&', 'CNOTICE': True}
```

The `'max_length'` option in `bot.config` is set to `512`.

When the bot sends a message that is >512 characters long (I can see that message in the debug log), the server replies:

```
:NuclearFallout.WA.US.GameSurge.net 417 _nick :Input line was too long
```

That could mean that either:

- `irc3` is not splitting messages to make sure packets remain below 512 characters — if it's not supposed to at the moment, I'd like it to do so
- GameSurge uses a maximum length lesser than 512 and doesn't communicate it to `irc3` to split messages — I haven't tried to verify that, it's possible that GameSurge uses 512 as limit

HTH.@gawel: As I remember you can track what the bot receive/send by using `irc3 -vdr config.ini`@lenormf: After debugging the issue, I found two problems, and one suspicious thing:

- the `IrcBot.privmsg()` function assumes that messages (i.e. words sent by users) must be `self.config.max_length` in length, when that number applies to the whole IRC message (i.e. `PRIVMSG target :message\r\n`) — other parts of the code that deal with protocol might have this issue as well, I didn't check

https://github.com/gawel/irc3/blob/2d7d65ca753fdd13b686d0ba5d4c180131ada6f6/irc3/rfc2812.txt#L272-L274

https://github.com/gawel/irc3/blob/2d7d65ca753fdd13b686d0ba5d4c180131ada6f6/irc3/rfc2812.txt#L300-L304

- the length of the IRC message is supposed to be counted in bytes (ASCII or encoded UTF8), but `utils.split_messages()` works on unicode strings, which can throw off the count — I couldn't really find anything in the RFC about that after a cursory look, maybe some servers (other than GameSurge) are lenient and count codepoints?

- the bot sends messages one after the other without any form of rate limiting, which results in it being kicked for excess flood, once the above problems are solved — I can see a `flood_delay` local configuration option, but from what I can tell the server doesn't seem to indicate what minimum delay should separate messages that it could be compared to

Here is a debugging patch that I used to diagnose the above issues:

```diff
diff --git a/irc3/__init__.py b/irc3/__init__.py
index 2cd5fe7..12ee2c9 100644
--- a/irc3/__init__.py
+++ b/irc3/__init__.py
@@ -179,6 +179,7 @@ class IrcBot(base.IrcObject):
 
     def send_line(self, data, nowait=False):
         """send a line to the server. replace CR by spaces"""
+        self.log.debug("DBG: (%d) %s", len(data), data)
         data = data.replace('\n', ' ').replace('\r', ' ')
         f = asyncio.Future(loop=self.loop)
         if self.queue is not None and nowait is False:
@@ -225,15 +226,23 @@ class IrcBot(base.IrcObject):
     def privmsg(self, target, message, nowait=False):
         """send a privmsg to target"""
         if message:
-            messages = utils.split_message(message, self.config.max_length)
             if isinstance(target, DCCChat):
+                messages = utils.split_message(message, self.config.max_length)
                 for message in messages:
                     target.send_line(message)
             elif target:
+                command_prefix = 'PRIVMSG %s :' % target
+                self.log.debug("MAX_LENGTH: %d" % self.config.max_length)
+                assert self.config.max_length > len(command_prefix) + 2
+                self.log.debug("SPLIT: %s" % message)
+                messages = utils.split_message(message, self.config.max_length - len(command_prefix) - 2, self.log)
+                self.log.debug("MESSAGES: %s", messages)
                 f = None
                 for message in messages:
-                    f = self.send_line('PRIVMSG %s :%s' % (target, message),
+                    self.log.debug("DBG: MSG (%d) %s", len(message), message)
+                    f = self.send_line('%s%s' % (command_prefix, message),
                                        nowait=nowait)
+                    break
                 return f
 
     def action(self, target, message, nowait=False):
diff --git a/irc3/utils.py b/irc3/utils.py
index 4f158f8..fb91620 100644
--- a/irc3/utils.py
+++ b/irc3/utils.py
@@ -164,8 +164,38 @@ class IrcString(BaseString):
 STRIPPED_CHARS = '\t '
 
 
-def split_message(message, max_length):
+def split_message(message, max_length, log=None):
     """Split long messages"""
+    def utf8_lead_byte(b):
+        '''A UTF-8 intermediate byte starts with the bits 10xxxxxx.'''
+        return (b & 0xC0) != 0x80
+
+    def utf8_byte_truncate(text, max_bytes):
+        '''If text[max_bytes] is not a lead byte, back up until a lead byte is
+        found and truncate before that character.'''
+        log.debug("IN")
+        utf8 = text.encode('utf8')
+        log.debug("utf8: %s", utf8)
+        if len(utf8) <= max_bytes:
+            log.debug("SPLIT: max bytes")
+            return utf8
+        log.debug("max_bytes: %d", max_bytes)
+        i = max_bytes
+        while i > 0 and not utf8_lead_byte(utf8[i]):
+            log.debug("I: %d", i)
+            i -= 1
+        log.debug("RET")
+        return utf8[:i]
+
+    if log is not None:
+        log.debug("SPLIT MSG: %d - %s", max_length, message)
+        s = utf8_byte_truncate(message, max_length)
+        log.debug("S: %s", s)
+        s = s.decode('utf8')
+        log.debug("DECODED")
+        yield s
+        return
+
     if len(message) > max_length:
         for message in textwrap.wrap(message, max_length):
             yield message
```

I used this implementation to split strings according to byte length: https://stackoverflow.com/a/43848928

HTH.@gawel: Thanks for digging into that. I don't like the idea of encoding/decoding the whole messages to... re-encode them before sending to the server. encoding/decoding are very CPU expensive AFAIK.@lenormf: Any chance we'll get a fix soon? My bot trips on half of the URLs (apparently the internet has decided that having 1k+ characters long sentences in the metadata is a good idea).

I understand the concern about the encoding round trips, however there doesn't seem to be any alternative that I can see.@gawel: I'll be ok with that if there is no need to re-decode the data. I'm pretty sure a few `if isinstance(data, bytes)` will be faster than decoding/encoding the whole messages multiple times. Don't know how hard it is to add those `if` and how many of them are required.

I'll try to give it a try soon...@gawel: For now I've added a prefix argument to split_message (inspired by your snippet). This should reduce your amount of errors. Maybe it's enough to "fix" the problem if you set max_length to an appropriate value to allow some extra unicode characters.. Maybe 500 instead of 512.

Looks like supporting both unicode and bytes in the whole process will not be easy at all..

Solution patch:
diff --git a/irc3/__init__.py b/irc3/__init__.py
index 2ed8256..681b298 100644
--- a/irc3/__init__.py
+++ b/irc3/__init__.py
@@ -230,6 +230,7 @@ class IrcBot(base.IrcObject):
             messages = utils.split_message(
                 message,
                 self.config.max_length,
+                self.encoding,
                 prefix=prefix,
             )
             if is_dcc:
@@ -254,6 +255,7 @@ class IrcBot(base.IrcObject):
             messages = utils.split_message(
                 message,
                 self.config.max_length,
+                self.encoding,
                 prefix=prefix,
             )
             if is_dcc:
@@ -269,7 +271,11 @@ class IrcBot(base.IrcObject):
     def ctcp(self, target, message, nowait=False):
         """send a ctcp to target"""
         if target and message:
-            messages = utils.split_message(message, self.config.max_length)
+            messages = utils.split_message(
+                message,
+                self.config.max_length,
+                self.encoding,
+            )
             f = None
             for message in messages:
                 f = self.send_line('PRIVMSG %s :\x01%s\x01' % (target,
@@ -280,7 +286,11 @@ class IrcBot(base.IrcObject):
     def ctcp_reply(self, target, message, nowait=False):
         """send a ctcp reply to target"""
         if target and message:
-            messages = utils.split_message(message, self.config.max_length)
+            messages = utils.split_message(
+                message,
+                self.config.max_length,
+                self.encoding,
+            )
             f = None
             for message in messages:
                 f = self.send_line('NOTICE %s :\x01%s\x01' % (target, message),
diff --git a/irc3/plugins/command.py b/irc3/plugins/command.py
index af509cd..0159195 100644
--- a/irc3/plugins/command.py
+++ b/irc3/plugins/command.py
@@ -423,7 +423,11 @@ class Commands(dict):
                         buf += line + ' '
                     else:
                         if buf is not None:
-                            for b in utils.split_message(buf, 160):
+                            for b in utils.split_message(
+                                buf,
+                                160,
+                                self.context.encoding,
+                            ):
                                 yield b
                             buf = None
                         line = line.replace('%%', self.context.config.cmd)
@@ -440,7 +444,10 @@ class Commands(dict):
                            if p.get('show_in_help_list', True)))
             cmds_str = ', '.join([self.cmd + k for k in cmds])
             lines = utils.split_message(
-                'Available commands: %s ' % cmds_str, 160)
+                'Available commands: %s ' % cmds_str,
+                160,
+                self.context.encoding,
+            )
             for line in lines:
                 yield line
             url = self.config.get('url')
diff --git a/irc3/utils.py b/irc3/utils.py
index 459bcab..0960fee 100644
--- a/irc3/utils.py
+++ b/irc3/utils.py
@@ -5,7 +5,6 @@ from . import tags
 import configparser
 import importlib
 import functools
-import textwrap
 import logging
 import os
 import re
@@ -156,17 +155,62 @@ class IrcString(str):
         return self._tagdict
 
 
-STRIPPED_CHARS = '\t '
+SPACE = ' '
 
 
-def split_message(message, max_length, prefix=''):
-    """Split long messages"""
-    max_length = max_length - len(prefix)
-    if len(message) > max_length:
-        for message in textwrap.wrap(message, max_length):
-            yield message
-    else:
-        yield message.rstrip(STRIPPED_CHARS)
+def split_message(message, max_bytes, encoding, prefix=''):
+    """Split long messages based on byte length, ensuring chunks
+    do not exceed max_bytes when encoded. Break up words when necessary.
+    """
+    if max_bytes <= 0:
+        raise ValueError("max_bytes must be positive")
+
+    prefix_bytes = len(prefix.encode(encoding))
+    max_bytes -= prefix_bytes
+    if max_bytes <= 0:
+        raise ValueError(f"{max_bytes=} too small for prefix")
+
+    byte_message = message.encode(encoding)
+    message_bytes = len(byte_message)
+
+    start = 0
+
+    while start < message_bytes:
+        end = min(start + max_bytes, message_bytes)
+        chunk = byte_message[start:end]
+
+        while True:
+            valid = True
+
+            # Ensure the chunk doesn't end mid-character.
+            try:
+                chunk_str = chunk.decode(encoding, errors='strict')
+            except UnicodeDecodeError:
+                valid = False
+
+            valid = valid and (
+                end == message_bytes or chunk_str[-1] == SPACE or SPACE
+                not in chunk_str
+            )
+
+            if valid:
+                break
+
+            chunk = chunk[:-1]
+            end -= 1
+
+            if end == start:
+                raise ValueError(f"cannot fit chunk in {max_bytes=}")
+
+        if start == 0:
+            # Keep leading spaces.
+            clean = chunk_str.rstrip(SPACE)
+        else:
+            clean = chunk_str.strip(SPACE)
+        if clean:
+            yield clean
+
+        start = end
 
 
 class Config(dict):
diff --git a/irc3d/__init__.py b/irc3d/__init__.py
index 356a9fb..e92122e 100644
--- a/irc3d/__init__.py
+++ b/irc3d/__init__.py
@@ -168,7 +168,11 @@ class IrcServer(base.IrcObject):
     def notice(self, client, message):
         """send a notice to client"""
         if client and message:
-            messages = utils.split_message(message, self.config.max_length)
+            messages = utils.split_message(
+                message,
+                self.config.max_length,
+                self.encoding,
+            )
             for msg in messages:
                 client.fwrite(':{c.srv} NOTICE {c.nick} :{msg}', msg=msg)
 


Test patch:
diff --git a/tests/test_utils.py b/tests/test_utils.py
index c4daa60..7f06cf6 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -103,13 +103,140 @@ class TestConfig(TestCase):
 
 class TestSplit(TestCase):
 
-    def callFTU(self, messages, max_length=10):
-        return list(split_message(' '.join(messages), max_length))
-
     def test_split_message(self):
-        messages = ['allo', 'alloallo']
-        self.assertEqual(messages, self.callFTU(messages))
-        messages = ['allo\t', 'alloallo']
-        self.assertEqual([m.strip() for m in messages], self.callFTU(messages))
-        messages = ['\x1d \x1f']
-        self.assertEqual(messages, self.callFTU(messages))
+        messages = [
+            'allo',
+            'allo\t',
+            '   allo',
+            'alloallo',
+            'Qwerty uiop asdfghjkl zxcvbnm',
+            '\x1d \x1f',
+            # "Hello world!" in Belarusian, Greek, and Japanese.
+            'Прывітанне свет!',
+            'Γειά σου Κόσμε!',
+            'こんにちは 世界',
+        ]
+        expected = [
+            ['allo'],
+            ['allo\t'],
+            ['   allo'],
+            ['alloallo'],
+            ['Qwerty', 'uiop', 'asdfghjkl', 'zxcvbnm'],
+            ['\x1d \x1f'],
+            ['Прыві', 'танне', 'свет!'],
+            ['Γειά', 'σου', 'Κόσμε', '!'],
+            ['こんに', 'ちは', '世界'],
+        ]
+
+        split = [
+            list(split_message(msg, 10, 'utf-8'))
+            for msg in messages
+        ]
+        self.assertEqual(split, expected)
+
+    def test_split_message_long(self):
+        message = (
+            'Harum qui commodi voluptas veritatis provident voluptatem '
+            'accusamus. Ut odio porro voluptas. Totam perspiciatis dolorem '
+            'maxime beatae sit. Consectetur ducimus qui ut quae. Dolor optio '
+            'minima cupiditate ut. Laborum officia ut voluptas est porro '
+            'nulla qui.'
+        )
+        expected = {
+            50: [
+                # 1
+                'Harum qui commodi voluptas veritatis provident',
+                # 2
+                'voluptatem accusamus. Ut odio porro voluptas.',
+                # 3
+                'Totam perspiciatis dolorem maxime beatae sit.',
+                # 4
+                'Consectetur ducimus qui ut quae. Dolor optio',
+                # 5
+                'minima cupiditate ut. Laborum officia ut voluptas',
+                # 6
+                'est porro nulla qui.',
+            ],
+            100: [
+                # 1
+                'Harum qui commodi voluptas veritatis provident voluptatem '
+                'accusamus. Ut odio porro voluptas. Totam',
+                # 2
+                'perspiciatis dolorem maxime beatae sit. Consectetur ducimus '
+                'qui ut quae. Dolor optio minima',
+                # 3
+                'cupiditate ut. Laborum officia ut voluptas est porro nulla '
+                'qui.',
+            ],
+            150: [
+                # 1
+                'Harum qui commodi voluptas veritatis provident voluptatem '
+                'accusamus. Ut odio porro voluptas. Totam perspiciatis '
+                'dolorem maxime beatae sit.',
+                # 2
+                'Consectetur ducimus qui ut quae. Dolor optio minima '
+                'cupiditate ut. Laborum officia ut voluptas est porro nulla '
+                'qui.',
+            ],
+            200: [
+                # 1
+                'Harum qui commodi voluptas veritatis provident voluptatem '
+                'accusamus. Ut odio porro voluptas. Totam perspiciatis '
+                'dolorem maxime beatae sit. Consectetur ducimus qui ut quae. '
+                'Dolor optio minima',
+                # 2
+                'cupiditate ut. Laborum officia ut voluptas est porro nulla '
+                'qui.',
+            ],
+            300: [message],
+        }
+
+        for max_bytes in expected.keys():
+            split = list(
+                split_message(message, max_bytes, 'utf-8')
+            )
+            self.assertEqual(split, expected[max_bytes])
+
+    def test_split_message_no_whitespace(self):
+        message_len = 100
+        message = 'A' * message_len
+
+        def split_lens(max_bytes):
+            split = list(
+                split_message(message, max_bytes, 'utf-8')
+            )
+            return len(''.join(split)), len(split)
+
+        result = [
+            (max_bytes, *split_lens(max_bytes))
+            for max_bytes in [*range(1, 10), *range(10, 110, 10)]
+        ]
+        expected = [
+            (1, 100, 100),
+            (2, 100, 50),
+            (3, 100, 34),
+            (4, 100, 25),
+            (5, 100, 20),
+            (6, 100, 17),
+            (7, 100, 15),
+            (8, 100, 13),
+            (9, 100, 12),
+            (10, 100, 10),
+            (20, 100, 5),
+            (30, 100, 4),
+            (40, 100, 3),
+            (50, 100, 2),
+            (60, 100, 2),
+            (70, 100, 2),
+            (80, 100, 2),
+            (90, 100, 2),
+            (100, 100, 1),
+        ]
+        self.assertEqual(result, expected)
+
+    def test_split_message_byte_max_bytes_too_small(self):
+        message = 'こんにちは'
+
+        for max_bytes in (1, 2):
+            with self.assertRaisesRegex(ValueError, f'{max_bytes=}'):
+                list(split_message(message, max_bytes, 'utf-8'))