| msg68517 - (view) |
Author: Simon Cross (hodgestar) |
Date: 2008-06-21 17:13 |
| It appears that ssl.SSLSocket attempts to override some of the methods socket.socket delegates to the underlying _socket.socket methods. However, this overriding fails because socket.socket.__init__ replaces all the methods mentioned in _delegate_methods. These methods are: recv, recvfrom, recv_into, recvfrom_into, send and sendto. ssl.SSLSocket implements recv and send. Neither of these overrides will actually work. ssl.SSLSocket also implements recv_from and send_to which seem to be attempts to override recvfrom and sendto. In the Py3k branch it looks like the overriding done by socket.socket.__init__ is gone so that these methods are now potentially overridable. This could potentially be emulated in trunk by checking for the methods using hasattr(...) before overriding them. I'm happy to create patches which fix the method names and clean up the methods on both branches and add tests. I just want to check that I have an accurate picture of what's needed before starting on them. |
|
|
| msg68913 - (view) |
Author: Bill Janssen (janssen) *  |
Date: 2008-06-28 21:31 |
| Thanks for pointing this out. I've got a patch for the overrides and the misnamings already. We don't have implementations for recvfrom_into, or recv_into. If you'd care to create a patch for that, it would help. |
|
|
| msg68921 - (view) |
Author: Bill Janssen (janssen) * |
Date: 2008-06-28 22:22 |
| I've committed the patch which I have for 2.6. Still need renamings for 3.0, and implementations of recvfrom_into and recv_into. |
|
|
| msg72448 - (view) |
Author: Bill Janssen (janssen) * |
Date: 2008-09-04 01:19 |
| Simon, Python 3.x now has recvfrom and recv_into, but not recvfrom_into. If you'd like to work up a patch for that, I'll add it to the next release cycle. |
|
|
| msg72586 - (view) |
Author: Simon Cross (hodgestar) |
Date: 2008-09-05 12:00 |
| I've attached a patch for trunk / 2.6 that adds recv_into and recvfrom_into methods to ssl.SSLSocket and does a minor re-ordering of the nasty lambdas in __init__. The recv_into method is a port of the one from 3.0. The recvfrom_into method is a simple "fail if we have an SSL object or pass down if we don't". For the record, I'm against the lambdas in SSLSocket.__init__ and would prefer the solution I suggested previously of putting a hasattr(...) check into socket.socket (I can submit a patch if useful). As is probably abundantly clear at the moment :), none of the SSLSocket methods in questions have tests in test_ssl -- I will move on to trying to add them next. I'm also going to attach a patch for 3.0 which adds recvfrom_into (a port of the 2.6 one). |
|
|
| msg72588 - (view) |
Author: Simon Cross (hodgestar) |
Date: 2008-09-05 12:01 |
| Attach recvfrom_into method patch for 3.0. |
|
|
| msg72601 - (view) |
Author: Simon Cross (hodgestar) |
Date: 2008-09-05 15:27 |
| Tests for recv* and send* methods in 3.0 (2.6 tests coming shortly). |
|
|
| msg72603 - (view) |
Author: Simon Cross (hodgestar) |
Date: 2008-09-05 16:49 |
| Test for recv* and send* in 2.6. The tests revealed some bugs so this patch fixes those and supercedes trunk-ssl-methods.patch. Of particular note is that recv_into called self.read(buf, nbytes) which isn't supported in _ssl.c in 2.6. I've worked around it by creating a temporary buffer in the Python code. This isn't great for large messages, but the alternative is to modify _ssl.c itself (which I suspect is unlikely to make it in to 2.6 at this stage). |
|
|
| msg72604 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2008-09-05 16:58 |
| > self.read(buf, nbytes) Shouldn't this function be named readinto()? |
|
|
| msg72606 - (view) |
Author: Simon Cross (hodgestar) |
Date: 2008-09-05 17:05 |
| >> self.read(buf, nbytes) > Shouldn't this function be named readinto()? There is no readinto function (and I suspect one is unlikely to be added now). In Py3k SSLSocket.read takes both len and buffer as optional arguments. |
|
|
| msg72607 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2008-09-05 17:08 |
| I asked this because I find the signature misleading: read(len) or read(buffer, len) io.py, for example, defines read(len) and read_into(buffer). |
|
|
| msg72608 - (view) |
Author: Bill Janssen (janssen) * |
Date: 2008-09-05 17:09 |
| Simon, thanks, this patch looks good to me (2.6 only, right?). I'll try it out and report back. If it looks good, I'll commit it. |
|
|
| msg72609 - (view) |
Author: Simon Cross (hodgestar) |
Date: 2008-09-05 17:15 |
| @Bill Janssen: There are currently two patch sets which I think should be applied. For 2.6 it's just trunk-ssl-tests.patch. For 3.0 it's 3k-ssl-methods.patch and 3k-ssl-tests.patch. @Amaury Forgeot d'Arc I agree it's not great nomenclature but that's a whole separate can of worms and not one I want to open in this bug (and in any case, it has practically zero hope of making it into 2.6/3.0 at this point). |
|
|
| msg72611 - (view) |
Author: Bill Janssen (janssen) * |
Date: 2008-09-05 17:24 |
| Re: nomenclature I think this is partly a design bug on my part, supporting the old pre-2.6 "read" and "write" methods on the SSL context. Users should really call "makefile" to get something they can "read" and "write"; those methods should, in 2.6, only be internal, and not exposed outside the class. Of course, that would complicate the socket.ssl compatibility code. |
|
|
| msg72781 - (view) |
Author: Bill Janssen (janssen) * |
Date: 2008-09-08 16:38 |
| I've applied Simon's patch to the 2.6 trunk. |
|
|
| msg72782 - (view) |
Author: Bill Janssen (janssen) * |
Date: 2008-09-08 16:45 |
| And for the 3K branch. Thanks! |
|
|
| msg72788 - (view) |
Author: Simon Cross (hodgestar) |
Date: 2008-09-08 20:52 |
| And thanks for looking at them and applying! :) |
|
|