Issue9177
Created on 2010-07-06 09:52 by cbay, last changed 2022-04-11 14:57 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| ssl-socket-readwrite-after-close.diff | senko, 2013-07-07 13:48 | review | ||
| Messages (9) | |||
|---|---|---|---|
| msg109379 - (view) | Author: Cyril (cbay) | Date: 2010-07-06 09:52 | |
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.
|
|||
| msg160140 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2012-05-07 11:41 | |
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.
|
|||
| msg192556 - (view) | Author: Senko Rasic (senko) * | Date: 2013-07-07 13:48 | |
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). |
|||
| msg193275 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2013-07-18 09:50 | |
Thanks for the patch, I will take a look. |
|||
| msg193334 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2013-07-18 23:36 | |
ssl-socket-readwrite-after-close.diff: instead of testing "not self._sslobj", I would prefer an explicit "self._sslobj is None". |
|||
| msg193335 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2013-07-18 23:38 | |
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")
|
|||
| msg193336 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2013-07-18 23:41 | |
Oops, remove "io.RawIOBase._checkClosed(self)" from my example (I read socket.py at the same time, SSLSocket inherits from socket, not from RawIOBase). |
|||
| msg193415 - (view) | Author: Roundup Robot (python-dev) ![]() |
Date: 2013-07-20 17:35 | |
New changeset eda7e86bf03c by Antoine Pitrou in branch 'default': Issue #9177: Calling read() or write() now raises ValueError, not AttributeError, on a closed SSL socket. http://hg.python.org/cpython/rev/eda7e86bf03c |
|||
| msg193417 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2013-07-20 17:39 | |
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! |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:57:03 | admin | set | github: 53423 |
| 2013-07-20 17:39:24 | pitrou | set | status: open -> closed resolution: fixed messages: + msg193417 stage: patch review -> resolved |
| 2013-07-20 17:35:32 | python-dev | set | nosy:
+ python-dev messages: + msg193415 |
| 2013-07-18 23:41:29 | vstinner | set | messages: + msg193336 |
| 2013-07-18 23:38:52 | vstinner | set | messages: + msg193335 |
| 2013-07-18 23:36:28 | vstinner | set | nosy:
+ vstinner messages: + msg193334 |
| 2013-07-18 09:50:00 | pitrou | set | stage: needs patch -> patch review messages: + msg193275 versions: + Python 3.4, - Python 3.3 |
| 2013-07-07 13:48:52 | senko | set | files:
+ ssl-socket-readwrite-after-close.diff nosy:
+ senko keywords: + patch |
| 2012-05-07 11:41:34 | pitrou | set | type: behavior -> enhancement messages: + msg160140 versions: - Python 2.7, Python 3.2 |
| 2012-05-07 11:19:11 | ezio.melotti | set | nosy:
+ pitrou, ezio.melotti stage: needs patch versions: + Python 3.3, - Python 2.6, Python 3.1 |
| 2010-07-06 09:52:58 | cbay | create | |

