Python 2.7 SocketServer socket shutdown is incompatible with pyOpenSSL

Bug #900792 reported by Jean-Paul Calderone
68
This bug affects 10 people
Affects Status Importance Assigned to Milestone
pyOpenSSL
Confirmed
Undecided
Unassigned
pyopenssl (Debian)
Fix Released
Unknown
pyopenssl (Ubuntu)
Confirmed
Undecided
Unassigned

Bug Description

pyOpenSSL's Connection object has a shutdown method taking no arguments. Standard sockets have a shutdown method taking one argument.

Prior to Python 2.7, SocketServer didn't try to use shutdown, but starting in 2.7 it does try to use shutdown. This fails because it passes SHUT_WR as an argument.

Traceback (most recent call last):
  File "demo.py", line 48, in <module>
    test()
  File "demo.py", line 44, in test
    httpd.serve_forever()
  File "/home/exarkun/Projects/python/branches/2.7/Lib/SocketServer.py", line 227, in serve_forever
    self._handle_request_noblock()
  File "/home/exarkun/Projects/python/branches/2.7/Lib/SocketServer.py", line 287, in _handle_request_noblock
    self.shutdown_request(request)
  File "/home/exarkun/Projects/python/branches/2.7/Lib/SocketServer.py", line 459, in shutdown_request
    request.shutdown(socket.SHUT_WR)
TypeError: shutdown() takes exactly 0 arguments (1 given)

Tags: patch
Revision history for this message
Jean-Paul Calderone (exarkun) wrote :
description: updated
Revision history for this message
Rémy Roy (remyroy) wrote :

Here is a patch I use to solve this bug.

Revision history for this message
Jean-Paul Calderone (exarkun) wrote :

Thanks. I'm not sure about this patch, at least the documentation part. It suggests that the read and write ends of the connection can actually be shut down separately, but they cannot be. At the very least, I would want to see the documentation for this change point out that the *how* parameter is completely ignored, and only present for compatibility with the normal socket interface. Perhaps even better would be to raise an exception on SHUT_RD and SHUT_WR and only allow SHUT_RDWR. However, this would still break SocketServer, which tries to SHUT_WR.

I wish I could suggest another change that seemed to have less chance to introduce confusion, but this seems like a sticky issue. The whole notion of transparently pretending that an SSL connection is identical to a TCP connection is half-baked. Even beyond that, trying to do it with a single API (ie, using Connection as both the nice, featureful SSL connection object *and* the pretend-to-be-TCP object) just makes matters worse. Perhaps the right direction to go in is to try splitting the TCP compatibility object off so it no longer gets mixed up with the SSL object.

Unit tests for the change are also necessary before it can go into trunk.

Thanks again.

Revision history for this message
Rémy Roy (remyroy) wrote :

I have reworked the documentation and I included the unit tests. I've put the changes in my shutdown-fix branch.

I agree with you regarding the best long term decision regarding this should be to split up the TCP concerns from the SSL concerns. I don't think I want to get into this as I lack the bigger picture for the refactoring. I think this fix can be a good incremental change for everyone who suffer from this bug in the meantime.

Revision history for this message
dronus (paul-geisler) wrote :

As pyOpenSSL Connection is meant as a drop in replacement, it should satisfy the TCP Connection API.
If SHUT_WR doesn't make any sense, there could be two ways handling it: Either ignore the parameter completely and explain the slightly different behaviour in the docs, or emulate the behaviour by storing the shutdown parameter and make other API calls react like TCP Connections would in this case.

Also, the TCP Connection developers may check if this parameter is worth it's circumstances and maybe drop it in the future.

Some day, when SSL is the way to go and plain TCP is only used as it's transport, the parameter may be removed again..

Revision history for this message
Jean-Paul Calderone (exarkun) wrote :

Quoting myself from a previous comment:

> The whole notion of transparently pretending that an SSL connection is identical to a TCP connection is half-baked. Even beyond that, trying to do it with a single API (ie, using Connection as both the nice, featureful SSL connection object *and* the pretend-to-be-TCP object) just makes matters worse. Perhaps the right direction to go in is to try splitting the TCP compatibility object off so it no longer gets mixed up with the SSL object.

Joe (j-moudrik)
Changed in pyopenssl:
status: New → Confirmed
Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "shutdown.patch" of this bug report has been identified as being a patch. The ubuntu-reviewers team has been subscribed to the bug report so that they can review the patch. In the event that this is in fact not a patch you can resolve this situation by removing the tag 'patch' from the bug report and editing the attachment so that it is not flagged as a patch. Additionally, if you are member of the ubuntu-reviewers team please also unsubscribe the team from this bug report.

[This is an automated message performed by a Launchpad user owned by Brian Murray. Please contact him regarding any issues with the action taken in this bug report.]

tags: added: patch
Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in pyopenssl (Ubuntu):
status: New → Confirmed
Changed in pyopenssl (Debian):
status: Unknown → Confirmed
Revision history for this message
Mark Stillwell (marklee) wrote :

Has there been any attempt to resolve this bug in 2014? It's still causing problems.

Revision history for this message
Jean-Paul Calderone (exarkun) wrote :

Note that issues are tracked on github now. All Launchpad issues were transferred there... Or I thought they were. I can't find this one on github, though. :(

pyOpenSSL code is also now on github. You can find branches and pull requests there (and presumably perusing them will answer your question about whether anyone is working on this issue).

Changed in pyopenssl (Debian):
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.