SSLSocket.recv(-1) triggers SystemError

[vadmium]

BPO	26644
Nosy	@benjaminp, @vadmium, @serhiy-storchaka
Files	ssl-negative.patch ssl-negative.v2.patch

^{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 = 'https://github.com/vadmium'
closed_at = <Date 2016-03-27.21:08:01.250>
created_at = <Date 2016-03-26.07:28:02.792>
labels = ['extension-modules', 'type-bug']
title = 'SSLSocket.recv(-1) triggers SystemError'
updated_at = <Date 2016-03-27.21:08:01.249>
user = 'https://github.com/vadmium'

bugs.python.org fields:

activity = <Date 2016-03-27.21:08:01.249>
actor = 'martin.panter'
assignee = 'martin.panter'
closed = True
closed_date = <Date 2016-03-27.21:08:01.250>
closer = 'martin.panter'
components = ['Extension Modules']
creation = <Date 2016-03-26.07:28:02.792>
creator = 'martin.panter'
dependencies = []
files = ['42295', '42299']
hgrepos = []
issue_num = 26644
keywords = ['patch']
message_count = 9.0
messages = ['262488', '262489', '262497', '262498', '262499', '262500', '262505', '262516', '262536']
nosy_count = 4.0
nosy_names = ['benjamin.peterson', 'python-dev', 'martin.panter', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue26644'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6']

[vadmium]

SystemError indicates an internal error that is not supposed to be triggerable from Python code. We should probably raise ValueError like plain sockets instead.

>>> s = create_connection(("python.org", 443))
>>> s.recv(-1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: negative buffersize in recv
>>> ss = ssl.wrap_socket(s)
>>> ss.recv(-1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/proj/python/cpython/Lib/ssl.py", line 910, in recv
    return self.read(buflen)
  File "/home/proj/python/cpython/Lib/ssl.py", line 787, in read
    return self._sslobj.read(len, buffer)
  File "/home/proj/python/cpython/Lib/ssl.py", line 573, in read
    v = self._sslobj.read(len or 1024)
SystemError: Negative size passed to PyBytes_FromStringAndSize

[vadmium]

Proposed patch

[benjaminp]

Is this what other file-like objects do with negatives sizes?

[vadmium]

Socket objects aren’t exactly file-like. Plain non-SSL sockets don’t even have read() methods.

I think giving a meaning to recv(-1) would be an (unwanted) new feature, rather than a bug fix. If you want a file-like object linked to a socket, I would suggest using something like the makefile() method instead of adding to the low-level socket object API.

But to answer your question: no, most file methods treat a negative size as a special request to read until EOF, e.g. read(-1), readline(-1) and readlines(-1) of RawIOBase, BufferedIOBase and TextIOBase. On the other hand, BufferedIOBase.read1(-1) is poorly defined and supported (bpo-23214), but may end up meaning something like “read an arbitrary non-zero chunk with a minimum amount of low-level calls and processing”.

[benjaminp]

Thanks for the explanation. Your patch lgtm.

On Sat, Mar 26, 2016, at 15:01, Martin Panter wrote:

Martin Panter added the comment:

Socket objects aren’t exactly file-like. Plain non-SSL sockets don’t even
have read() methods.

I think giving a meaning to recv(-1) would be an (unwanted) new feature,
rather than a bug fix. If you want a file-like object linked to a socket,
I would suggest using something like the makefile() method instead of
adding to the low-level socket object API.

But to answer your question: no, most file methods treat a negative size
as a special request to read until EOF, e.g. read(-1), readline(-1) and
readlines(-1) of RawIOBase, BufferedIOBase and TextIOBase. On the other
hand, BufferedIOBase.read1(-1) is poorly defined and supported (Issue
23214), but may end up meaning something like “read an arbitrary non-zero
chunk with a minimum amount of low-level calls and processing”.

----------

---

Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue26644\>

---

[vadmium]

Thanks for the reviews. Here is a patch that avoids breaking read(-1, buffer).

[serhiy-storchaka]

LGTM.

[python-dev]

New changeset af92651c22e9 by Martin Panter in branch '3.5':
Issue bpo-26644: Raise ValueError for negative SSLSocket.recv() and read()
https://hg.python.org/cpython/rev/af92651c22e9

New changeset b84d136e0028 by Martin Panter in branch '2.7':
Issue bpo-26644: Raise ValueError for negative SSLSocket.recv() and read()
https://hg.python.org/cpython/rev/b84d136e0028

New changeset 80934ad2356d by Martin Panter in branch 'default':
Issue bpo-26644: Merge SSL negative read fix from 3.5
https://hg.python.org/cpython/rev/80934ad2356d

[vadmium]

The Python 2 fix was slightly different, due to the lack of Argument Clinic.