Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Bug #73462 - Persistent connections don't set $connect_errno #2195

Closed
wants to merge 3 commits into from
Closed

Fix Bug #73462 - Persistent connections don't set $connect_errno #2195

wants to merge 3 commits into from

Conversation

darkain
Copy link
Contributor

@darkain darkain commented Nov 8, 2016

Persistent connections skipped resetting $connect_error and $connect_errno values
This adds the "clear error" line to persistent connections for consistency

Persistent connections skipped resetting $connect_error and $connect_errno values
This adds the "clear error" line to persistent connections for consistency
@darkain
Copy link
Contributor Author

darkain commented Nov 9, 2016

The Travis CI environment is entirely unpredictable. Just pushed in a commit of just white-space change to the markdown file, and all compilation tests passed this time, vs failing randomly and arbitrarily before.

@KalleZ
Copy link
Member

KalleZ commented Nov 9, 2016

@darkain don't bother too much with Travis, I stopped caring much about it a while ago since as you mentioned, is way too inconsistent

@marcosptf
Copy link
Contributor

this bugfix need a test, to be sure that it works fine.
what you think about?

The connection is now verified to be persistent when.
If the connection isn't persistent, then the bug wouldn't present itself anyways.
@php-pulls
Copy link

Comment on behalf of krakjoe at php.net:

adding label

@php-pulls php-pulls added the Bug label Nov 15, 2016
@krakjoe
Copy link
Member

krakjoe commented Jan 3, 2017

Since this targets a security fix only branch of PHP, and since the author seems to have abandoned working on it anyway, I'm closing this PR.

Please take this action as encouragement to open a clean PR against a supported branch.

@krakjoe krakjoe closed this Jan 3, 2017
@nikic
Copy link
Member

nikic commented Jan 3, 2017

Doesn't look abandoned to me...

@nikic nikic reopened this Jan 3, 2017
@krakjoe
Copy link
Member

krakjoe commented Jan 3, 2017

Also it appears to apply cleanly to master at least ... I'm sure I checked that ... my mistake ...

@nikic
Copy link
Member

nikic commented Jan 3, 2017

Merged via cfd6e14, thanks!

@nikic nikic closed this Jan 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants