[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Antoine Pitrou added the comment: As for partial writes, I'm not sure if it's backwards compatible to turn them on by default, but it might be nice if the option were exposed. Partial writes may have less benefit in Python than in C since we'd have to reallocate and copy a string instead of just moving a pointer. You can slice a memoryview() to avoid a copy. But I'm not sure of the point of partial writes here: can't you just send slices that are small enough (e.g. 4KB each)? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Roundup Robot added the comment: New changeset 60310223d075 by Antoine Pitrou in branch 'default': Issue #8240: Set the SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER flag on SSL sockets. http://hg.python.org/cpython/rev/60310223d075 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Antoine Pitrou added the comment: Ok, I should have fixed the original issue. If you want to see an option to enable partial writes, please open a separate issue. -- resolution: - fixed stage: needs patch - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Changes by Ken Giusti kgiu...@gmail.com: -- nosy: +Ken.Giusti ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Antoine Pitrou added the comment: I'm thinking that perhaps we should simply enable SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER by default. Ben, what do you think? Does the current behaviour allow to catch bugs? -- type: behavior - enhancement versions: +Python 3.4 -Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Ben Darnell added the comment: I vote for enabling SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER by default. It can catch mistakes (i.e. failure to check the return value of send) in Python just as easily as in C, but I don't think those mistakes are common enough to be worth the headache of this error. The false positive rate of this error is higher in Python than in C because we don't have direct control over memory and pointers. As for partial writes, I'm not sure if it's backwards compatible to turn them on by default, but it might be nice if the option were exposed. Partial writes may have less benefit in Python than in C since we'd have to reallocate and copy a string instead of just moving a pointer. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Ben Darnell added the comment: Related pypy issue: https://bugs.pypy.org/issue1238 -- nosy: +Ben.Darnell ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Changes by Jesús Cea Avión j...@jcea.es: -- nosy: +jcea ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Antoine Pitrou pit...@free.fr added the comment: See issue12197 for a related request. -- versions: +Python 3.3 -Python 3.2 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Changes by Antoine Pitrou pit...@free.fr: -- stage: commit review - needs patch versions: +Python 3.2 -Python 2.6, Python 2.7 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Antoine Pitrou pit...@free.fr added the comment: Patch should probably be rewritten to add a `mode` property on the new SSLContext object instead. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Antoine Pitrou pit...@free.fr added the comment: Wouldn't it be nicer if mode was a property? Good point. I guess it would indeed... -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Cyril c...@excellency.fr added the comment: Here is a patch that implements SSLSocket.get_mode/set_mode, with the SSL_MODE_ENABLE_PARTIAL_WRITE and SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER constants defined in the ssl module. The patch contains a test case and documentation. It's made against trunk 44327 and also applies nicely with --fuzz=3 on a 2.6.5. There are no compatibility issues as no specific mode is set by default. It's up to the application to call SSLSocket.set_mode before use. I've tested my own use case with a set_mode(SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER), it works nicely. -- Added file: http://bugs.python.org/file16800/ssl_mode.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Antoine Pitrou pit...@free.fr added the comment: The patch adds a new feature, which makes it unsuitable for 2.6. I guess it could be applied to the 2.7 trunk, although a beta is being released and I'm not sure new features are really welcome afterwards. This one is really small and non-controversial, though, so I'd advocate accepting it. The patch itself looks good. -- nosy: +benjamin.peterson stage: test needed - commit review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Benjamin Peterson benja...@python.org added the comment: Wouldn't it be nicer if mode was a property? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Cyril c...@excellency.fr added the comment: I had a look at how M2Crypto and pyOpenSSL handled this: - M2Crypto has wrappers around SSL_set_mode that let you set the modes you want. From their changelog [1], it was required to be able to operate with Twisted. By default, though, they only set SSL_MODE_AUTO_RETRY. - pyOpenSSL enables everything by default, and there's no set_mode wrapper. Here is the relevant code: /* Some initialization that's required to operate smoothly in Python */ SSL_CTX_set_mode(self-ctx, SSL_MODE_ENABLE_PARTIAL_WRITE | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER | SSL_MODE_AUTO_RETRY); I don't see any other possible alternative. I'm not sure which one is better. Implementing a set_mode wrapper with no mode set by default has no compatibility issues, although we'd still have that 'bad write retry' OpenSSL error. On the other hand, setting SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER by default is easy but we lose some security (and, possibly, some compatibility problems, although I doubt anyone relies on the 'bad write retry' error). What do you think? I'd be ready to write the patch for the set_mode wrapper if you want. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Cyril c...@excellency.fr added the comment: Hmm, indeed. What you can do, very simply, is cache the getvalue() result once you have generated it. After some thoughts, it's not really an option: my cStringIO.StringIO buffer is, well a buffer. To append data to the buffer, I call buffer.write(). When I've got a chance to send data over the socket (remember, it's async, so I don't really know when it's going to happen), I call buffer.getvalue(). If socket.write() returns zero byte written, I'll have to wait until I get another chance to send my buffer. But in the meantime, some more data might get appended to the buffer, and the string returned by getvalue() will be different from the first call (and thus, I can't really cache it). I could find some tricks (like using multiple buffers), but it would be ugly. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Antoine Pitrou pit...@free.fr added the comment: If socket.write() returns zero byte written, I'll have to wait until I get another chance to send my buffer. But in the meantime, some more data might get appended to the buffer, and the string returned by getvalue() will be different from the first call (and thus, I can't really cache it). I could find some tricks (like using multiple buffers), but it would be ugly. Right. I think we should somehow support your use case, but I'm not sure whether it should be the default. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
STINNER Victor victor.stin...@haypocalc.com added the comment: ..., the doc says about SSLSocket.write: Returns the number of bytes written. It actually either returns 0 or len(data), at least as long as we don't have SSL partial writes. That's a different behaviour from regular sockets, and I had to look in _ssl.c to figure out why I never had values inbetween. You should open a new issue for this point. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Cyril c...@excellency.fr added the comment: The following test case exhibits the bug, but I'm not sure it will fail every time as it depends on 2 things: - your connection speed (I guess) - I used the following trick to have 2 identical strings with a different id (memory address): data = (('xx'[0] + 'xx'[1:])*1, ('xx'[0] + 'xx'[1:])*1) I'm not sure it will work all the time though. -- Added file: http://bugs.python.org/file16669/test_ssl.py.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Changes by R. David Murray rdmur...@bitdance.com: -- components: +Extension Modules -Library (Lib) nosy: +giampaolo.rodola ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Changes by R. David Murray rdmur...@bitdance.com: -- priority: - normal stage: - test needed type: - behavior ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Changes by STINNER Victor victor.stin...@haypocalc.com: -- nosy: +haypo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
STINNER Victor victor.stin...@haypocalc.com added the comment: If I understood correctly, the patch only concerns non blocking socket if SSL_write() returns 0? If SSL_write() returns a non zero value, can you use: ssl_socket.send(data[count:])? About the string identifier trick, you should add an assertion to ensure that identifiers are differents. Example: a = 'x' * 2 # create a copy with a different memory address b = a[0:] + a[1:] assert (a == b) and (a is not b) data = a, b See also issue #8222: enabling SSL_MODE_AUTO_RETRY on SSL sockets. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Cyril c...@excellency.fr added the comment: You're right about the assert, I've just uploaded a new patch. In non-blocking mode, ssl_socket.send(data) will return either 0 (which means nothing was sent, you'll have to try again), or len(data) when everything was sent. It can't return anything inbetween. This is because SSL_MODE_ENABLE_PARTIAL_WRITE is not enabled. In my opinion, SSL_MODE_ENABLE_PARTIAL_WRITE should probably be enabled, although I don't know if it would have any consequence on existing code. Note that _ssl.c header has: XXX should partial writes be enabled, SSL_MODE_ENABLE_PARTIAL_WRITE? However, it's totally unrelated to our bug. Issue #8222 is also unrelated since SSL_MODE_AUTO_RETRY only applies to blocking sockets. By the way, this bug was triaged test needed. Am I missing anything? This is my first reported bug, I'm not sure about the process. -- Added file: http://bugs.python.org/file16670/test_ssl.py.patch.v2 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Cyril c...@excellency.fr added the comment: I forgot to talk about the conditions in which I stumbled upon that bug. I use a cStringIO.StringIO as a send buffer. When the socket is ready to send data, I call ssl_socket.send(send_buffer.getvalue()). Unfortunately, two consecutive calls to send_buffer.getvalue() may return a different object (i.e. a string with a different memory address). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
R. David Murray rdmur...@bitdance.com added the comment: test needed is in reference to your assertion that you weren't sure your test would fail reliably. A test that fails some times and passes some times is...suboptimal when dealing with a buildbot testing infrastructure :) -- nosy: +r.david.murray ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Changes by Antoine Pitrou pit...@free.fr: -- nosy: +janssen ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Antoine Pitrou pit...@free.fr added the comment: Since this error seems to be aimed at warning about potential programming errors, I'm not sure it should be silenced. The obvious fix should be to pass the same argument every time (until the data finally gets written). -- nosy: +pitrou ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Cyril c...@excellency.fr added the comment: r.david.murray: ah, sure :) However, I'm not sure a test case is absolutely required for this issue for two reasons: - the fix is trivial: it's a one-liner that enables a SSL mode that explicitely authorizes SSL_write to be called a second time with a a different memory pointer than the first time. Since memory pointers are opaque to Python programmers anyway, I doubt it could break code (unless you'd expect the failure, of course :) ) - tests about SSL in non-blocking mode are almost inexistant, I think. The only one I could find tests the handshake. See issue #3890 for instance. Probably because writing tests in non-blocking mode isn't easy. However, my test may be correct, I'm just not sure it will pass everywhere :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Cyril c...@excellency.fr added the comment: pitrou: that's debatable, since the Python programmer has no control over memory pointers. As I said, I have a cStringIO buffer, and two consecutive calls to buffer.getvalue() yield different objects. What can I do about it? I think it's a rather sane scenario, and I don't feel I'm doing anything wrong. If you think the programmer should be alerted about it, however, then we should at least say it explicitely in the documentation and probably return an explicit Python error. I had to google quite a bit before finding out what this error meant: error:1409F07F:SSL routines:SSL3_WRITE_PENDING:bad write retry -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Antoine Pitrou pit...@free.fr added the comment: pitrou: that's debatable, since the Python programmer has no control over memory pointers. No, but he has control over whether he always uses the same object, or generates a new argument everytime. As I said, I have a cStringIO buffer, and two consecutive calls to buffer.getvalue() yield different objects. What can I do about it? I think it's a rather sane scenario, and I don't feel I'm doing anything wrong. Hmm, indeed. What you can do, very simply, is cache the getvalue() result once you have generated it. If you think the programmer should be alerted about it, however, then we should at least say it explicitely in the documentation and probably return an explicit Python error. I had to google quite a bit before finding out what this error meant: error:1409F07F:SSL routines:SSL3_WRITE_PENDING:bad write retry Indeed, this is cryptic. By the way, I've found a thread explaining this in greater detail: http://readlist.com/lists/openssl.org/openssl-users/0/1794.html Basically, even when SSL_write() says the write must be retried, it does process and buffer some of your data, so that if you retry with different data, some junk will be written out on the SSL socket. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8240] ssl.SSLSocket.write may fail on non-blocking sockets
Cyril c...@excellency.fr added the comment: Switching to a documentation issue is fine to me. Indeed I can just cache the result of StringIO.getvalue(), although it feels a bit crude. I won't be able to create a documentation patch since English is not my primary language. While you're at it, the doc says about SSLSocket.write: Returns the number of bytes written. It actually either returns 0 or len(data), at least as long as we don't have SSL partial writes. That's a different behaviour from regular sockets, and I had to look in _ssl.c to figure out why I never had values inbetween. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8240 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com