Skip to content

Conversation

@bbroerman30
Copy link
Contributor

This should fix bugs 61285 and 62605. I have tested with blocking and non-blocking SSL streams in PHP and in a PHP extension I am working on.

Read and write on blocking SSL based IO will now obey the configured timeout.
@bbroerman30
Copy link
Contributor Author

Looks like the git clone on the build request above timed out... You may want to re-try, with higher timeout values on GIT.

Updated to meet existing style, and PHP coding standards.
@cataphract
Copy link
Contributor

There seems to be quite a lot of duplicate logic. Do you think you can refactor this? See also php_openssl_enable_crypto.

@bbroerman30
Copy link
Contributor Author

When I get the opportunity, I will look into it. I really don't have
much spare time, and my goal for that weekend was to get it working
properly. I will try to spend some time on it this weekend.

Thanks

On 2013-02-03 10:26, Gustavo Lopes wrote:

There seems to be quite
a lot of duplicate logic. Do you think you can refactor this? See also
php_openssl_enable_crypto.

Reply to this email directly or
view it on GitHub [1].

Links:

[1]
#264 (comment)

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.
@boenrobot
Copy link

Now that the aforementioned refactoring is done...

Could anyone merge this already please?

@boenrobot
Copy link

@bbroerman30 BTW, you have finished said refactoring, right?

@bbroerman30
Copy link
Contributor Author

A long time ago.


From: Vasil Rangelov [mailto:[email protected]]
Sent: Monday, July 22, 2013 1:48 PM
To: php/php-src
Cc: Brad Broerman
Subject: Re: [php-src] Updated openSSL to fix timout issue on SSL streams -
Bug #61285 (#264)

@bbroerman30 https://github.com/bbroerman30 BTW, you have finished said
refactoring, right?

Reply to this email directly or view it on
#264 (comment) GitHub.
<https://github.com/notifications/beacon/DjvRmz9yGC8Pn2OiPh_tkqTbKPAuSlWG9qN
yDKejn9sxbAo6dhO-ubE2J1QhunY2.gif>

Conflicts:
	ext/openssl/xp_ssl.c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants