uSpike / telnetio

Python Sans-IO telnet parser
MIT License
3 stars 0 forks source link

ValueError: SE: buffer too short #85

Open ethindp opened 9 months ago

ethindp commented 9 months ago

I'm getting this error when negotiating options. Is this a bug with the published wheel or am I doing something wrong?

uSpike commented 9 months ago

Can you add some more information? What data are you inputting? What exactly is the error/traceback?

ethindp commented 9 months ago

@uSpike When I'm negotiating options via IAC DO/WILL (or, rather, the server is doing the negotiation, I'm just accepting), one of two things happens:

If I negotiate some features but, for DOs, I say that there aren't any options supported, some negotiation works before the library starts subcommand negotiation and it gives this error:

>briefcase dev

[mudclient] Installing requirements...
# ...
Installing dev requirements... done

Starting in dev mode...
===========================================================================
Command(cmd=<Opt.DO: 253>, opt=34)
Command(cmd=<Opt.WILL: 251>, opt=3)
Command(cmd=<Opt.DO: 253>, opt=31)
Command(cmd=<Opt.DO: 253>, opt=24)
Command(cmd=<Opt.WILL: 251>, opt=86)
Command(cmd=<Opt.WILL: 251>, opt=70)
Command(cmd=<Opt.WILL: 251>, opt=69)
Command(cmd=<Opt.WILL: 251>, opt=201)
Command(cmd=<Opt.WILL: 251>, opt=91)
Task exception was never retrieved
source_traceback: Object created at (most recent call last):
  File "Lib\site-packages\toga_winforms\app.py", line 257, in run_app
    self.create()
  File "Lib\site-packages\toga_winforms\app.py", line 113, in create
    self.interface._startup()
  File "Lib\site-packages\toga\app.py", line 629, in _startup
    self.startup()
  File "app.py", line 49, in startup
    self.loop.create_task(self.start_telnet())
Traceback (most recent call last):
  File "app.py", line 69, in start_telnet
    self.machine.receive_data(data)
  File "Lib\site-packages\telnetio\_machine.py", line 120, in receive_data
    self._state.send(byte)
  File "Lib\site-packages\telnetio\_machine.py", line 210, in _state_sb
    raise ValueError("SE: buffer too short")
ValueError: SE: buffer too short

That's generated via this code:

    def handle_telnet_event(self, event):
        print(event)
        if isinstance(event, Command):
            self.handle_telnet_command(event)
        elif isinstance(event, SubCommand):
            self.handle_telnet_subcommand(event)
        else:
            print(event)

    def handle_telnet_command(self, event):
        match event.cmd:
            case Opt.NOP: return # Do nothing, no operation
            case Opt.WILL | Opt.WONT | Opt.DO | Opt.DONT:
                option = event.opt
                cmd = event.cmd
                bitmask = 1 << option
                option_enabled = self.features & bitmask
                match cmd:
                    case Opt.WILL:
                        if not option_enabled:
                            self.features |= bitmask
                            self.machine.send_command(Opt.DO, option)
                    case Opt.WONT:
                        if option_enabled:
                            self.features &= ~bitmask
                            self.machine.send_command(Opt.DONT, option)
                    case Opt.DO:
                        if not option_enabled:
                            if self.is_option_supported(option):
                                self.features |= bitmask
                                self.machine.send_command(Opt.WILL, option)
                            else:
                                self.machine.send_command(Opt.WONT, option)
                    case Opt.DONT:
                        if option_enabled:
                            self.features &= ~bitmask
                            self.machine.send_command(Opt.WONT, option)
            case Opt.DM: return # We can't handle this
            case Opt.BRK | Opt.EL:
                self.command_input.value = ""
            case Opt.EC:
                self.command_input.value = self.command_input.value[:len(self.command_input.value)-1]
            case _: return # Unknown command

    def is_option_supported(self, feature):
        return False # Currently we legitimately don't support anything

    def handle_telnet_subcommand(self, event):
        print(event) # We don't handle subcommands for now

On the other hand, if I claim that all possible options are supported, I get a very different error:

