Skip to content

Conversation

@tiran
Copy link
Member

@tiran tiran commented Sep 6, 2017

SSLSocket.sendall() now uses memoryview to create slices of data. This fix
support for all bytes-like object. It is also more efficient and avoids
costly copies.

Signed-off-by: Christian Heimes [email protected]

https://bugs.python.org/issue27340

@tiran tiran added needs backport to 2.7 type-bug An unexpected behavior, bug, or error labels Sep 6, 2017
@tiran tiran force-pushed the bpo-27340-ssl-sendall branch from fd5b2fa to 9d442ed Compare September 6, 2017 13:56
Lib/ssl.py Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should cast the memoryview to a bytes unit, otherwise slicing may work in non-byte units. See e.g. BufferedIOBase.readinto at https://github.com/python/cpython/blob/master/Lib/_pyio.py#L695

Copy link
Member

@vadmium vadmium left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that “bytes-like object” is supposed to cover objects without any array dimensions, and arrays of things other than bytes. But naively slicing a memoryview doesn’t work in those cases, and len may not be supported. Test cases:

>>> nonarray = ctypes.c_int()
>>> h.request("POST", "/", body=nonarray)
  File "/usr/lib/python3.5/ssl.py", line 883, in sendall
    amount = len(data)
TypeError: object of type 'c_long' has no len()
>>> nonbyte_array = array.array("I", (0,)) * int(10e6)
>>> # I expect the server may not receive all 40+ MB if a “send” call does a short write

The way I handle this is with memoryview.cast:

with memoryview(byteslike) as view, view.cast("B") as bytes_view:
    ...

E.g. https://github.com/python/cpython/blob/v3.6.2/Lib/_compression.py#L66

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes . . . bytes-like objects.

@tiran
Copy link
Member Author

tiran commented Sep 6, 2017

Thanks @vadmium and @pitrou

I have updated my PR.

@tiran tiran force-pushed the bpo-27340-ssl-sendall branch from 8987de2 to 2a4e458 Compare September 6, 2017 23:35
SSLSocket.sendall() now uses memoryview to create slices of data. This fix
support for all bytes-like object. It is also more efficient and avoids
costly copies.

Signed-off-by: Christian Heimes <[email protected]>
Signed-off-by: Christian Heimes <[email protected]>
@tiran tiran force-pushed the bpo-27340-ssl-sendall branch from 2a4e458 to 1dae12f Compare September 7, 2017 18:04
@tiran tiran merged commit 888bbdc into python:master Sep 7, 2017
@tiran tiran deleted the bpo-27340-ssl-sendall branch September 7, 2017 21:18
@tiran
Copy link
Member Author

tiran commented Sep 7, 2017

2.7 is not affected.

tiran added a commit to tiran/cpython that referenced this pull request Sep 7, 2017
* bpo-27340: Use memoryview in SSLSocket.sendall()

SSLSocket.sendall() now uses memoryview to create slices of data. This fix
support for all bytes-like object. It is also more efficient and avoids
costly copies.

Signed-off-by: Christian Heimes <[email protected]>

* Cast view to bytes, fix typo

Signed-off-by: Christian Heimes <[email protected]>.
(cherry picked from commit 888bbdc)
@miss-islington
Copy link
Contributor

🐍🍒⛏🤖 Thanks @tiran for the PR, and @tiran for merging it 🌮🎉.I'm working now to backport this PR to: 3.6.

@miss-islington
Copy link
Contributor

Sorry, @tiran and @tiran, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.

@bedevere-bot
Copy link

GH-3434 is a backport of this pull request to the 3.6 branch.

tiran added a commit that referenced this pull request Sep 7, 2017
* bpo-27340: Use memoryview in SSLSocket.sendall()

SSLSocket.sendall() now uses memoryview to create slices of data. This fix
support for all bytes-like object. It is also more efficient and avoids
costly copies.

Signed-off-by: Christian Heimes <[email protected]>

* Cast view to bytes, fix typo

Signed-off-by: Christian Heimes <[email protected]>.
(cherry picked from commit 888bbdc)
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

Successfully merging this pull request may close these issues.

8 participants