ssl.read/write on closed socket raises AttributeError

[cbay]

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]

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.

[pitrou]

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.

[senko]

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]

Thanks for the patch, I will take a look.

[vstinner]

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

[vstinner]

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]

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]

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]

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!