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! :)
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:35 | admin | set | github: 47412 |
2008-09-08 20:52:59 | hodgestar | set | messages:
+ msg72788 |
2008-09-08 16:45:50 | janssen | set | status: open -> closed resolution: accepted -> fixed messages:
+ msg72782 |
2008-09-08 16:38:11 | janssen | set | messages:
+ msg72781 |
2008-09-05 17:24:28 | janssen | set | messages:
+ msg72611 |
2008-09-05 17:15:18 | hodgestar | set | messages:
+ msg72609 |
2008-09-05 17:09:56 | janssen | set | messages:
+ msg72608 |
2008-09-05 17:08:48 | amaury.forgeotdarc | set | messages:
+ msg72607 |
2008-09-05 17:05:03 | hodgestar | set | messages:
+ msg72606 |
2008-09-05 16:58:58 | amaury.forgeotdarc | set | nosy:
+ amaury.forgeotdarc messages:
+ msg72604 |
2008-09-05 16:49:18 | hodgestar | set | files:
+ trunk-ssl-tests.patch messages:
+ msg72603 |
2008-09-05 15:27:50 | hodgestar | set | files:
+ 3k-ssl-tests.patch messages:
+ msg72601 |
2008-09-05 12:01:37 | hodgestar | set | files:
+ 3k-ssl-methods.patch messages:
+ msg72588 |
2008-09-05 12:00:25 | hodgestar | set | files:
+ trunk-ssl-methods.patch keywords:
+ patch messages:
+ msg72586 |
2008-09-04 01:19:05 | janssen | set | messages:
+ msg72448 |
2008-06-28 22:22:51 | janssen | set | messages:
+ msg68921 |
2008-06-28 21:31:03 | janssen | set | assignee: janssen resolution: accepted messages:
+ msg68913 nosy:
+ janssen |
2008-06-21 21:20:37 | giampaolo.rodola | set | nosy:
+ giampaolo.rodola |
2008-06-21 17:13:39 | hodgestar | create | |