Bug #103427 potential bug in parse_packet
Submitted: 22 Apr 2021 9:28 Modified: 7 May 2021 12:13
Reporter: Siyuan Ren Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server: C API (client library) Severity:S3 (Non-critical)
Version:8.0.22 OS:Any
Assigned to: CPU Architecture:Any
Tags: open_cursor, parse_packet, protocol

[22 Apr 2021 9:28] Siyuan Ren
Description:
in file sql/protocol_classic/cc
bool Protocol_classic::parse_packet(union COM_DATA *data,
                                    enum_server_command cmd) {
      // ...
    case COM_STMT_EXECUTE:
      // ...
      // Get execution flags
      data->com_stmt_execute.open_cursor = static_cast<bool>(*read_pos);

the `read_pos` is 128(in the example of How to repeat) but is casted into bool value 1

in sql/sql_parse.cc
bool dispatch_command(THD *thd, const COM_DATA *com_data,
                      enum enum_server_command command) {

    case COM_STMT_EXECUTE: {
        mysqld_stmt_execute(thd, stmt, com_data->com_stmt_execute.has_new_types,
                            com_data->com_stmt_execute.open_cursor, parameters);

in sql/sql_prepare.cc
void mysqld_stmt_execute(THD *thd, Prepared_statement *stmt, bool has_new_types,
                         ulong execute_flags, PS_PARAM *parameters) {
    bool open_cursor = execute_flags & (ulong)CURSOR_TYPE_READ_ONLY;
    stmt->execute_loop(&expanded_query, open_cursor);

`open_cursor` is coincidentally set to true, thus doing some opening cursor things.

bool Prepared_statement::execute(String *expanded_query, bool open_cursor) {
  if (open_cursor) {
      result = new (m_arena.mem_root) Query_fetch_protocol_binary(thd);
      mysql_open_cursor(thd, result, &cursor)

Is it reasonable? Dose prepare/execute should ever open a cursor?
BTW, I want to ask a trivial question: how does mysqld_stmt_execute() output, is it the same with Query_result_send::send_data() in client interaciotn?
BTW again, what's the difference between `Prepare_statement::m_active_protocol` and `THD::m_protocol/protocol_text/protocol_binary`? It confused me a lot.
I'll appreciate it very much if you reply.

How to repeat:
use mysql.connector

cursor.execute('''
            CREATE TABLE Y(
            id INT,
            y_string CHAR(31),
            value double,
            idxk INT,
            PRIMARY KEY ( id ),
            UNIQUE uni_idx(idxk)
            ) ENGINE=InnoDB;
        ''')

cursor = connection.cursor(prepared=True)
connection.start_transaction()
sql="INSERT INTO Y (id, y_string, value, idxk) VALUES(%s, %s, %s, %s)"
cursor.execute(sql, (1, "y_qwe", -12, 1,))
connection.commit()
cursor.close()

if the sql has no parameters, `open_cursor` is false
[23 Apr 2021 10:21] Siyuan Ren
I found this commit(https://github.com/mysql/mysql-connector-python/commit/1d4f43557b78906d334e378036891794d75...) that fixed the open_cursor flag to be always 0. In prior example I used older version mysql-connector thus the flag is set to 128 in _prepare_binary_integer().
Let's me answer my questions: protocols are used in a LIFO mannar which do different operations on THD::packet. Protocol_binary is pushed before stmt execution in mysqld_stmt_execute() and popped after execution.
[28 Apr 2021 2:53] MySQL Verification Team
Hi,

Are you confirming you do not have a problem with the latest python connector?

thanks
[28 Apr 2021 8:19] Siyuan Ren
Yea, it runs without problems with the latest python connector.
However, the bug point is that the `open_cursor` flag is casted into a boolean value, followed by a bitwise AND to determine doing some opening cursor things.
[7 May 2021 12:13] MySQL Verification Team
Hi,

Thanks! That does seem like a bug :)

all best
Bogdan