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: infinite recursion in mysql connector and improve mysql EE bridge tests #9571
Conversation
98096c9
to
ed4aec1
Compare
e547848
to
c683d1f
Compare
@@ -305,6 +315,8 @@ prepare_sql_to_conn(Conn, [{Key, SQL} | PrepareList]) when is_pid(Conn) -> | |||
?SLOG(info, LogMeta#{result => success}), | |||
prepare_sql_to_conn(Conn, PrepareList); | |||
{error, Reason} -> | |||
% FIXME: we should try to differ on transient failers and |
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.
Possible to fix now?
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 took a shot at it but came to the conclusion that it might require a more thorough refactor. The code is hard to follow as-is and already has several layers of error coping mechanisms, and these should be unified. Created EMQX-8582 to track it.
An infinite loop was triggered in the mysql connector when a query used a prepared statement key that was not among the defined prepared statements on start. We now check that the key is defined among the prepared statements before recursing. It seems that this bug was never triggered in any production code flow and simply found while writing tests. An error return spell fix is also included as well as a FIXME comment regarding running mysql:prepare and not distinguishing between transient failures and syntax errors. Syntax errors should not be retried.
Co-authored-by: Thales Macedo Garitezi <thalesmg@gmail.com>
c6a8dfd
to
13942f5
Compare
Pull Request Test Coverage Report for Build 3756578837
💛 - Coveralls |
the slim build fails will be fixed in another PR. |
This is a continuation of EMQX-7923 and implements further EE mysql bridge tests covering:
A bug was found while implementing tests regarding prepared statements. An infinite loop was triggered in the mysql connector when a query used a prepared statement key that was not among the defined prepared statements on start. We now check that the key is defined among the prepared statements before recursing. It seems that this bug was never
triggered in any production code.
Fixes EMQX-8414.
PR Checklist
Please convert it to a draft if any of the following conditions are not met. Reviewers may skip over until all the items are checked:
changes/
dir