Skip to content

subclasses of telnetlib.Telnet may crash when used as context manager #118042

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
NichtJens opened this issue Apr 18, 2024 · 7 comments
Closed

subclasses of telnetlib.Telnet may crash when used as context manager #118042

NichtJens opened this issue Apr 18, 2024 · 7 comments
Labels
type-bug An unexpected behavior, bug, or error

Comments

@NichtJens
Copy link

NichtJens commented Apr 18, 2024

Bug report

Bug description:

Subclasses of telnetlib.Telnet may crash when used as context manager due to missing self.sock:

Exception ignored in: <function Telnet.__del__ at 0x7faeb8656ee0>
Traceback (most recent call last):
  File "/usr/lib/python3.8/telnetlib.py", line 239, in __del__
    self.close()
  File "/usr/lib/python3.8/telnetlib.py", line 265, in close
    sock = self.sock
AttributeError: 'TelnetSubclass' object has no attribute 'sock'

A simple example that can trigger the issue would be a Telnet subclass where aliases to hostnames would be provided for making the telnet connection:

import telnetlib

ALIAS_TO_HOST = {
    "alias1": "localhost",
    "alias2": "anotherhost"
}

class AliasTelnet(telnetlib.Telnet):

#    sock = None  # this mitigates the problem

    def __init__(self, alias):
        host = ALIAS_TO_HOST.get(alias)
        if not host:
#            self.sock = None  # alternatively this mitigates the problem
            raise ValueError(f'cannot map alias "{alias}" to host name')

        super().__init__(host)


# this works
with AliasTelnet("alias1") as tn:
    print(tn.read_some().decode())

# this fails
with AliasTelnet("thisisnotaknownalias") as tn:
    pass

What happens is that upon leaving the context due to an exception Telnet.__del__ is called, which calls Telnet.close. If this happens before Telnet.__init__ was called (i.e., the line super().__init__(host) above) the sock attribute does not exist yet.

A simple solution would be to adjust the close method to this:

    def close(self):
        """Close the connection."""
        try:
            sock = self.sock
        except AttributeError:
            sock = None
       ...

Alternatively, sock = None could be attached to the class:

class Telnet:

    sock = None

Probably the first version is preferable since it solves the problem where it happens.

I can, of course, provide a PR for either of the changes. Question is whether it's worth it since telnetlib is going to be deprecated. (Sadly, I'd like to add. I actually like using it and telnetlib3 is not really a straight forward replacement due to being fully async. Any way to petition for telnetlib to stay?)

CPython versions tested on:

3.8, 3.12

Operating systems tested on:

Linux

Linked PRs

@NichtJens NichtJens added the type-bug An unexpected behavior, bug, or error label Apr 18, 2024
@serhiy-storchaka
Copy link
Member

How can it call Telnet.__exit__ before finishing Telnet.__init__?

@NichtJens
Copy link
Author

NichtJens commented Apr 23, 2024

How can it call Telnet.__exit__ before finishing Telnet.__init__?

I am confused. I think I gave an example for this in the report. The short answer is "subclasses". Here is the relevant part of the example:

class AliasTelnet(telnetlib.Telnet):

    def __init__(self, alias):
        host = ALIAS_TO_HOST.get(alias)
        if not host:
            raise ValueError(f'cannot map alias "{alias}" to host name')

        super().__init__(host)

Raising something in AliasTelnet.__init__ will call Telnet.__exit__ on its way out.

@serhiy-storchaka
Copy link
Member

Why does it call __exit__ on its way out? Normally, __exit__ is only called after __enter__, implicitly in the with statement, and you need already created object for this. __init__ should finish successfully before you can use the object as a context manager.

@NichtJens
Copy link
Author

I am sorry, I misread the traceback. It does not happen in __exit__ but in __del__. It's actually saying that in the traceback:

Exception ignored in: <function Telnet.__del__ at 0x7faeb8656ee0>

@NichtJens
Copy link
Author

I updated the report accordingly.

@serhiy-storchaka
Copy link
Member

Indeed, this is a bug. Your second version is more preferable because it is simpler. There is already a check for self.sock.

Note that the telnetlib module was deprecated in Python 3.11 and removed in 3.13.

3.12 is the only version that will get the fix. 3.11 and older only accept security fixes.

@NichtJens
Copy link
Author

Your second version is more preferable because it is simpler. There is already a check for self.sock.

I would have argued the opposite. In particular since you felt it was needed to add the comment:

# for __del__()

So, solving the problem where it happens (reading sock = self.sock) feels cleaner.

But both are fine ...

Note that the telnetlib module was deprecated in Python 3.11 and removed in 3.13.

3.12 is the only version that will get the fix. 3.11 and older only accept security fixes.

Yes, I figured that -- see last paragraph in the report :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants