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

ext/mysqlnd: fix passing wrong parameter after a893a49 #18014

Open
wants to merge 1 commit into
base: PHP-8.3
Choose a base branch
from

Conversation

manuelm
Copy link
Contributor

@manuelm manuelm commented Mar 10, 2025

Stumbled upon this. Looks like a wrong replacement in a893a49.

Copy link
Member

@kamil-tekiela kamil-tekiela left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Is there any way to add a test for this?

@kamil-tekiela
Copy link
Member

It should also be fixed in earlier versions I presume.

@manuelm manuelm changed the base branch from master to PHP-8.3 March 10, 2025 13:08
@manuelm manuelm changed the base branch from PHP-8.3 to master March 10, 2025 13:08
@manuelm
Copy link
Contributor Author

manuelm commented Mar 10, 2025

Good catch! Is there any way to add a test for this?

I'll take a look.

It should also be fixed in earlier versions I presume.

yeah. forgot about upmerging. PHP-8.3 right?

@manuelm manuelm force-pushed the mysqlnd-persistent-password branch from a50b327 to 985a078 Compare March 10, 2025 13:13
@manuelm manuelm changed the base branch from master to PHP-8.3 March 10, 2025 13:13
@manuelm
Copy link
Contributor Author

manuelm commented Mar 10, 2025

Good catch! Is there any way to add a test for this?

Unfortunately these member variables aren't even used in any internal code. So testing is not possible. Maybe these are some left overs from the reconnect feature?

@kamil-tekiela
Copy link
Member

I don't think we ever had a reconnect feature in mysqlnd. It could be a left-over from some old feature, which sounds plausible given that there were no bug reports for this yet. I think this needs more investigation. If it's not used, then this piece of code should be removed.

@manuelm
Copy link
Contributor Author

manuelm commented Mar 11, 2025

So I looked back in the history and these variables are in since the introduction of st_mysqlnd_connection back in 2007. However even then there was no actual use of the stored information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants