-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix(mysql): validate parameter count for prepared statements #3857
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
base: main
Are you sure you want to change the base?
Conversation
1d2eba4
to
b1caf01
Compare
Add validation to ensure the number of provided parameters matches the expected count for MySQL prepared statements. This prevents protocol errors by returning an error if the counts do not match before sending the statement for execution.
b1caf01
to
149a36c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like I mentioned, I'm no maintainer but here is some feedback. Also, it is generally recommended to add a regression test when fixing an issue so that might be worth adding.
if arguments.types.len() != metadata.parameters { | ||
return Err(Error::Protocol(format!( | ||
"Prepared statement expected {} parameters but {} parameters were provided", | ||
metadata.parameters, | ||
arguments.types.len() | ||
))); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a maintainer but there is the err_protocol
macro from sqlx-core
which can be used here.
if arguments.types.len() != metadata.parameters { | ||
return Err(Error::Protocol(format!( | ||
"Prepared statement expected {} parameters but {} parameters were provided", | ||
metadata.parameters, | ||
arguments.types.len() | ||
))); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
Replace direct Error::Protocol(format!()) calls with err_protocol! macro in MySQL connection executor.
Does your PR solve an issue?
fixes #3774
Is this a breaking change?
No. This PR adds proper error handling when the number of parameters provided doesn't match the number expected by a prepared statement in MySQL.