Starting in dev mode...
===========================================================================
Command(cmd=<Opt.DO: 253>, opt=34)
Command(cmd=<Opt.WILL: 251>, opt=3)
Command(cmd=<Opt.DO: 253>, opt=31)
Command(cmd=<Opt.DO: 253>, opt=24)
Command(cmd=<Opt.WILL: 251>, opt=86)
Command(cmd=<Opt.WILL: 251>, opt=70)
Command(cmd=<Opt.WILL: 251>, opt=69)
Command(cmd=<Opt.WILL: 251>, opt=201)
Command(cmd=<Opt.WILL: 251>, opt=91)
SubCommand(cmd=<Opt.LINEMODE: 34>, opts=bytearray(b'\x01\x03'))
SubCommand(cmd=<Opt.LINEMODE: 34>, opts=bytearray(b'\x01\x03'))
Task exception was never retrieved
source_traceback: Object created at (most recent call last):
  File "Lib\site-packages\toga_winforms\app.py", line 257, in run_app
    self.create()
  File "Lib\site-packages\toga_winforms\app.py", line 113, in create
    self.interface._startup()
  File "Lib\site-packages\toga\app.py", line 629, in _startup
    self.startup()
  File "app.py", line 49, in startup
    self.loop.create_task(self.start_telnet())
Traceback (most recent call last):
  File "app.py", line 69, in start_telnet
    self.machine.receive_data(data)
  File "Lib\site-packages\telnetio\_machine.py", line 120, in receive_data
    self._state.send(byte)
  File "Lib\site-packages\telnetio\_machine.py", line 212, in _state_sb
    cmd = Opt(buf[0])
          ^^^^^^^^^^^
  File "Python312\Lib\enum.py", line 744, in __call__
    return cls.__new__(cls, value)
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "Python312\Lib\enum.py", line 1158, in __new__
    raise ve_exc
ValueError: 24 is not a valid Opt

(As an aside, 24 is a valid option: it asks the client about it's terminal type.)

I've no idea what's happening with the first error since it doesn't make any sense. However, the second is problematic, particularly for MUDs; my recommendation would be to remove the Opt enum and just pass the option directly onto the client library/application, for two reasons:

  1. DO/WILL/WONT/WILL are not telnet options; they are commands, so passing them off as options is misleading. This one is however a pedantic point and not the most important reason.
  2. The most important reason for this is that telnet is designed to be flexible. Ideally, one should not need to copy the library directly into their application to handle options this library doesn't know about (e.g. GMCP or MCCP).
uSpike commented 9 months ago

OK thank you for the additional info.

DO/WILL/WONT/WILL are not telnet options; they are commands, so passing them off as options is misleading. This one is however a pedantic point and not the most important reason.

This was changed in main but not released yet. Options are just int now.

Also receive_data now returns a list of events including Error instead of raising, so you can ignore said errors if you want.

Can you test with the HEAD of main? There was a small bit of work I wanted to do before releasing another version (1.x) but I could be convinced to cut a release with these changes if they help you.

ethindp commented 9 months ago

@uSpike So, I updated to master and I still get that error, except that it doesn't throw an exception this time (and I'm not really sure what to do when I get an error like that -- do I just drop the problematic byte and move on?). That is: I get Error: Error(kind=<ErrorKind.SE_BUFFER_TOO_SHORT: 3>, data=b'V'), followed by exceptions because it gets treated like ordinary data. My event handling is like this:

            while True:
                data = await reader.read(4096)
                if not data:
                    break
                if data:
                    received_data = bytearray()
                    events = self.machine.receive_data(data)
                    for event in events:
                        if isinstance(event, Command):
                            await self.handle_telnet_command(event.cmd, event.opt)
                        elif isinstance(event, SubCommand):
                            await self.handle_telnet_subcommand(event)
                        elif isinstance(event, Data):
                            received_data += event.msg
                        elif isinstance(event, Error):
                            print(f"Error: {event}")
                        else:
                            print(event)
                    if len(received_data) > 0:
                        await self.handle_telnet_recv(received_data)

I don't know if this is a problem with telnetio or the application I'm connecting to.

ethindp commented 9 months ago

Also, for some reason if I ignore errors, the received bytes end up being garbage. I'm decoding them, so I think this might (?) be a bug.

uSpike commented 8 months ago

and I'm not really sure what to do when I get an error like that -- do I just drop the problematic byte and move on?)

That's up to you. It seems that invalid data is being sent to the state machine, so it's up to you to continue or not.

It's hard for me to help diagnose your issue without having a way to reproduce it. To answer your initial question, there are no known bugs.

If you can log all of the events, or even better all of the input data which causes your error, I could help more easily.