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

fix: handle prepare response when number of fields in the prepare header does not match the actual number in the response #2056

Closed
wants to merge 11 commits into from

Conversation

sidorares
Copy link
Owner

@sidorares sidorares commented Jun 12, 2023

ref: #2052

Update: main reason for the #2052 is not the medatdata_follows flag and/or missing parameters. The issue is caused by prepare header returning number of placeholders in the query ( 1 for select * from user order by ? ) but the actual number of parameters following the header is 0 - see #2052 (comment). Changing the scope of this PR to handle "ignored" parameters / columns


Context: we currently expect that prepare response has exactly num fields + num parameters ColumnnDefinition packets in the response. However ether of them could be skipped by the server even when num fields is > 0 and/or num parameters > 0.

See https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_com_stmt_prepare.html

if num_params > 0 and CLIENT_OPTIONAL_RESULTSET_METADATA is not set or if medatdata_follows is RESULTSET_METADATA_FULL num_params packets will follow. Then a EOF_Packet will be transmitted, provided that CLIENT_DEPRECATE_EOF is not set.

Descoping:

  • parse metadata_follows in COM_STMT_PREPARE_OK packet deserealisation
  • expect possible EOF packet where we currently read ColumnDefinition based on OPTIONAL_RESULTSET_METADATA / DEPRECATE_EOF connection flags and metadata_follows COM_STMT_PREPARE_OK packet
  • verify that empty parameters / fields array does not break rest of the "execute PS" logic. For example, we currently might fail .execute() call if expected and actual number of parameters do not match. In that case 2 options: store them separately ( reported number of parameters and array of deserialised parameter metadata ( could be empty ) OR fill array with default / unknown metadata.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 12, 2023

Coverage report

The coverage rate is 90.10981072099408%

The branch rate is 84.96134926212228%

100% of new lines are covered.

@sidorares sidorares mentioned this pull request Jul 5, 2023
4 tasks
@sidorares sidorares changed the title fix: handle prepare response with missing metadata fix: handle prepare response when number of fields in the prepare header does not match the actual number in the response Jul 5, 2023
@sidorares
Copy link
Owner Author

I messed up something with cherry picking, recreating this in a new PR

@sidorares sidorares closed this Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant