-
-
Notifications
You must be signed in to change notification settings - Fork 29.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SSL sockets do not retain the parent socket's attributes #52770
Comments
In 3.x, SSL sockets are created by dup()ing the original socket and then closing it. No attempt is made to conserve the socket's characteristics, such as the timeout and probably other flags. I understand that it may be too late to change the design decision of using dup() and closing the original, but perhaps we should make a best effort to retain the original socket's characteristics. Or perhaps this limitation should be clearly documented (since especially 2.x works differently). |
It is actually a little funnier. dup() preserves the |
Another consequence is that the following check in __init__: timeout = self.gettimeout()
if timeout == 0.0:
# non-blocking
raise ValueError("do_handshake_on_connect should not be specified for non-blocking sockets") could never get triggered since the timeout is reset to None by virtue of creating a new socket object. |
I committed a fix+tests for the timeout value in r80456 (py3k) and r80457 (3.1). Apparently the socket objects' own dup() method doesn't try to retain anything else than the timeout. I'm leaving this issue as "pending" in case criticism or better options are provided :) |
Well, at the risk of stating the obvious, perhaps the dup() thing should be eliminated. The justification for it seems less than clear, and apparently it causes some problems. That might be a direction to consider in the long term, though, rather than as a (different) immediate fix for this issue. |
I've just found another problem while investigating the cause of some I've reproduced it on an XP VM and the explanation is that, sometimes, I will fix the buildbot issue by using a different logic (simply, call |
getpeername() "sometimes" failing "soon" after a socket is created is a semi-well-known Windows socket... feature. For whatever that's worth. |
I have tried refactoring the ssl code in order not to inherit from socket anymore, but it turned out suboptimal because chunks of code from the socket class then have to be duplicated. Therefore, I instead propose a much simpler approach which is to add a forget() method to socket objects; this method "closes" the socket object (sets the internal fd to -1) without closing the underlying fd at all. This allows to create another socket (actually SSLSocket) from the same fd without having to dup() it. Here is a patch; if the principle is accepted, I will add tests and docs. |
It might be nice to see the version that avoids the dup() and has the duplicate code instead (interesting trade-off ;). Just for the sake of comparison against the forget() proposal. |
Here it is. There's probably a bit of polishing left to do, in the (unlikely) case this is chosen as a solution. |
If nobody chimes in, I will opt for the socket.forget() solution. |
sockforget.patch committed in r83869 (py3k). Thank you for the comments. |
In case someone lands here from the 3.2 what’s new document: The method has been renamed to detach. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: