Fix MySQL reset packet with prepared statements #611
Conversation
Fixed bug with mysql PS processing/add reset statement handler
decryptor/mysql/response_proxy.go
Outdated
| @@ -380,8 +380,11 @@ func (handler *Handler) ProxyClientConnection(ctx context.Context, errCh chan<- | |||
|
|
|||
| handler.setQueryHandler(handler.QueryResponseHandler) | |||
| break | |||
| case CommandStatementClose, CommandStatementSendLongData, CommandStatementReset: | |||
| case CommandStatementClose, CommandStatementSendLongData: | |||
| clientLog.Debugln("Close|SendLongData|Reset command") | |||
There was a problem hiding this comment.
| clientLog.Debugln("Close|SendLongData|Reset command") | |
| clientLog.Debugln("Close|SendLongData command") |
decryptor/mysql/response_proxy.go
Outdated
| err := packet.SetParameters(newParameters) | ||
| if err != nil { | ||
| log.WithError(err).Error("Failed to update Bind packet") | ||
| return nil |
There was a problem hiding this comment.
we should not send it as is because it is dangerous behavior. application expects that when it sent data out it will be protected and in safe in the database but acra will pass it as is if something fails on its side. Acra should not pass unprotected data to the database if it marked to be protected.
tests/test.py
Outdated
| if args is None: | ||
| args = [] | ||
| with contextlib.closing(mysql.connector.connect( | ||
| use_unicode=False, raw=self.connection_args.raw, charset='ascii', |
There was a problem hiding this comment.
why we don't use unicode and use charset ascii instead of utf8?
There was a problem hiding this comment.
Yes, we can use Unicode. It affects how the non-binary string will be returned.
So then in _result_to_dict we don't need to do the following
columns_name = [bytes(i[0], encoding='utf8').decode('utf8') for i in description]
but can just do (column will be passed in Unicode):
columns_name = [i[0] for i in description]
Regarding the utf8 I tested but it fails, seems driver not supported that, I get the following on charset='utf8'
mysql.connector.errors.ProgrammingError: Character set '255' unsupported
| @@ -4689,6 +4759,24 @@ def executePreparedStatement(self, query, args=None): | |||
| ).execute_prepared_statement(query, args=args) | |||
|
|
|||
|
|
|||
| class TestMysqlConnectorCBinaryPreparedStatement(BasePrepareStatementMixin, BaseTestCase): | |||
There was a problem hiding this comment.
can you add some comment with description what we expected this connector will do? it will help to understand at the future if this test will fail after updating driver or something else. and explain what we expected and should reproduce. because after a year if this test will fail, we will not remember what behavior we expected and what to do to back it again
Fixed after review
Added error returning on GetBindParameters failure
Fixed bug with MySQL PreparedStatements processing, add
Resetpacket handler. Add check on params number inExecuteStatement.PR is draft, as
cossacklabs/ci-py-go-themiscontains new mysql-pyhton-connector with CMySQLConnector. So we need to test it on the CIChecklist
with new changes