Skip to content
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

ssl.read/write on closed socket raises AttributeError #53423

Closed
cbay mannequin opened this issue Jul 6, 2010 · 9 comments
Closed

ssl.read/write on closed socket raises AttributeError #53423

cbay mannequin opened this issue Jul 6, 2010 · 9 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@cbay
Copy link
Mannequin

cbay mannequin commented Jul 6, 2010

BPO 9177
Nosy @pitrou, @vstinner, @ezio-melotti
Files
  • ssl-socket-readwrite-after-close.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2013-07-20.17:39:24.195>
    created_at = <Date 2010-07-06.09:52:58.849>
    labels = ['type-feature', 'library']
    title = 'ssl.read/write on closed socket raises AttributeError'
    updated_at = <Date 2013-07-20.17:39:24.194>
    user = 'https://bugs.python.org/cbay'

    bugs.python.org fields:

    activity = <Date 2013-07-20.17:39:24.194>
    actor = 'pitrou'
    assignee = 'none'
    closed = True
    closed_date = <Date 2013-07-20.17:39:24.195>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2010-07-06.09:52:58.849>
    creator = 'cbay'
    dependencies = []
    files = ['30844']
    hgrepos = []
    issue_num = 9177
    keywords = ['patch']
    message_count = 9.0
    messages = ['109379', '160140', '192556', '193275', '193334', '193335', '193336', '193415', '193417']
    nosy_count = 6.0
    nosy_names = ['pitrou', 'vstinner', 'ezio.melotti', 'cbay', 'python-dev', 'senko']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue9177'
    versions = ['Python 3.4']

    @cbay
    Copy link
    Mannequin Author

    cbay mannequin commented Jul 6, 2010

    This:

    import socket, ssl
    
    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    ssl_sock = ssl.wrap_socket(s)
    ssl_sock.connect(('www.verisign.com', 443))
    ssl_sock.close()
    ssl_sock.read(1024)

    raises:

    Traceback (most recent call last):
      File "/tmp/bug.py", line 10, in <module>
        ssl_sock.read(1024)
      File "/path/to/lib/python2.7/ssl.py", line 138, in read
        return self._sslobj.read(len)
    AttributeError: 'NoneType' object has no attribute 'read'

    I would expect a socket.error instead, which mimics the way regular sockets behave. Indeed, this code:

    import socket, ssl
    
    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    s.connect(('www.verisign.com', 80))
    s.close()
    s.recv(1024)

    raises:

    Traceback (most recent call last):
      File "/tmp/bug.py", line 6, in <module>
        s.recv(1024)
      File "/path/to/lib/python2.7/socket.py", line 170, in _dummy
        raise error(EBADF, 'Bad file descriptor')
    socket.error: [Errno 9] Bad file descriptor

    I've tested on the latest trunks on both 2.7 and 3.2. I've also tested on 2.6 and 3.1.

    I can write a patch that fixes it if the bug is accepted.

    @cbay cbay mannequin added type-bug An unexpected behavior, bug, or error stdlib Python modules in the Lib dir labels Jul 6, 2010
    @pitrou
    Copy link
    Member

    pitrou commented May 7, 2012

    I don't think mimicking EBADF is very useful. Reading from a closed socket is usually a programming error, so it's not the kind of error you'll want to catch at runtime.

    AttributeError may not be very pretty though, so perhaps a ValueError can be raised as with closed files:

    >>> f = open("LICENSE")
    >>> f.close()
    >>> f.read()
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    ValueError: I/O operation on closed file.

    @pitrou pitrou added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels May 7, 2012
    @senko
    Copy link
    Mannequin

    senko mannequin commented Jul 7, 2013

    Here's a patch that adds checks and ValueError raises to SSLSocket.read and SSLSocket.write.

    My first attempt was to add the check to _checkClosed to mirror the IOBase._checkClosed, but in SSLSocket its semantics are different (the idea is for the subclass to add custom checks if needed), and it's called from a lot of places that do gracefully handle closed sockets.

    So I opted to add it manually to only the read and write methods (which allowed for more specific error messages).

    @pitrou
    Copy link
    Member

    pitrou commented Jul 18, 2013

    Thanks for the patch, I will take a look.

    @vstinner
    Copy link
    Member

    ssl-socket-readwrite-after-close.diff: instead of testing "not self._sslobj", I would prefer an explicit "self._sslobj is None".

    @vstinner
    Copy link
    Member

    The check should be moved into the _checkClosed() method. Example:

    def _checkClosed(self):
        io.RawIOBase._checkClosed(self)
        if self._sslobj is None:
            raise ValueError("I/O operation on closed SSL socket")

    @vstinner
    Copy link
    Member

    Oops, remove "io.RawIOBase._checkClosed(self)" from my example (I read socket.py at the same time, SSLSocket inherits from socket, not from RawIOBase).

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 20, 2013

    New changeset eda7e86bf03c by Antoine Pitrou in branch 'default':
    Issue bpo-9177: Calling read() or write() now raises ValueError, not AttributeError, on a closed SSL socket.
    http://hg.python.org/cpython/rev/eda7e86bf03c

    @pitrou
    Copy link
    Member

    pitrou commented Jul 20, 2013

    Ok, I tweaked the error message a bit: _sslobj can also be None when unwrap() has been called on the socket.
    Thank you for contribution!

    @pitrou pitrou closed this as completed Jul 20, 2013
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants