Fix 61285 - SSL connections do not timeout #971
Conversation
Read and write on blocking SSL based IO will now obey the configured timeout.
Updated to meet existing style, and PHP coding standards.
Per requests from users, I refactored the read / write methods and pulled out some of the common code between the new refactored method and php_openssl_enable_crypto(). Personally, I think that too much factoring can reduce readability, but it was specifically asked for.
Conflicts: ext/openssl/xp_ssl.c
|
Mod changed from 0644 to 755, seems by mistake |
|
@staabm In code somewhere? If you are referring to file permissions on xp_ssl.c, I have it as 655 on my filesystem. I didn't know that translated into the git repo... |
|
It seems to be fixed now |
|
I wonder if they (openssl / php maintainers) want my updates for 5.5.20 and 5.6.4. I'd probably have to re-fork php-src to get them (no easy way to merge all of the changes in all branches in php-src with this old clone) but I do have fixes for this for 5.5.20 and 5.6.4 |
|
Looking over the Travis CI log, I don't see why it failed, or what it has to do with the openssl extension... Can someone enlighten me as to what to look for? |
…nal code, to make the DIFF easier to read.
|
//cc @rdlowrey |
|
Any idea on if/when this will be pulled? |
| @@ -1819,6 +1903,32 @@ static size_t php_openssl_sockop_read(php_stream *stream, char *buf, size_t coun | |||
| } | |||
| /* }}} */ | |||
|
|
|||
| struct timeval subtractTimeval( struct timeval a, struct timeval b ) | |||
|
@jpauli updated the 2 functions to static. As for the time function, I was trying to keep it the same as the other calls in the module. Perhaps Daniel Lowrey or Chris Wright should comment to see if that should be done throughout the module? |
|
Once this is merged into trunk, I can re-create my fork and provide fixes for PHP 5.5 and 5.6 if wanted. |
|
ok. I'm wondering if this is ever going to be merged... |
|
I've tested this against current master and everything seems good. As soon as you add the minor OCD change mentioned here I'll merge this PR for master. @bbroerman30: It would be great if you could apply these same changes for 5.5 and 5.6. Like ... really great. That would make it possible to close at least three other bug reports. |
|
As soon as this one gets merged in, I'll create a new fork of php-src and get it merged in to those as well. I'll make that edit above tonight... |
|
@bbroerman30 @rdlowrey is this PR also related to https://bugs.php.net/bug.php?id=48524 or https://bugs.php.net/bug.php?id=41631 ? |
|
I would say Probably. -Brad On 1/21/2015 3:59 AM, Markus Staab wrote:
|
|
@rdlowrey the tweak is done. Once I can, I'll delete my php-src fork and re-fork it to get the other branches so I can apply the same fix to them. |
|
should https://github.com/php/php-src/pull/971/files#r22718485 be addressed? |
|
@staabm TBH I'm not terribly concerned. The non-monotonic @bbroerman30 Awesome. Will merge at my earliest convenience. |
|
Created PR #1038 for the 5.5 branch. |
Updated xp_ssl.c to use non-blocking SSL read and write along with php_pollfd_for to perform a timed wait if data can not be read / written. Tested with blocking (no timeout set), and non-blocking IO